[Pal/Linux-SGX] Correctly set masks in SIGSTRUCT

Previously, SIGSTRUCT.ATTRIBUTEMASK (which contains the masks for
enclave attributes such as DEBUG and for XFRM/XCR0 bits) and
SIGSTRUCT.MISCMASK (which contains the mask for Extended SSA frame
features) were incorrectly set to mirror SIGSTRUCT.ATTRIBUTES and
SIGSTRUCT.MISCSELECT.

This commit replaces this incorrect logic with hard-coded values for
SIGSTRUCT.ATTRIBUTEMASK and SIGSTRUCT.MISCMASK. In particular, all bits
in the masks are set to one (meaning "EINIT checks each attribute"). The
only exceptions currently are not-security-critical AVX and AVX512 bits
-- they are not reflected in the mask, and they are set in
SIGSTRUCT.ATTRIBUTES if the system supports them during generation of
the `.token` file.

Signed-off-by: Dmitrii Kuvaiskii <dmitrii.kuvaiskii@intel.com>

Pal/src/host/Linux-SGX/generated-offsets.c

@@ -23,6 +23,12 @@ __attribute__((__used__)) static void dummy(void) {
     DEFINE(SGX_MISCSELECT_EXINFO, SGX_MISCSELECT_EXINFO);
     DEFINE(SE_KEY_SIZE, SE_KEY_SIZE);
 
+    DEFINE(SGX_FLAGS_MASK_CONST_HI, SGX_FLAGS_MASK_CONST_HI);
+    DEFINE(SGX_FLAGS_MASK_CONST_LO, SGX_FLAGS_MASK_CONST_LO);
+    DEFINE(SGX_XFRM_MASK_CONST_HI, SGX_XFRM_MASK_CONST_HI);
+    DEFINE(SGX_XFRM_MASK_CONST_LO, SGX_XFRM_MASK_CONST_LO);
+    DEFINE(SGX_MISCSELECT_MASK_CONST, SGX_MISCSELECT_MASK_CONST);
+
     /* defines in pal-arch.h */
     DEFINE(STACK_PROTECTOR_CANARY_DEFAULT, STACK_PROTECTOR_CANARY_DEFAULT);
 

Pal/src/host/Linux-SGX/sgx_arch.h

@@ -66,16 +66,69 @@ typedef uint8_t sgx_isvfamily_id_t[SGX_ISV_FAMILY_ID_SIZE];
 #define SGX_FLAGS_PROVISION_KEY 0x10ULL
 #define SGX_FLAGS_LICENSE_KEY   0x20ULL
 
+/* EINIT must verify *all* SECS.ATTRIBUTES[63..0] bits (FLAGS bits) against
+ * SIGSTRUCT.ATTRIBUTES[63..0].
+ *
+ * Notes:
+ *   - Two instances of the same enclave with even one-bit difference in attributes-flags (e.g., one
+ *     with DEBUG == 0 and another with DEBUG == 1) will have two different SIGSTRUCTs.
+ *   - Important consequence of the above: a debug version of the enclave requires a different
+ *     SIGSTRUCT file than a production version of the enclave.
+ *   - CET and KSS bits are not yet reflected in Graphene, i.e., Graphene doesn't support Intel CET
+ *     technology and Key Separation and Sharing feature.
+ */
+/* FIXME: This FLAGS mask should be represented as one 64-bit constant (instead of the current two
+ *        32-bit constants), but our constant-generation build scripts use assembly macros (see
+ *        `generated-offsets-build.h`) that represent 64-bit constants as negative integers. Same
+ *        issue applies to the XFRM mask below. */
+#define SGX_FLAGS_MASK_CONST_HI 0xffffffffUL
+#define SGX_FLAGS_MASK_CONST_LO 0xffffffffUL
+
+/* Note that XFRM follows XCR0 register format. Also note that XFRM bits 0 (x87 support) and 1 (SSE
+ * support) must be always set in Intel SGX (otherwise EINIT instruction fails). This is why these
+ * two bits are called "legacy" here.
+ *
+ * From Intel SDM, Section 41.7.2.1: "Because bits 0 and 1 of XFRM must always be set, the use of
+ * Intel SGX requires that SSE be enabled (CR4.OSFXSR = 1)." */
 #define SGX_XFRM_LEGACY   0x03ULL
-#define SGX_XFRM_AVX      0x06ULL
+#define SGX_XFRM_AVX      0x04ULL
 #define SGX_XFRM_MPX      0x18ULL
-#define SGX_XFRM_AVX512   0xe6ULL
+#define SGX_XFRM_AVX512   0xe4ULL
 #define SGX_XFRM_PKRU     0x200ULL
 #define SGX_XFRM_RESERVED (~(SGX_XFRM_LEGACY | SGX_XFRM_AVX | SGX_XFRM_MPX | SGX_XFRM_AVX512 | \
                              SGX_XFRM_PKRU))
 
