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.

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

Remove templates & make IAS certificate generation more robust #459

Merged
merged 2 commits into from
Jan 12, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
4 changes: 1 addition & 3 deletions build/__tools__/clean.sh
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,8 @@ check_python_version
# -----------------------------------------------------------------

yell --------------- COMMON ---------------
cd $SRCDIR/common/crypto/verify_ias_report
rm -f ias-certificates.cpp

cd $SRCDIR/common
cmake --build build --target clean
rm -rf build

yell --------------- BIN ---------------
Expand Down
2 changes: 2 additions & 0 deletions build/cmake/SGX.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@ IF (NOT DEFINED CMAKE_LIBRARY_OUTPUT_DIRECTORY)
MESSAGE(FATAL_ERROR "CMAKE_LIBRARY_OUTPUTDIRECTORY must be set")
ENDIF()

SET(IAS_CERTIFICATE_URL "https://certificates.trustedservices.intel.com/Intel_SGX_Attestation_RootCA.pem")

################################################################################
# Internal SGX Variables
################################################################################
Expand Down
2 changes: 1 addition & 1 deletion common/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ OPTION(BUILD_UNTRUSTED "Build modules for running with SGX outside an enclave" O
OPTION(BUILD_CLIENT "Build modules for running clients without SGX" OFF)
OPTION(BLOCK_STORE_DEBUG "Debug logging for block store operations" OFF)

CMAKE_MINIMUM_REQUIRED(VERSION 3.10 FATAL_ERROR)
CMAKE_MINIMUM_REQUIRED(VERSION 3.16 FATAL_ERROR)
FIND_PACKAGE(PkgConfig REQUIRED)

IF (NOT DEFINED ENV{PDO_SOURCE_ROOT})
Expand Down
12 changes: 9 additions & 3 deletions common/crypto/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -39,18 +39,24 @@ 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(PROJECT_GENERATED_IAS_SOURCES ${CMAKE_CURRENT_SOURCE_DIR}/verify_ias_report/ias-certificates.txt)
SET_SOURCE_FILES_PROPERTIES(${PROJECT_GENERATED_IAS_SOURCES} PROPERTIES GENERATED TRUE)
SET(FETCH_IAS_CERTS ${CMAKE_CURRENT_SOURCE_DIR}/verify_ias_report/fetch_ias_certificates.sh)

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
COMMAND ${FETCH_IAS_CERTS} ${IAS_CERTIFICATE_URL} ${PROJECT_GENERATED_IAS_SOURCES}
DEPENDS ${FETCH_IAS_CERTS}
WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}/verify_ias_report
)

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

SET_PROPERTY(
TARGET generate-ias-files
APPEND
PROPERTY ADDITIONAL_CLEAN_FILES ${PROJECT_GENERATED_IAS_SOURCE})

IF (${SGX_MODE} STREQUAL "HW")
SET(IAS_CA_CERT_REQUIRED "IAS_CA_CERT_REQUIRED=1")
ENDIF()
Expand Down
1 change: 0 additions & 1 deletion common/crypto/crypto.h
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
2 changes: 1 addition & 1 deletion common/crypto/verify_ias_report/.gitignore
Original file line number Diff line number Diff line change
@@ -1 +1 @@
ias-certificates.cpp
ias-certificates.txt
49 changes: 0 additions & 49 deletions common/crypto/verify_ias_report/build_ias_certificates_cpp.sh

This file was deleted.

69 changes: 69 additions & 0 deletions common/crypto/verify_ias_report/fetch_ias_certificates.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
#!/bin/bash
# Copyright 2023 Intel Corporation
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

# This script sets up the IAS root certificate for inclusing in the
# verification module.
#
# Two parameters:
# $1 -- the URL where the IAS certificate can be retrieved
# $2 -- the file where the certificate should be written

# -----------------------------------------------------------------
# -----------------------------------------------------------------
source ${PDO_SOURCE_ROOT}/bin/lib/common.sh

IAS_CERTIFICATE_URL=$1

# -----------------------------------------------------------------
# set up the temporary files
# -----------------------------------------------------------------
SAVE_FILE=$(mktemp /tmp/pdo-ias-certificate.XXXXXXXXX)
STRING_FILE=$(mktemp /tmp/pdo-ias-certificate-string.XXXXXXXXX)

function cleanup {
rm -f ${SAVE_FILE} ${STRING_FILE}
}

trap 'echo "**ERROR - line $LINENO**"; cleanup; exit 1' HUP INT QUIT PIPE TERM ERR

# If there is no requirement for HW support, then we don't need
# a valid certificate; just generate a dummy string
if [ "${SGX_MODE}" != "HW" ]; then
echo 'R"IASCERT(' > ${STRING_FILE}
echo 'NO CERTIFICATE REQUIRED' >> ${STRING_FILE}
echo ')IASCERT"' >> ${STRING_FILE}

try mv ${STRING_FILE} $2
exit
fi

