Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move trusted root cert configuration to cmake and cleanup #440

Closed
wants to merge 3 commits into from
Closed

Move trusted root cert configuration to cmake and cleanup #440

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d39e3c1
move trusted root cert configuration to cmake and cleanup
bvavala May 6, 2023
5fc70ec
embed the header template in the cmake file, and remove the template
bvavala May 8, 2023
eb55e08
add show progress
bvavala May 8, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 22 additions & 18 deletions common/crypto/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,23 +39,23 @@ ENDIF()
# by the client (ias verification requires sgx).
################################################################################
IF (BUILD_TRUSTED OR BUILD_UNTRUSTED)
SET(PROJECT_GENERATED_IAS_SOURCES ${CMAKE_CURRENT_SOURCE_DIR}/verify_ias_report/ias-certificates.cpp)
SET_SOURCE_FILES_PROPERTIES(${PROJECT_GENERATED_IAS_SOURCES} PROPERTIES GENERATED TRUE)

ADD_CUSTOM_COMMAND(
OUTPUT ${PROJECT_GENERATED_IAS_SOURCES}
COMMAND ./build_ias_certificates_cpp.sh
DEPENDS verify_ias_report/ias-certificates.template verify_ias_report/build_ias_certificates_cpp.sh
WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}/verify_ias_report
SET(CRYPTO_BUILD_DIR ${CMAKE_BINARY_DIR}/crypto)
SET(INTEL_ROOT_CACERT_PATH ${CRYPTO_BUILD_DIR}/Intel_SGX_Attestation_RootCA.pem)
SET(IAS_CERTIFICATES_H_PATH ${CRYPTO_BUILD_DIR}/ias-certificates.h)

FILE(DOWNLOAD
https://certificates.trustedservices.intel.com/Intel_SGX_Attestation_RootCA.pem
${INTEL_ROOT_CACERT_PATH}
TLS_VERIFY ON
)
FILE(READ
${INTEL_ROOT_CACERT_PATH}
INTEL_ROOT_CA_PEM
)
CONFIGURE_FILE(
${CMAKE_CURRENT_SOURCE_DIR}/verify_ias_report/ias-certificates.h.in
${IAS_CERTIFICATES_H_PATH}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i suggest you add a custom command to the clean target to remove these

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

one more... it might also be useful to turn on "show progress" for this file

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i suggest you add a custom command to the clean target to remove these

We could, but we would end up in a situation where cmake generates the artifacts and make cleans them up.
In that case, it's probably better to keep the script for fetching/generating these artifacts, which is called by make.

Keeping these artifacts in the build folder may still make sense considering that:

  • the certificate is valid until 2049 and
  • the header is rebuilt (by cmake!) when the source changes.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

one more... it might also be useful to turn on "show progress" for this file

done

)

ADD_CUSTOM_TARGET(generate-ias-files DEPENDS ${PROJECT_GENERATED_IAS_SOURCES})

IF (${SGX_MODE} STREQUAL "HW")
SET(IAS_CA_CERT_REQUIRED "IAS_CA_CERT_REQUIRED=1")
ENDIF()

LIST(APPEND IAS_SOURCES ${PROJECT_GENERATED_IAS_SOURCES})
ENDIF()

################################################################################
Expand All @@ -65,8 +65,10 @@ IF (BUILD_TRUSTED)
ADD_LIBRARY(${T_CRYPTO_LIB_NAME} STATIC ${PROJECT_HEADERS} ${PROJECT_SOURCES} ${IAS_HEADERS} ${IAS_SOURCES})
SGX_PREPARE_TRUSTED(${T_CRYPTO_LIB_NAME})

#include the crypto build dir to help find the cmake generated headers
TARGET_INCLUDE_DIRECTORIES(${T_CRYPTO_LIB_NAME} PRIVATE ${CRYPTO_BUILD_DIR})

TARGET_COMPILE_DEFINITIONS(${T_CRYPTO_LIB_NAME} PRIVATE ${PDO_DEFAULT_SIGCURVE})
TARGET_COMPILE_DEFINITIONS(${T_CRYPTO_LIB_NAME} PRIVATE ${IAS_CA_CERT_REQUIRED})
ENDIF()

################################################################################
Expand All @@ -76,8 +78,10 @@ IF (BUILD_UNTRUSTED)
ADD_LIBRARY(${U_CRYPTO_LIB_NAME} STATIC ${PROJECT_HEADERS} ${PROJECT_SOURCES} ${IAS_HEADERS} ${IAS_SOURCES})
SGX_PREPARE_UNTRUSTED(${U_CRYPTO_LIB_NAME})