+/* EINIT must verify most of the SECS.ATTRIBUTES[127..64] bits (XFRM/XCR0 bits) against
+ * SIGSTRUCT.ATTRIBUTES[127..64].
+ *
+ * Notes:
+ *   - Verified bits include: bit 0 + bit 1 (X87 + SSE, always enabled in SGX), bit 3 + bit 4
+ *     (BNDREG + BNDCSR, enables Intel MPX), bit 9 (PKRU, enables Intel MPK), and all reserved bits.
+ *   - Not-verified bits include: bit 2 (AVX), bit 5 + bit 6 + bit 7 (AVX-512). These bits are
+ *     considered not security-critical.
+ *   - Two instances of the same enclave with difference in X87, SSE, MPX, MPK bits (e.g., one
+ *     with PKRU == 0 and another with PKRU == 1) will have two different SIGSTRUCTs. However,
+ *     difference in AVX/AVX-512 bits does not lead to different SIGSTRUCTs.
+ *   - Important consequence of the above: the same enclave (with the same SIGSTRUCT) may run on
+ *     machines with and without AVX/AVX-512, but e.g. a PKRU-requiring enclave may run only on
+ *     machine with PKRU.
+ *   - CET bits are not yet reflected in Graphene, i.e., Graphene doesn't support Intel CET.
+  */
+#define SGX_XFRM_MASK_CONST_HI 0xffffffffUL
+#define SGX_XFRM_MASK_CONST_LO 0xffffff1bUL
+
 #define SGX_MISCSELECT_EXINFO 0x01UL
 
