gramine icon indicating copy to clipboard operation
gramine copied to clipboard

[tools] Fix OID for RA-TLS

Open jiazhang0 opened this issue 1 year ago • 2 comments

The oid parameter of mbedtls_x509write_crt_set_extension() accepts an encoded OID octet string, rather than an ASN.1 encoded OID object (type 6).

Without the fix, the SGX quote extension in the dump result shows a weird OID: 0.6.9.42.840.113741.1337.6: ..............r3..L..

The corrected OID should be: 1.2.840.113741.1337.6: ..............r3..L..

Signed-off-by: Jia Zhang [email protected]

Description of the changes

The OID should be canonical and defined correctly. Currently the Evidence Extension format has a wrong OID due to misusing the OID value.

How to test this PR?

cd CI-Examples/ra-tls-mbedtls
vim src/client.c

manually apply this patch:

diff --git a/CI-Examples/ra-tls-mbedtls/src/client.c b/CI-Examples/ra-tls-mbedtls/src/client.c
index 4d2f71ba..1655dad2 100644
--- a/CI-Examples/ra-tls-mbedtls/src/client.c
+++ b/CI-Examples/ra-tls-mbedtls/src/client.c
@@ -108,6 +108,10 @@ static int my_verify_measurements(const char* mrenclave, const char* mrsigner,
 static int my_verify_callback(void* data, mbedtls_x509_crt* crt, int depth, uint32_t* flags) {
     (void)data;
 
+    FILE *fp = fopen("/tmp/dump.der", "wb");
+    fwrite(crt->raw.p, crt->raw.len, 1, fp);
+    fclose(fp);
+
     if (depth != 0) {
         /* the cert chain in RA-TLS consists of single self-signed cert, so we expect depth 0 */
         return MBEDTLS_ERR_X509_INVALID_FORMAT;

Then dump the certificate and show its content in text:

make app dcap RA_TYPE=dcap
gramine-sgx ./server &
RA_TLS_ALLOW_DEBUG_ENCLAVE_INSECURE=1 \
RA_TLS_ALLOW_OUTDATED_TCB_INSECURE=1 \
./client dcap
openssl x509 -in /tmp/dump.der -inform der -text | grep 'X509v3 extensions' -A9

Results is:

        X509v3 extensions:
            X509v3 Basic Constraints: 
                CA:FALSE
            X509v3 Subject Key Identifier: 
                65:23:59:F3:28:2A:5A:DD:79:4E:18:85:48:E6:97:38:73:E6:F7:D6
            X509v3 Authority Key Identifier: 
                keyid:65:23:59:F3:28:2A:5A:DD:79:4E:18:85:48:E6:97:38:73:E6:F7:D6

            0.6.9.42.840.113741.1337.6: 
                ..............r3..L..

After applying this patch, the result would be:

        X509v3 extensions:
            X509v3 Basic Constraints: 
                CA:FALSE
            X509v3 Subject Key Identifier: 
                3A:09:B0:A2:FA:6C:A8:55:BF:AA:0F:67:74:E9:18:08:E8:B5:2A:E5
            X509v3 Authority Key Identifier: 
                keyid:3A:09:B0:A2:FA:6C:A8:55:BF:AA:0F:67:74:E9:18:08:E8:B5:2A:E5

            1.2.840.113741.1337.6: 
                ..............r3..L..

This change is Reviewable

jiazhang0 avatar Jul 30 '22 06:07 jiazhang0

@kailun-qin Nice follow up.

To check out the certificate, modify https://github.com/gramineproject/gramine/blob/master/CI-Examples/ra-tls-mbedtls/src/client.c#L121 and save the content of crt->raw.p to a file (e.g, dump.der) and then run openssl x509 -in dump.der -inform der -text to show the text.

We can check the validation of a OID value in www.oid-info.com. The expected OID prefix of sgx quote embedded in x509 should be 1.2.840.113741.

jiazhang0 avatar Aug 02 '22 00:08 jiazhang0

Reviewable status: 0 of 2 files reviewed, 3 unresolved discussions, not enough approvals from maintainers (3 more required), not enough approvals from different teams (2 more required, approved so far: ) (waiting on @jiazhang0)

a discussion (no related file):

Could you give some links/references about this? How do you know what format mbedTLS expects the OID to be in? What's wrong with the current OID?

Currently, mbedtls_x509write_crt_set_extension() in our case sets the quote octet string to the specified OID in a ASN.1 Tag(0x06)Length(0x09)Value(0x2A, ...) aka TLV format. Then underneath, mbedTLS simply sets this entire TLV into the oid.p: https://github.com/Mbed-TLS/mbedtls/blob/919ff15ecf7b68e7d8ab4d470f3139b9040498e4/library/asn1write.c#L442.

However, looking at the oid field itself: https://github.com/Mbed-TLS/mbedtls/blob/919ff15ecf7b68e7d8ab4d470f3139b9040498e4/include/mbedtls/asn1.h#L152-L158) - it's actually a mbedtls_asn1_buf which's already in a ASN.1 TLV format. Hence, setting the entire TLV to oid.p does not look reasonable. Instead, we should only set the V part to it (which is proposed by this PR).

Also, we can see mbedTLS internally manages the TL part of OID: https://github.com/Mbed-TLS/mbedtls/blob/919ff15ecf7b68e7d8ab4d470f3139b9040498e4/library/x509_create.c#L336-L339. As the API int mbedtls_x509write_crt_set_extension( mbedtls_x509write_cert *ctx, const char *oid, size_t oid_len, int critical, const unsigned char *val, size_t val_len ) implies, mbedTLS already knows that it operates on an oid of the extension, it's natural that it can manage it itself. Though from document perspective, I didn't find a good reference... (as always)

Also, doesn't this PR break backward-compatibility? How can Gramine with this PR interoperate with e.g. Gramine v1.2?

Good question. Looks like this breaks the backward-compatiblity of Gramine.

I'm not sure what the backward-compatiblity of Gramine exactly means. If we just talk about the backward-compatiblity of Gramine ra-tls, I think it is not a problem.

The OID detection occurs in ra-tls client side, which searches the OID to detect whether the server side sends a certificate with Evidence Extension.

In old implementation, the ASN.1 encoding for the Evidence Extension OID is 13-byte value 0x06, 0x0B, 0x06, 0x09, 0x2A, 0x86, 0x48, 0x86, 0xF8, 0x4D, 0x8A, 0x39, 0x06, and client side uses the 11-byte pattern 0x06, 0x09, 0x2A, 0x86, 0x48, 0x86, 0xF8, 0x4D, 0x8A, 0x39, 0x06 in memmem() (see https://github.com/gramineproject/gramine/blob/master/tools/sgx/ra-tls/ra_tls_verify_common.c#L114) to find out it.

In new implementation, the ASN.1 encoding for the Evidence Extension OID becomes 11-byte value 0x06, 0x09, 0x2A, 0x86, 0x48, 0x86, 0xF8, 0x4D, 0x8A, 0x39, 0x06. The old client side using 11-byte pattern can still find out it. In another case, the old server sides uses the 13-byte long value for OID encoding, and the new client can still find out it with the 9-byte pattern "0x2A, 0x86, 0x48, 0x86, 0xF8, 0x4D, 0x8A, 0x39, 0x06".

So I think the backward-compatiblity is still kept.

jiazhang0 avatar Aug 02 '22 01:08 jiazhang0

I created #1039 for new standard Interoperable RA-TLS. I am closing this PR now, since the current RA-TLS implementation is legacy anyway.

dimakuv avatar Nov 21 '22 11:11 dimakuv