From 6a26dab73a69071082ca77708c226530ea7008e7 Mon Sep 17 00:00:00 2001 From: Chris Conlon Date: Thu, 19 May 2022 16:46:41 -0600 Subject: [PATCH] X.509 cert validity for CertFromX509() and EncodeCert() shouldn't be protected by WOLFSSL_ALT_NAMES --- src/x509.c | 4 +-- tests/api.c | 56 ++++++++++++++++++---------------- wolfcrypt/src/asn.c | 8 ++--- wolfssl/wolfcrypt/asn_public.h | 2 +- 4 files changed, 34 insertions(+), 36 deletions(-) diff --git a/src/x509.c b/src/x509.c index a117a4308..64e218b3f 100644 --- a/src/x509.c +++ b/src/x509.c @@ -8171,7 +8171,6 @@ WOLF_STACK_OF(WOLFSSL_X509)* wolfSSL_X509_chain_up_ref( } #endif /* WOLFSSL_CERT_REQ */ -#ifdef WOLFSSL_ALT_NAMES /* converts WOLFSSL_AN1_TIME to Cert form, returns positive size on * success */ static int CertDateFromX509(byte* out, int outSz, WOLFSSL_ASN1_TIME* t) @@ -8189,7 +8188,6 @@ WOLF_STACK_OF(WOLFSSL_X509)* wolfSSL_X509_chain_up_ref( } return t->length + sz; } -#endif /* WOLFSSL_ALT_NAMES */ /* convert a WOLFSSL_X509 to a Cert structure for writing out */ static int CertFromX509(Cert* cert, WOLFSSL_X509* x509) @@ -8209,7 +8207,6 @@ WOLF_STACK_OF(WOLFSSL_X509)* wolfSSL_X509_chain_up_ref( cert->version = (int)wolfSSL_X509_get_version(x509); - #ifdef WOLFSSL_ALT_NAMES if (x509->notBefore.length > 0) { cert->beforeDateSz = CertDateFromX509(cert->beforeDate, CTC_DATE_SIZE, &x509->notBefore); @@ -8234,6 +8231,7 @@ WOLF_STACK_OF(WOLFSSL_X509)* wolfSSL_X509_chain_up_ref( cert->afterDateSz = 0; } + #ifdef WOLFSSL_ALT_NAMES cert->altNamesSz = FlattenAltNames(cert->altNames, sizeof(cert->altNames), x509->altNames); #endif /* WOLFSSL_ALT_NAMES */ diff --git a/tests/api.c b/tests/api.c index d1393fd01..86f010b1c 100644 --- a/tests/api.c +++ b/tests/api.c @@ -38533,7 +38533,12 @@ static void test_wolfSSL_PEM_write_bio_X509(void) BIO* input; BIO* output; - X509* x509 = NULL; + X509* x509a = NULL; + X509* x509b = NULL; + ASN1_TIME* notBeforeA = NULL; + ASN1_TIME* notAfterA = NULL; + ASN1_TIME* notBeforeB = NULL; + ASN1_TIME* notAfterB = NULL; int expectedLen; printf(testingFmt, "wolfSSL_PEM_write_bio_X509()"); @@ -38542,51 +38547,50 @@ static void test_wolfSSL_PEM_write_bio_X509(void) "certs/test/cert-ext-multiple.pem", "rb")); AssertIntEQ(wolfSSL_BIO_get_len(input), 2000); + /* read PEM into X509 struct, get notBefore / notAfter to verify against */ + AssertNotNull(PEM_read_bio_X509(input, &x509a, NULL, NULL)); + AssertNotNull(notBeforeA = X509_get_notBefore(x509a)); + AssertNotNull(notAfterA = X509_get_notAfter(x509a)); + + /* write X509 back to PEM BIO */ AssertNotNull(output = BIO_new(wolfSSL_BIO_s_mem())); - - AssertNotNull(PEM_read_bio_X509(input, &x509, NULL, NULL)); - - AssertIntEQ(PEM_write_bio_X509(output, x509), WOLFSSL_SUCCESS); - -#ifdef WOLFSSL_ALT_NAMES - /* Here we copy the validity struct from the original */ + AssertIntEQ(PEM_write_bio_X509(output, x509a), WOLFSSL_SUCCESS); + /* compare length against expected */ expectedLen = 2000; -#else - /* Only difference is that we generate the validity in generalized - * time. Generating UTCTime vs Generalized time should be fixed in - * the future */ - expectedLen = 2004; -#endif AssertIntEQ(wolfSSL_BIO_get_len(output), expectedLen); + /* read exported X509 PEM back into struct, sanity check on export, + * make sure notBefore/notAfter are the same. */ + AssertNotNull(PEM_read_bio_X509(output, &x509b, NULL, NULL)); + AssertNotNull(notBeforeB = X509_get_notBefore(x509b)); + AssertNotNull(notAfterB = X509_get_notAfter(x509b)); + AssertIntEQ(ASN1_TIME_compare(notBeforeA, notBeforeB), 0); + AssertIntEQ(ASN1_TIME_compare(notAfterA, notAfterB), 0); + X509_free(x509b); + /* Reset output buffer */ BIO_free(output); AssertNotNull(output = BIO_new(wolfSSL_BIO_s_mem())); /* Test forcing the AKID to be generated just from KeyIdentifier */ - if (x509->authKeyIdSrc != NULL) { - XMEMMOVE(x509->authKeyIdSrc, x509->authKeyId, x509->authKeyIdSz); - x509->authKeyId = x509->authKeyIdSrc; - x509->authKeyIdSrc = NULL; - x509->authKeyIdSrcSz = 0; + if (x509a->authKeyIdSrc != NULL) { + XMEMMOVE(x509a->authKeyIdSrc, x509a->authKeyId, x509a->authKeyIdSz); + x509a->authKeyId = x509a->authKeyIdSrc; + x509a->authKeyIdSrc = NULL; + x509a->authKeyIdSrcSz = 0; } - AssertIntEQ(PEM_write_bio_X509(output, x509), WOLFSSL_SUCCESS); + AssertIntEQ(PEM_write_bio_X509(output, x509a), WOLFSSL_SUCCESS); /* Check that we generate a smaller output since the AKID will * only contain the KeyIdentifier without any additional * information */ -#ifdef WOLFSSL_ALT_NAMES /* Here we copy the validity struct from the original */ expectedLen = 1688; -#else - /* UTCTime vs Generalized time */ - expectedLen = 1692; -#endif AssertIntEQ(wolfSSL_BIO_get_len(output), expectedLen); - X509_free(x509); + X509_free(x509a); BIO_free(input); BIO_free(output); diff --git a/wolfcrypt/src/asn.c b/wolfcrypt/src/asn.c index e174d9501..e05ead251 100644 --- a/wolfcrypt/src/asn.c +++ b/wolfcrypt/src/asn.c @@ -21772,7 +21772,6 @@ static void SetTime(struct tm* date, byte* output) } #endif -#ifdef WOLFSSL_ALT_NAMES #ifndef WOLFSSL_ASN_TEMPLATE /* Copy Dates from cert, return bytes written */ @@ -21793,7 +21792,6 @@ static int CopyValidity(byte* output, Cert* cert) } #endif /* !WOLFSSL_ASN_TEMPLATE */ -#endif /* Simple name OID size. */ @@ -24017,16 +24015,14 @@ static int EncodeCert(Cert* cert, DerCert* der, RsaKey* rsaKey, ecc_key* eccKey, return PUBLIC_KEY_E; der->validitySz = 0; -#ifdef WOLFSSL_ALT_NAMES - /* date validity copy ? */ + /* copy date validity if already set in cert struct */ if (cert->beforeDateSz && cert->afterDateSz) { der->validitySz = CopyValidity(der->validity, cert); if (der->validitySz <= 0) return DATE_E; } -#endif - /* date validity */ + /* set date validity using daysValid if not set already */ if (der->validitySz == 0) { der->validitySz = SetValidity(der->validity, cert->daysValid); if (der->validitySz <= 0) diff --git a/wolfssl/wolfcrypt/asn_public.h b/wolfssl/wolfcrypt/asn_public.h index 58ddf6476..fcfff28d6 100644 --- a/wolfssl/wolfcrypt/asn_public.h +++ b/wolfssl/wolfcrypt/asn_public.h @@ -390,11 +390,11 @@ typedef struct Cert { #ifdef WOLFSSL_ALT_NAMES byte altNames[CTC_MAX_ALT_SIZE]; /* altNames copy */ int altNamesSz; /* altNames size in bytes */ +#endif byte beforeDate[CTC_DATE_SIZE]; /* before date copy */ int beforeDateSz; /* size of copy */ byte afterDate[CTC_DATE_SIZE]; /* after date copy */ int afterDateSz; /* size of copy */ -#endif #ifdef WOLFSSL_CERT_EXT byte skid[CTC_MAX_SKID_SIZE]; /* Subject Key Identifier */ int skidSz; /* SKID size in bytes */