Merge #3985

3985: Add positive testcases for config_id verification on icelake VMs r=mingweishih a=manojrupireddy Additional check is added to skip tests on coffee lake VMs. Fix #3903 Made changes to oesign tool. Users need to specify EnableKSS field in conf file to leverage SGX KSS features. Signed-off-by: manoj rupireddy <marupire@microsoft.com> Co-authored-by: manoj rupireddy <marupire@microsoft.com>
openenclave · Jun 23, 2021 · b2908df · b2908df
2 parents a5f59b2 + 89ba93a
commit b2908df
Show file tree

Hide file tree

Showing 18 changed files with 394 additions and 154 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -30,6 +30,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - SGX enclaves created using OE SDK can now be debugged using `oelldb`.
   `oelldb` is a python based extension for LLDB that supports debugging SGX enclaves. lldb-7 or above is required.
 
+### Changed
+- Developers can specify the EnableKSS with a binary value in the enclave config file or set the value via the newly added OE_SET_ENCLAVE_SGX2 macro, which is used to set SGX2-specific properties. Setting EnableKSS to true is a pre requisite to leverage SGX KSS (Key Separation and Sharing) Features.
 
 ### Deprecated
 - The `Release` build type for building the Open Enclave SDK from source is deprecated. The recommendation is using `RelWithDebInfo` instead.

diff --git a/docs/GettingStartedDocs/buildandsign.md b/docs/GettingStartedDocs/buildandsign.md
@@ -99,6 +99,7 @@ NumTCS=1
 ```
 Additionally, a developer can specify additional Key Sharing and Separation (KSS) identity properties
 for use on the platforms that support it (for SGX enclave only):
+- **EnableKSS**: Is the enclave allowed to use Key Sharing and Seperation (KSS).
 - **FamilyID**: The product family identity (ISVFAMILYID for SGX) a developer can specify to group
 different enclaves under a common identity such as an identifier for the application suite
 which includes several enclave apps.
@@ -114,19 +115,21 @@ ExtendedProductID=2768c720-1e28-11eb-adc1-0242ac120002
 ```
 
 As a convenience, you can specify the enclave properties in code using the
-`OE_SET_ENCLAVE_SGX_KSS` macro if KSS properties are included.  For example, the equivalent properties could be
+`OE_SET_ENCLAVE_SGX2` macro.  For example, the equivalent properties could be
 defined in any .c or .cpp file compiled into the enclave:
 
 ```c
-OE_SET_ENCLAVE_SGX_KSS(
-    1,    /* ProductID */
-    1,    /* SecurityVersion */
-    1,    /* Debug */
-    47183823-2574-4bfd-b411-99ed177d3e43, /* FamilyID */
-    2768c720-1e28-11eb-adc1-0242ac120002, /* ExtendedProductID */
-    1024, /* NumHeapPages: heap size in units of 4KB pages */
-    1024, /* NumStackPages: stack size, in units of 4KB pages */
-    1);   /* NumTCS */
+OE_SET_ENCLAVE_SGX2(
+    1,     /* ProductID */
+    1,     /* SecurityVersion */
+    {0},   /* ExtendedProductID */
+    {0},   /* FamilyID */
+    true,  /* Debug */
+    true,  /* CapturePFGPExceptions */
+    true,  /* EnableKSS */
+    1024,  /* NumHeapPages */
+    1024,  /* NumStackPages */
+    1);    /* NumTCS */
 ```
 
 You can also specify the enclave properties in code using the

diff --git a/include/openenclave/bits/sgx/sgxproperties.h b/include/openenclave/bits/sgx/sgxproperties.h
@@ -202,46 +202,6 @@ typedef struct _oe_sgx_enclave_properties
     STACK_PAGE_COUNT,                                                     \
     TCS_COUNT)
 