#include the crypto build dir to help find the cmake generated headers
TARGET_INCLUDE_DIRECTORIES(${U_CRYPTO_LIB_NAME} PRIVATE ${CRYPTO_BUILD_DIR})

TARGET_COMPILE_DEFINITIONS(${U_CRYPTO_LIB_NAME} PRIVATE ${PDO_DEFAULT_SIGCURVE})
TARGET_COMPILE_DEFINITIONS(${U_CRYPTO_LIB_NAME} PRIVATE ${IAS_CA_CERT_REQUIRED})

# critical flag for conditional compile in source code
TARGET_COMPILE_DEFINITIONS(${U_CRYPTO_LIB_NAME} PRIVATE "_UNTRUSTED_=1")
Expand Down
1 change: 0 additions & 1 deletion common/crypto/crypto.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,5 @@
#include "skenc.h"
#if _CLIENT_ONLY_
#else
#include "verify_ias_report/ias-certificates.h"
#include "verify_ias_report/verify-report.h"
#endif
1 change: 0 additions & 1 deletion common/crypto/verify_ias_report/.gitignore
View file
Open in desktop

This file was deleted.

49 changes: 0 additions & 49 deletions common/crypto/verify_ias_report/build_ias_certificates_cpp.sh
View file
Open in desktop

This file was deleted.

32 changes: 0 additions & 32 deletions common/crypto/verify_ias_report/ias-certificates.h
View file
Open in desktop

This file was deleted.

18 changes: 18 additions & 0 deletions common/crypto/verify_ias_report/ias-certificates.h.in
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
/*
* Copyright 2023 Intel Corporation
*
* SPDX-License-Identifier: Apache-2.0
*/

#pragma once

#ifdef __cplusplus
extern "C" {
#endif
extern const char ias_report_signing_ca_cert_pem[] =
R"MLT(${INTEL_ROOT_CA_PEM}
)MLT"
;
#ifdef __cplusplus
}
#endif
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i still dont understand why this is necessary.
you can build the file in the cmake completely without using a template.

29 changes: 0 additions & 29 deletions common/crypto/verify_ias_report/ias-certificates.template
View file
Open in desktop

This file was deleted.

7 changes: 0 additions & 7 deletions common/crypto/verify_ias_report/verify-report.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -212,12 +212,6 @@ verify_status_t verify_ias_report_signature(const char* ias_attestation_signing_
}

verify_status_t verify_ias_certificate_chain(const char* cert_pem)
#ifndef IAS_CA_CERT_REQUIRED
{
return VERIFY_FAILURE; // fail (conservative approach for simulator-mode and in absence of CA
// certificate)
}
#else // IAS_CA_CERT_REQUIRED is defined
{
/* Using the IAS CA certificate as a root of trust. */
/* Checking that cert is signed by CA. */
Expand Down Expand Up @@ -266,7 +260,6 @@ verify_status_t verify_ias_certificate_chain(const char* cert_pem)
err:
return VERIFY_FAILURE;
}
#endif // IAS_CA_CERT_REQUIRED