+/* EINIT must verify *all* SECS.MISCSELECT bits against SIGSTRUCT.MISCSELECT.
+ *
+ * Notes:
+ *   - Two instances of the same enclave (one with EXINFO == 0 and another with EXINFO == 1) will
+ *     have two different SIGSTRUCTs.
+ *   - CPINFO bit is not yet reflected in Graphene, i.e., information about control protection
+ *     exception (#CP) will not be reported on AEX.
+  */
+#define SGX_MISCSELECT_MASK_CONST 0xffffffffUL
+
 typedef struct {
     uint64_t          size;
     uint64_t          base;

python/graphenelibos/sgx_get_token.py

@@ -34,15 +34,13 @@ def set_optional_sgx_features(attr):
     xfrms = int.from_bytes(attr['xfrms'], byteorder='little')
     xfrmmask = int.from_bytes(attr['xfrm_mask'], byteorder='little')
 
-    new_xfrms = 0
+    new_xfrms = xfrms
     for (bits, feature) in optional_sgx_features.items():
-        # Check if SIGSTRUCT allows enabling an optional CPU feature.
-        # If all the xfrm bits for a feature, after applying xfrmmask, are set in xfrms,
-        # we can set the remaining bits if the feature is available.
-        # If the xfrmmask includes all the required xfrm bits, then these bits cannot be
-        # changed in xfrm (need to stay the same as signed).
-        if xfrms & (bits & xfrmmask) == (bits & xfrmmask) and feature in cpu_features:
-            new_xfrms |= xfrms | bits
+        # check if SIGSTRUCT.ATTRIBUTEMASK.XFRM doesn't care whether an optional CPU feature is
+        # enabled or not (XFRM mask should completely unset these bits); set these CPU features as
+        # enabled if so and if the current system supports these features (for performance)
+        if bits & xfrmmask == 0 and feature in cpu_features:
+            new_xfrms |= bits
 
     attr['xfrms'] = new_xfrms.to_bytes(length=8, byteorder='little')
 
@@ -143,14 +141,14 @@ def create_dummy_token(attr):
     fields['isvprodidle'] = (208, '<H')
     fields['isvsvnle'] = (210, '<H')
     fields['reserved4'] = (212, '24B')
-    fields['misc_mask'] = (236, '<I')
+    fields['misc_select'] = (236, '<L')
     fields['flagmask'] = (240, '<Q') # attrmask
     fields['xfrmmask'] = (248, '<Q') # attrmask
     fields['keyid'] = (256, '32B')
     fields['mac'] = (288, '16B')
 
     # fields read by create_enclave() in sgx_framework.c
-    actual_fields = ['flags', 'xfrms', 'misc_mask']
+    actual_fields = ['flags', 'xfrms', 'misc_select']
 
     for key in actual_fields:
         field = fields[key]
@@ -186,6 +184,8 @@ def main(args=None):
     print(f'    isv_svn:     {attr["isv_svn"]}')
     print(f'    attr.flags:  {int.from_bytes(attr["flags"], byteorder="big"):016x}')
     print(f'    attr.xfrm:   {int.from_bytes(attr["xfrms"], byteorder="big"):016x}')
+    print(f'    mask.flags:  {int.from_bytes(attr["flag_mask"], byteorder="big"):016x}')
+    print(f'    mask.xfrm:   {int.from_bytes(attr["xfrm_mask"], byteorder="big"):016x}')
     print(f'    misc_select: {int.from_bytes(attr["misc_select"], byteorder="big"):08x}')
     print(f'    misc_mask:   {int.from_bytes(attr["misc_mask"], byteorder="big"):08x}')
     print(f'    modulus:     {attr["modulus"].hex()[:32]}...')

python/graphenelibos/sgx_sign.py

@@ -586,10 +586,11 @@ def generate_sigstruct(attr, args, mrenclave):
                     '<4L', 0x00000101, 0x00000060, 0x00000060, 0x00000001),
         'hw_version': (offs.SGX_ARCH_ENCLAVE_CSS_HW_VERSION, '<L', 0x00000000),
         'misc_select': (offs.SGX_ARCH_ENCLAVE_CSS_MISC_SELECT, '4s', attr['misc_select']),
-        'misc_mask': (offs.SGX_ARCH_ENCLAVE_CSS_MISC_MASK, '4s', attr['misc_select']),
+        'misc_mask': (offs.SGX_ARCH_ENCLAVE_CSS_MISC_MASK, '<L', offs.SGX_MISCSELECT_MASK_CONST),
         'attributes': (offs.SGX_ARCH_ENCLAVE_CSS_ATTRIBUTES, '8s8s', attr['flags'], attr['xfrms']),
-        'attribute_mask': (offs.SGX_ARCH_ENCLAVE_CSS_ATTRIBUTE_MASK,
-                           '8s8s', attr['flags'], attr['xfrms']),
+        'attribute_mask': (offs.SGX_ARCH_ENCLAVE_CSS_ATTRIBUTE_MASK, '<4L',
+                           offs.SGX_FLAGS_MASK_CONST_LO, offs.SGX_FLAGS_MASK_CONST_HI,
+                           offs.SGX_XFRM_MASK_CONST_LO, offs.SGX_XFRM_MASK_CONST_HI),
         'enclave_hash': (offs.SGX_ARCH_ENCLAVE_CSS_ENCLAVE_HASH, '32s', mrenclave),
         'isv_prod_id': (offs.SGX_ARCH_ENCLAVE_CSS_ISV_PROD_ID, '<H', attr['isv_prod_id']),
         'isv_svn': (offs.SGX_ARCH_ENCLAVE_CSS_ISV_SVN, '<H', attr['isv_svn']),