-/**
- * Defines the SGX properties for an enclave with KSS properties
- *
- * Maps to _OE_SET_ENCLAVE_SGX_IMPL and set the KSS attribute bit
- * @param[in] PRODUCT_ID ISV assigned Product ID (ISVPRODID) to use in the
- * enclave signature
- * @param[in] SECURITY_VERSION ISV assigned Security Version number (ISVSVN)
- * to use in the enclave signature
- * @param[in] EXTENDED_PRODUCT_ID ISV assigned Extended Product ID (ISVEXTPRODID)
- *  to use in the enclave signature
- * @param[in] FAMILY_ID ISV assigned Product Family ID (ISVFAMILYID)
- * to use in the enclave signature
- * @param[in] ALLOW_DEBUG If true, allows the enclave to be created with
- * OE_ENCLAVE_FLAG_DEBUG and debugged at runtime
- * @param[in] HEAP_PAGE_COUNT Number of heap pages to allocate in the enclave
- * @param[in] STACK_PAGE_COUNT Number of stack pages per thread to reserve in
- * the enclave
- * @param[in] TCS_COUNT Number of concurrent threads in an enclave to support
- */
- #define OE_SET_ENCLAVE_SGX_KSS(                                          \
-    PRODUCT_ID,                                                           \
-    SECURITY_VERSION,                                                     \
-    EXTENDED_PRODUCT_ID,                                                  \
-    FAMILY_ID,                                                            \
-    ALLOW_DEBUG,                                                          \
-    HEAP_PAGE_COUNT,                                                      \
-    STACK_PAGE_COUNT,                                                     \
-    TCS_COUNT)                                                            \
- _OE_SET_ENCLAVE_SGX_IMPL(                                                \
-    PRODUCT_ID,                                                           \
-    SECURITY_VERSION,                                                     \
-    EXTENDED_PRODUCT_ID,                                                  \
-    FAMILY_ID,                                                            \
-    ALLOW_DEBUG,                                                          \
-    true,                                                                 \
-    0,                                                                    \
-    HEAP_PAGE_COUNT,                                                      \
-    STACK_PAGE_COUNT,                                                     \
-    TCS_COUNT)
-
 /**
  * Defines the SGX2 properties for an enclave
  *

diff --git a/tests/config_id/CMakeLists.txt b/tests/config_id/CMakeLists.txt
@@ -7,4 +7,13 @@ if (BUILD_ENCLAVES)
   add_subdirectory(enc)
 endif ()
 
-add_enclave_test(tests/config_id config_id_host config_id_enc)
+add_enclave_test(tests/config_id_unsigned_no_kss_flags config_id_host
+                 config_id_enc_unsigned no_kss_flags)
+add_enclave_test(tests/config_id_unsigned_with_kss_flags config_id_host
+                 config_id_kss_enc_unsigned with_kss_flags)
+add_enclave_test(tests/config_id_signed_no_kss_flags config_id_host
+                 config_id_enc_signed no_kss_flags)
+add_enclave_test(tests/config_id_kss_signed_with_kss_flags config_id_host
+                 config_id_kss_enc_signed with_kss_flags)
+add_enclave_test(tests/config_id_signed_no_kss_conf_flags config_id_host
+                 config_id_no_kss_conf_enc_signed no_kss_flags)
diff --git a/tests/config_id/common.h b/tests/config_id/common.h
@@ -0,0 +1,12 @@
+// Copyright (c) Open Enclave SDK contributors.
+// Licensed under the MIT License.
+
+#include "openenclave/bits/types.h"
+
+const uint8_t original_config_id[64] = {
+    0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11,  12,  13,  143, 153,
+    0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11,  12,  134, 14,  154,
+    0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11,  125, 13,  14,  155,
+    0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 116, 12,  13,  14,  156};
+
+const uint16_t original_config_svn = 65535;
diff --git a/tests/config_id/config_id.edl b/tests/config_id/config_id.edl
@@ -2,9 +2,10 @@
 // Licensed under the MIT License.
 
 enclave {
-    from "openenclave/edl/logging.edl" import oe_write_ocall;
+    from "openenclave/edl/logging.edl" import *;
     from "openenclave/edl/sgx/platform.edl" import *;
     trusted {
-        public int enclave_test_config_id();
+        public oe_result_t enclave_test_config_id_non_kss();
+        public oe_result_t enclave_test_config_id();
     };
 };
diff --git a/tests/config_id/enc/CMakeLists.txt b/tests/config_id/enc/CMakeLists.txt
@@ -10,14 +10,57 @@ add_custom_command(
     edger8r --trusted ${EDL_FILE} --search-path ${PROJECT_SOURCE_DIR}/include
     ${DEFINE_OE_SGX} --search-path ${CMAKE_CURRENT_SOURCE_DIR})
 
+add_enclave(TARGET config_id_kss_enc_unsigned SOURCES enc.c props_kss.c
+            ${CMAKE_CURRENT_BINARY_DIR}/config_id_t.c)
+
+enclave_include_directories(
+  config_id_kss_enc_unsigned PRIVATE ${CMAKE_CURRENT_BINARY_DIR}
+  ${CMAKE_CURRENT_SOURCE_DIR})
+
+add_enclave(TARGET config_id_enc_unsigned SOURCES enc.c props.c
+            ${CMAKE_CURRENT_BINARY_DIR}/config_id_t.c)
+
+enclave_include_directories(
+  config_id_enc_unsigned PRIVATE ${CMAKE_CURRENT_BINARY_DIR}
+  ${CMAKE_CURRENT_SOURCE_DIR})
+
+add_enclave(
+  TARGET
+  config_id_kss_enc
+  CONFIG
+  sign_kss.conf
+  SOURCES
+  enc.c
+  props_kss.c
+  ${CMAKE_CURRENT_BINARY_DIR}/config_id_t.c)
+
+enclave_include_directories(
+  config_id_kss_enc PRIVATE ${CMAKE_CURRENT_BINARY_DIR}
+  ${CMAKE_CURRENT_SOURCE_DIR})
+
 add_enclave(
   TARGET
   config_id_enc
-  UUID
-  25419627-14f6-4625-9329-cf5f10a57fea
+  CONFIG
+  sign.conf
   SOURCES
-  enc.cpp
+  enc.c
+  props.c
   ${CMAKE_CURRENT_BINARY_DIR}/config_id_t.c)
 
 enclave_include_directories(config_id_enc PRIVATE ${CMAKE_CURRENT_BINARY_DIR}
                             ${CMAKE_CURRENT_SOURCE_DIR})
+
+add_enclave(
+  TARGET
+  config_id_no_kss_conf_enc
+  CONFIG
+  sign.conf
+  SOURCES
+  enc.c
+  props_kss.c
+  ${CMAKE_CURRENT_BINARY_DIR}/config_id_t.c)
+
+enclave_include_directories(
+  config_id_no_kss_conf_enc PRIVATE ${CMAKE_CURRENT_BINARY_DIR}
+  ${CMAKE_CURRENT_SOURCE_DIR})
diff --git a/tests/config_id/enc/enc.c b/tests/config_id/enc/enc.c
@@ -0,0 +1,116 @@
+// Copyright (c) Open Enclave SDK contributors.
+// Licensed under the MIT License.
+
+#include <openenclave/attestation/attester.h>
+#include <openenclave/attestation/sgx/evidence.h>
+#include <openenclave/attestation/verifier.h>
+#include <openenclave/internal/hexdump.h>
+#include <openenclave/internal/raise.h>
+#include <openenclave/internal/report.h>
+#include <openenclave/internal/tests.h>
+#include <stdio.h>
+#include <string.h>
+#include "../common.h"
+#include "config_id_t.h"
+
+static const oe_uuid_t _ecdsa_uuid = {OE_FORMAT_UUID_SGX_ECDSA};
+
+static void* _find_claim(
+    const oe_claim_t* claims,
+    size_t claims_size,
+    const char* name)
+{
+    for (size_t i = 0; i < claims_size; i++)
+    {
+        // Claim names are case sensitive.
+        if (strcmp(claims[i].name, name) == 0)
+            return claims[i].value;
+    }
+    return NULL;
+}
+
+oe_result_t enclave_test_config_id()
+{
+    OE_TRACE_INFO("enclave_config_id_test_kss_properties invoked\n");
+
+    oe_result_t result = OE_OK;
+    uint8_t* evidence = NULL;
+    size_t evidence_size = 0;
+    oe_claim_t* claims = NULL;
+    size_t claims_length = 0;
+
+    OE_TRACE_ERROR("========== Getting evidence with KSS feature\n");
+
+    OE_CHECK(oe_attester_initialize());
+
+    oe_uuid_t selected_format;
+    oe_attester_select_format(&_ecdsa_uuid, 1, &selected_format);
+
+    OE_CHECK(oe_get_evidence(
+        &selected_format,
+        OE_EVIDENCE_FLAGS_EMBED_FORMAT_ID,
+        NULL,
+        0,
+        NULL,
+        0,
+        &evidence,
+        &evidence_size,
+        NULL,
+        0));
+
+    OE_CHECK(oe_verifier_initialize());
+
+    OE_CHECK(oe_verify_evidence(
+        NULL,
+        evidence,
+        evidence_size,
+        NULL,
+        0,
+        NULL,
+        0,
+        &claims,
+        &claims_length));
+
+    uint8_t* config_id;
+    uint16_t* config_svn;
+
+    config_id =
+        (uint8_t*)_find_claim(claims, claims_length, OE_CLAIM_SGX_CONFIG_ID);
+
+    config_svn =
+        (uint16_t*)_find_claim(claims, claims_length, OE_CLAIM_SGX_CONFIG_SVN);
+
+    if (memcmp(config_id, original_config_id, sizeof(original_config_id)))
+    {
+        OE_TRACE_INFO("\noriginal_config_id :\n0x");
+        oe_hex_dump(original_config_id, OE_COUNTOF(original_config_id));
+        OE_TRACE_INFO("\nparsed config_id :\n0x");
+        oe_hex_dump(config_id, OE_COUNTOF(original_config_id));
+        OE_RAISE_MSG(
+            OE_REPORT_PARSE_ERROR,
+            "========== Read wrong config id from the report");
+    }
+
+    if (memcmp(config_svn, &original_config_svn, sizeof(original_config_svn)))
+    {
+        OE_RAISE_MSG(
+            OE_REPORT_PARSE_ERROR,
+            "========== Read wrong config svn(0x%x) from the report, "
+            "expected(0x%x)",
+            *config_svn,
+            original_config_svn);
+    }
+
+done:
+    oe_free_evidence(evidence);
+    oe_free_claims(claims, claims_length);
+    oe_attester_shutdown();
+    oe_verifier_shutdown();
+    return result;
+}
+
+oe_result_t enclave_test_config_id_non_kss()
+{
+    OE_TRACE_INFO("enclave function invoked on non kss image\n");
+    return OE_OK;
+}
diff --git a/tests/config_id/enc/enc.cpp b/tests/config_id/enc/enc.cpp
diff --git a/tests/config_id/enc/props.c b/tests/config_id/enc/props.c
@@ -0,0 +1,12 @@
+// Copyright (c) Open Enclave SDK contributors.
+// Licensed under the MIT License.
+
+#include <openenclave/enclave.h>
+
+OE_SET_ENCLAVE_SGX(
+    1,    /* ProductID */
+    1,    /* SecurityVersion */
+    true, /* Debug */
+    1024, /* NumHeapPages */
+    64,   /* NumStackPages */
+    1);   /* NumTCS */
diff --git a/tests/config_id/enc/props_kss.c b/tests/config_id/enc/props_kss.c
@@ -0,0 +1,16 @@
+// Copyright (c) Open Enclave SDK contributors.
+// Licensed under the MIT License.
+
+#include <openenclave/enclave.h>
+
+OE_SET_ENCLAVE_SGX2(
+    1,     /* ProductID */
+    1,     /* SecurityVersion */
+    {0},   /* ExtendedProductID */
+    {0},   /* FamilyID */
+    true,  /* Debug */
+    false, /* CapturePFGPExceptions */
+    true,  /* RequireKSS */
+    1024,  /* NumHeapPages */
+    1024,  /* NumStackPages */
+    1);    /* NumTCS */
diff --git a/tests/config_id/enc/sign.conf b/tests/config_id/enc/sign.conf
@@ -0,0 +1,10 @@
+# Copyright (c) Open Enclave SDK contributors.
+# Licensed under the MIT License.
+
+# Enclave settings:
+Debug=1
+NumHeapPages=1024
+NumStackPages=1024
+NumTCS=2
+ProductID=1
+SecurityVersion=1
diff --git a/tests/config_id/enc/sign_kss.conf b/tests/config_id/enc/sign_kss.conf
@@ -0,0 +1,11 @@
+# Copyright (c) Open Enclave SDK contributors.
+# Licensed under the MIT License.
+
+# Enclave settings:
+Debug=1
+NumHeapPages=1024
+NumStackPages=1024
+NumTCS=2
+ProductID=1
+SecurityVersion=1
+EnableKSS=1