/**
* Check if isvEnclaveQuoteStatus is "OK"
Expand Down
1 change: 0 additions & 1 deletion common/interpreter/wawaka_wasm/WasmCryptoExtensions.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
#include "packages/parson/parson.h"

#include "crypto.h"
#include "crypto/verify_ias_report/ias-certificates.h"
#include "error.h"
#include "jsonvalue.h"
#include "log.h"
Expand Down
9 changes: 0 additions & 9 deletions common/tests/crypto/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,6 @@ set(CMAKE_ARCHIVE_OUTPUT_DIRECTORY ${TESTS_OUTPUT_DIR})
set(CMAKE_LIBRARY_OUTPUT_DIRECTORY ${TESTS_OUTPUT_DIR})
set(CMAKE_RUNTIME_OUTPUT_DIRECTORY ${TESTS_OUTPUT_DIR})

IF (BUILD_TRUSTED OR BUILD_UNTRUSTED)
IF (NOT ${SGX_USE_SIMULATOR})
SET(IAS_CA_CERT_REQUIRED_FLAGS "IAS_CA_CERT_REQUIRED=1")
ENDIF()
ENDIF()

SET(CRYPTO_TEST_SOURCE ${CMAKE_CURRENT_SOURCE_DIR}/testCrypto.cpp)
SET(CRYPTO_TEST_INCLUDE ${CMAKE_CURRENT_SOURCE_DIR})

Expand All @@ -43,7 +37,6 @@ IF (BUILD_UNTRUSTED)
# Same compile options as untrusted library
TARGET_INCLUDE_DIRECTORIES(${UNTRUSTED_TEST_NAME} PRIVATE ${CMAKE_CURRENT_SOURCE_DIR})

TARGET_COMPILE_DEFINITIONS(${UNTRUSTED_TEST_NAME} PRIVATE ${IAS_CA_CERT_REQUIRED_FLAGS})
TARGET_COMPILE_DEFINITIONS(${UNTRUSTED_TEST_NAME} PRIVATE "_UNTRUSTED_=1")

# Link the untrusted test application against the untrusted library and openssl
Expand Down Expand Up @@ -75,8 +68,6 @@ IF (BUILD_TRUSTED)
ADD_EXECUTABLE(${TRUSTED_TEST_NAME} trusted/app/TestApp.cpp ${ENCLAVE_EDGE_SOURCES})
SGX_PREPARE_UNTRUSTED(${TRUSTED_TEST_NAME})

TARGET_COMPILE_DEFINITIONS(${TRUSTED_TEST_NAME} PRIVATE ${IAS_CA_CERT_REQUIRED_FLAGS})

TARGET_INCLUDE_DIRECTORIES(${TRUSTED_TEST_NAME} PRIVATE ${CRYPTO_TEST_INCLUDE})

TARGET_LINK_LIBRARIES(${TRUSTED_TEST_NAME} "-Wl,--start-group")
Expand Down
15 changes: 2 additions & 13 deletions common/tests/crypto/testCrypto.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@

#include "c11_support.h"
#include "crypto.h"
#include "crypto/verify_ias_report/ias-certificates.h"
#include "error.h"
#include "log.h"
#include "packages/parson/parson.h"
Expand All @@ -41,6 +40,8 @@ namespace constants = pdo::crypto::constants;
// Error handling
namespace Error = pdo::error;

extern const char ias_report_signing_ca_cert_pem[];

// XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX
// test signature schemes
int testSig(pcrypto::sig::SigCurve sigCurve)
Expand Down Expand Up @@ -1266,28 +1267,16 @@ d4poyb6IW8KCJbxfMJvkordNOgOUUxndPHEi/tb/U7uLjLOgPA==
{ // verify IAS CA certificate against hard-coded one
// TODO: Could check for identity but should probably also work in usual check?
int r = verify_ias_certificate_chain(ias_report_signing_ca_cert_pem);
#ifdef IAS_CA_CERT_REQUIRED
// success expected
COND2LOGERR(r != VERIFY_SUCCESS,
"verify good IAS CA certificate with IAS CA certificate required\n");
#else
// failure expected
COND2LOGERR(r != VERIFY_FAILURE,
"verify good IAS CA certificate with IAS CA certificate NOT required\n");
#endif
}

{ // verify IAS report signing certificate
int r = verify_ias_certificate_chain(ias_report_signing_cert_pem);
#ifdef IAS_CA_CERT_REQUIRED
// success expected
COND2LOGERR(r != VERIFY_SUCCESS,
"verify IAS report signing certificate with IAS CA certificate required\n");
#else
// failure expected
COND2LOGERR(r != VERIFY_FAILURE,
"verify IAS report signing certificate with IAS CA certificate NOT required\n");
#endif
}

{ // verify IAS report signing certificate with null certificate
Expand Down
1 change: 0 additions & 1 deletion common/tests/crypto/trusted/enclave/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ SGX_PREPARE_TRUSTED(${PROJECT_NAME})
ADD_DEPENDENCIES(${PROJECT_NAME} ${T_COMMON_LIB_NAME})
ADD_DEPENDENCIES(${PROJECT_NAME} ${T_CRYPTO_LIB_NAME})

TARGET_COMPILE_DEFINITIONS(${PROJECT_NAME} PRIVATE ${IAS_CA_CERT_REQUIRED_FLAGS})
TARGET_INCLUDE_DIRECTORIES(${PROJECT_NAME} PRIVATE ${CRYPTO_TEST_INCLUDE})

TARGET_LINK_LIBRARIES(${PROJECT_NAME} -Wl,--version-script=${ENCLAVE_LDS})
Expand Down