# -----------------------------------------------------------------
# get the certificate and format it as needed
# -----------------------------------------------------------------

# This is a small hack to make the script work for people
# who would otherwise attempt to retrieve the certficiates
# without a proxy server
Comment on lines +57 to +58
Copy link
Contributor

Choose a reason for hiding this comment

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

Text seems a bit misleading? This is really only for folks (a) inside intel with a proxy server but (b) define no_proxy generically as intel.com rather more narrowly as, e.g., .jf.intel.com,10.0.0.0/8,134.134.0.0/16,localhost,.local,127.0.0.0/8,172.16.0.0/12,192.168.0.0/16,.devtools.intel.com,.iglb.intel.com,isscorp.intel.com,.corp.intel.com,.caas.intel.com,certificates.intel.com ?

Copy link
Contributor Author

@cmickeyb cmickeyb Jan 4, 2024

Choose a reason for hiding this comment

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

its not possible with no-proxy alone to enumerate all possible hosts that might be internal. you can certainly add in some portion of the correct hosts & make it work. or you can just set this to force access through the proxy.

and while this is specifically a problem for intel employees who don't want to enumerate all possible exceptions in the no_proxy variable, those who don't have the problem will require no changes.

Copy link
Contributor

Choose a reason for hiding this comment

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

I guess with my ramble in the second part i hid the main point i wanted to say is that the hack is specific to intel folks in some circumstances. Hopefully, below text makes it a bit clearer.

Suggested change
# who would otherwise attempt to retrieve the certficiates
# without a proxy server
# inside intel who would otherwise attempt to retrieve the certficiates
# without a proxy server due to `no_proxy=intel.com` or alike

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I guess with my ramble in the second part i hid the main point i wanted to say is that the hack is specific to intel folks in some circumstances. Hopefully, below text makes it a bit clearer.

I specifically do not want to reference intel in any external code. We KNOW that its there for us. But the code is intended to be a general (if not very important under most circumstances) solution.

if [ "${PDO_FORCE_IAS_PROXY}" == "true" ]; then
try curl --noproxy '' --retry 3 --max-time 10 -sL --output ${SAVE_FILE} ${IAS_CERTIFICATE_URL}
else
try curl --retry 3 --max-time 10 -sL --output ${SAVE_FILE} ${IAS_CERTIFICATE_URL}
fi

echo 'R"IASCERT(' > ${STRING_FILE}
cat ${SAVE_FILE} >> ${STRING_FILE}
echo ')IASCERT"' >> ${STRING_FILE}

try mv ${STRING_FILE} $2
32 changes: 0 additions & 32 deletions common/crypto/verify_ias_report/ias-certificates.h

This file was deleted.

29 changes: 0 additions & 29 deletions common/crypto/verify_ias_report/ias-certificates.template

This file was deleted.

7 changes: 5 additions & 2 deletions common/crypto/verify_ias_report/verify-report.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,12 @@
#include <string.h>

#include "c11_support.h"
#include "ias-certificates.h"
#include "parson.h"

const char* const ias_report_signing_ca_cert_pem =
#include "ias-certificates.txt"
;

//<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
//########### INTERNAL FUNCTIONS #########################################
//########################################################################
Expand Down Expand Up @@ -211,7 +214,7 @@ verify_status_t verify_ias_report_signature(const char* ias_attestation_signing_
return VERIFY_FAILURE;
}

verify_status_t verify_ias_certificate_chain(const char* cert_pem)
verify_status_t verify_ias_certificate_chain(const char* const cert_pem)
#ifndef IAS_CA_CERT_REQUIRED
{
return VERIFY_FAILURE; // fail (conservative approach for simulator-mode and in absence of CA
Expand Down
4 changes: 3 additions & 1 deletion common/crypto/verify_ias_report/verify-report.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@

#include <sgx_quote.h>

extern const char* const ias_report_signing_ca_cert_pem;

typedef enum
{
VERIFY_SUCCESS,
Expand Down Expand Up @@ -52,7 +54,7 @@ verify_status_t verify_enclave_quote_status(const char* ias_report,
unsigned int ias_report_len,
unsigned int quote_status_flags);
verify_status_t verify_ias_certificate_chain(const char* cert_pem);
verify_status_t verify_ias_report_signature(const char* ias_attestation_signing_cert_pem,
verify_status_t verify_ias_report_signature(const char* const ias_attestation_signing_cert_pem,
const char* ias_report,
const unsigned int ias_report_len,
const char* ias_signature,
Expand Down
1 change: 0 additions & 1 deletion common/interpreter/wawaka_wasm/WasmCryptoExtensions.cpp
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
2 changes: 1 addition & 1 deletion common/tests/crypto/testCrypto.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@

#include "c11_support.h"
#include "crypto.h"
#include "crypto/verify_ias_report/ias-certificates.h"
#include "crypto/verify_ias_report/verify-report.h"
#include "error.h"
#include "log.h"
#include "packages/parson/parson.h"
Expand Down
Loading