Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
3758: Remove OpenSSL experimental flag r=mingweishih a=mingweishih

This PR moves the OpenSSL out of experimental features. The changes include
- Set the `BUILD_OPENSSL` to `ON` by default for SGX build (OP-TEE is not supported yet)
- Remove the dedicated OpenSSL configuration from Jenkins pipeline (now OpenSSL will be built on every SGX configurations).
- Fix tests to make them work on Windows build
- Fix the bug where build dependency is not correctly enforced, can be reproduced when building OE with single thread (Fixes openenclave#3745)
- Fix bugs in the tls_e2e tests that cause failure on the gcc release build.
- Update documents

Signed-off-by: Ming-Wei Shih <[email protected]>

Co-authored-by: Ming-Wei Shih <[email protected]>
  • Loading branch information
oeciteam and mingweishih committed Dec 3, 2020
2 parents 6bbb41b + 4ea06e1 commit ad47fba
Show file tree
Hide file tree
Showing 12 changed files with 72 additions and 23 deletions.
1 change: 0 additions & 1 deletion .jenkins/pipelines/Azure/Linux/Jenkinsfile
Original file line number Diff line number Diff line change
Expand Up @@ -426,7 +426,6 @@ try{
"ACC1804 GNU gcc SGX1FLC": { ACCGNUTest() },

"ACC1804 clang-7 Release EEID Experimental LVI FULL Tests": { ACCTest(AGENTS_LABELS["acc-ubuntu-18.04"], 'clang-7', 'Release', ['-DLVI_MITIGATION=ControlFlow', '-DLVI_MITIGATION_SKIP_TESTS=OFF', '-DWITH_EEID=ON']) },
"ACC1804 clang-7 Release OpenSSL Experimental LVI FULL Tests": { ACCTest(AGENTS_LABELS["acc-ubuntu-18.04"], 'clang-7', 'Release', ['-DLVI_MITIGATION=ControlFlow', '-DLVI_MITIGATION_SKIP_TESTS=OFF', '-DBUILD_OPENSSL=ON']) },

"RHEL-8 clang-8 simulation Release": { ACCTest(AGENTS_LABELS['acc-rhel-8'], 'clang', 'Release', ['-DHAS_QUOTE_PROVIDER=OFF'], ['OE_SIMULATION=1']) },
"RHEL-8 clang-8 simulation Debug": { ACCTest(AGENTS_LABELS['acc-rhel-8'], 'clang', 'Debug', ['-DHAS_QUOTE_PROVIDER=OFF'], ['OE_SIMULATION=1']) },
Expand Down
3 changes: 2 additions & 1 deletion 3rdparty/openssl/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -292,7 +292,8 @@ ExternalProject_Add(
INSTALL_COMMAND "")

add_dependencies(openssl_includes openssl_configure)
add_dependencies(openssl_generated openssl_configure)
add_dependencies(openssl_copied openssl_configure)
add_dependencies(openssl_generated openssl_includes)
add_dependencies(openssl_generated openssl_copied)

# Add opensslcrypto (the libcrypto in openssl).
Expand Down
7 changes: 3 additions & 4 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -158,10 +158,9 @@ else () # NOT OE_SGX
endif ()
endif ()

option(BUILD_OPENSSL "[EXPERIMENTAL] Build OpenSSL libraries." OFF)

if (BUILD_OPENSSL AND OE_TRUSTZONE)
message(FATAL_ERROR "BUILD_OPENSSL is not supported on ARM yet.")
if (OE_SGX)
# Currently we only support OpenSSL on SGX.
set(BUILD_OPENSSL ON)
endif ()

set(DEFAULT_TEST_ENCLAVE_CRYPTO_LIB
Expand Down
3 changes: 0 additions & 3 deletions docs/OpenSSLSupport.md
Original file line number Diff line number Diff line change
Expand Up @@ -87,9 +87,6 @@ In addition, OpenSSL by default disables the following algorithms/features
- Heartbeats extension
- SCTP (Stream Control Transimission Protocol) protocol

*NOTE*: The current support is still experimental and for SGX only. To use OpenSSL libraries, developers should specify
`-DBUILD_OPENSSL=ON` cmake option when building the OE SDK.

# How to use RAND APIs

*Note:* Starting from v0.13, users no longer need to manually opt into the RDRAND engine (as described in this
Expand Down
5 changes: 1 addition & 4 deletions enclave/crypto/openssl/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,6 @@ maybe_build_using_clangw(oecryptoopenssl)

enclave_enable_code_coverage(oecryptoopenssl)

enclave_include_directories(oecryptoopenssl PUBLIC
$<BUILD_INTERFACE:${OE_INCDIR}/openenclave>)

enclave_include_directories(
oecryptoopenssl PRIVATE
$<BUILD_INTERFACE:${PROJECT_SOURCE_DIR}/3rdparty/mbedtls/mbedtls/include>
Expand All @@ -35,7 +32,7 @@ enclave_include_directories(
enclave_link_libraries(oecryptoopenssl PUBLIC openssl)

# Enforce the correct build dependency such that OpenSSL will be built first.
add_enclave_dependencies(oecryptoopenssl opensslcrypto)
add_enclave_dependencies(oecryptoopenssl openssl)

set_enclave_property(TARGET oecryptoopenssl PROPERTY ARCHIVE_OUTPUT_DIRECTORY
${OE_LIBDIR}/openenclave/enclave)
Expand Down
2 changes: 1 addition & 1 deletion tests/crypto/cert_tests.c
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
#include <stdio.h>
#include <string.h>
#if defined(OE_USE_OPENSSL)
#include "../../../enclave/crypto/openssl/cert.h"
#include "../../../../enclave/crypto/openssl/cert.h"
#endif

/* Test the internal API that is only avaiable on OpenSSL-based implementation.
Expand Down
5 changes: 3 additions & 2 deletions tests/crypto/enclave/enc/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ if (BUILD_OPENSSL)
enclave_compile_definitions(crypto_openssl_enc PRIVATE CODE_COVERAGE)
endif ()

enclave_include_directories(crypto_openssl_enc PRIVATE
${CMAKE_CURRENT_BINARY_DIR})
enclave_include_directories(
crypto_openssl_enc PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}
${CMAKE_CURRENT_BINARY_DIR})
endif ()
48 changes: 48 additions & 0 deletions tests/openssl_unsupported/enc/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,54 @@ function (add_unsupported_test NAME)
enclave_link_libraries(openssl_unsupported_${NAME}_enc openssl oelibc
oehostsock oehostfs oehostresolver)

if (WIN32)
maybe_build_using_clangw(openssl_unsupported_${NAME}_enc)

# maybe_build_using_clangw populates variables in its parent scope (ie current scope)
# Propagate these variables back up to the caller.

# Propagate library names variables.
set(CMAKE_STATIC_LIBRARY_PREFIX
"${CMAKE_STATIC_LIBRARY_PREFIX}"
PARENT_SCOPE)
set(CMAKE_STATIC_LIBRARY_SUFFIX
"${CMAKE_STATIC_LIBRARY_SUFFIX}"
PARENT_SCOPE)

# Propagate library tool variables.
set(CMAKE_C_CREATE_STATIC_LIBRARY
"${CMAKE_C_CREATE_STATIC_LIBRARY}"
PARENT_SCOPE)
set(CMAKE_CXX_CREATE_STATIC_LIBRARY
"${CMAKE_CXX_CREATE_STATIC_LIBRARY}"
PARENT_SCOPE)

# Propagate linker variables.
set(CMAKE_EXECUTABLE_SUFFIX
"${CMAKE_EXECUTABLE_SUFFIX}"
PARENT_SCOPE)
set(CMAKE_C_STANDARD_LIBRARIES
"${CMAKE_C_STANDARD_LIBRARIES}"
PARENT_SCOPE)
set(CMAKE_C_LINK_EXECUTABLE
"${CMAKE_C_LINK_EXECUTABLE}"
PARENT_SCOPE)
set(CMAKE_CXX_STANDARD_LIBRARIES
"${CMAKE_CXX_STANDARD_LIBRARIES}"
PARENT_SCOPE)
set(CMAKE_CXX_LINK_EXECUTABLE
"${CMAKE_CXX_LINK_EXECUTABLE}"
PARENT_SCOPE)

# Propagate cpmpiler variables.
set(CMAKE_C_COMPILE_OBJECT
"${CMAKE_C_COMPILE_OBJECT}"
PARENT_SCOPE)
set(CMAKE_CXX_COMPILE_OBJECT
"${CMAKE_CXX_COMPILE_OBJECT}"
PARENT_SCOPE)
endif ()

# Exclude the enclave from build.
# From: https://stackoverflow.com/questions/30155619/expected-build-failure-tests-in-cmake
set_enclave_properties(openssl_unsupported_${NAME}_enc PROPERTIES
Expand Down
4 changes: 0 additions & 4 deletions tests/openssl_unsupported/host/host.c
Original file line number Diff line number Diff line change
@@ -1,14 +1,10 @@
// Copyright (c) Open Enclave SDK contributors.
// Licensed under the MIT License.

#include <limits.h>
#include <openenclave/host.h>
#include <openenclave/internal/error.h>
#include <openenclave/internal/tests.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
#include "openssl_unsupported_u.h"

int main(int argc, const char* argv[])
Expand Down
2 changes: 1 addition & 1 deletion tests/tls_e2e/client_enc/openssl_client.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ int launch_tls_client(
}

OE_TRACE_INFO(TLS_CLIENT "new ssl conntection getting created \n");
sscanf(server_port, "%d", &server_port_num); // conver to char* to int
server_port_num = (uint16_t)atoi(server_port);
if (create_socket(client_socket, server_name, server_port_num) != 0)
{
OE_TRACE_ERROR(
Expand Down
1 change: 0 additions & 1 deletion tests/tls_e2e/common/openssl_utility.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ extern struct tls_control_args g_control_config;
oe_result_t generate_certificate_and_pkey(X509*& cert, EVP_PKEY*& pkey)
{
oe_result_t result = OE_FAILURE;
SSL_CTX_set_ecdh_auto(ctx, 1);
uint8_t* output_cert = nullptr;
size_t output_cert_size = 0;
uint8_t* private_key_buf = nullptr;
Expand Down
14 changes: 13 additions & 1 deletion tests/tls_e2e/server_enc/openssl_server.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ int launch_tls_client(
int create_listener_socket(uint16_t port, int& server_socket)
{
int ret = -1;
const int reuse = 1;
struct sockaddr_in addr;
addr.sin_family = AF_INET;
addr.sin_port = htons(port);
Expand All @@ -61,6 +62,17 @@ int create_listener_socket(uint16_t port, int& server_socket)
goto exit;
}

if (setsockopt(
server_socket,
SOL_SOCKET,
SO_REUSEADDR,
(const void*)&reuse,
sizeof(reuse)) < 0)
{
OE_TRACE_ERROR(TLS_SERVER "setsocket failed \n");
goto exit;
}

if (bind(server_socket, (struct sockaddr*)&addr, sizeof(addr)) < 0)
{
OE_TRACE_ERROR(TLS_SERVER "Unable to bind socket to the port \n");
Expand Down Expand Up @@ -194,7 +206,7 @@ int setup_tls_server(struct tls_control_args* config, char* server_port)
goto exit;
}

sscanf(server_port, "%d", &server_port_num); // conver to char* to int
server_port_num = (uint16_t)atoi(server_port);
if (create_listener_socket(server_port_num, server_socket_fd) != 0)
{
OE_TRACE_ERROR(TLS_SERVER
Expand Down

0 comments on commit ad47fba

Please sign in to comment.