From 2cd42c9e1678a96fa998fa6a0d857807c783ea26 Mon Sep 17 00:00:00 2001 From: jackctj117 Date: Fri, 24 Apr 2026 11:27:52 -0600 Subject: [PATCH] Skoll review --- certs/test-serial0/generate_certs.sh | 29 ++++- certs/test-serial0/include.am | 3 +- certs/test-serial0/intermediate_serial0.pem | 21 ++++ tests/api.c | 118 ++++++++++---------- tests/api/test_asn.c | 19 +++- wolfcrypt/src/asn.c | 39 +++++-- 6 files changed, 151 insertions(+), 78 deletions(-) create mode 100644 certs/test-serial0/intermediate_serial0.pem diff --git a/certs/test-serial0/generate_certs.sh b/certs/test-serial0/generate_certs.sh index 047b21703c..76df6f3f7f 100755 --- a/certs/test-serial0/generate_certs.sh +++ b/certs/test-serial0/generate_certs.sh @@ -11,6 +11,8 @@ # ee_serial0.pem - EE cert with serial 0 (rejected) # ee_normal.pem - Normal EE cert (serial 100) # selfsigned_nonca_serial0.pem - Self-signed non-CA, serial 0 +# intermediate_serial0.pem - Intermediate CA, serial 0 +# (CA:TRUE but issuer != subject) set -e @@ -23,7 +25,7 @@ echo "===================================================" # 1. Create Root CA with serial number 0 echo "" -echo "[1/4] Creating Root CA with serial number 0..." +echo "[1/5] Creating Root CA with serial number 0..." openssl req -x509 -newkey rsa:2048 -keyout root_serial0_key.pem -out root_serial0.pem \ -days 7300 -nodes -subj "/CN=Test Root CA Serial 0/O=wolfSSL Test/C=US" \ -set_serial 0 \ @@ -35,7 +37,7 @@ openssl x509 -in root_serial0.pem -noout -serial # 2. Create end-entity cert with serial 0 signed by root_serial0 echo "" -echo "[2/4] Creating end-entity certificate with serial number 0..." +echo "[2/5] Creating end-entity certificate with serial number 0..." openssl req -newkey rsa:2048 -keyout ee_serial0_key.tmp -out ee_serial0.csr.tmp -nodes \ -subj "/CN=End Entity Serial 0/O=wolfSSL Test/C=US" @@ -52,7 +54,7 @@ openssl x509 -in ee_serial0.pem -noout -serial # 3. Create normal end-entity cert signed by root CA with serial 0 echo "" -echo "[3/4] Creating normal end-entity certificate (signed by serial 0 root)..." +echo "[3/5] Creating normal end-entity certificate (signed by serial 0 root)..." openssl req -newkey rsa:2048 -keyout ee_normal_key.tmp -out ee_normal.csr.tmp -nodes \ -subj "/CN=End Entity Normal/O=wolfSSL Test/C=US" @@ -69,7 +71,7 @@ openssl x509 -in ee_normal.pem -noout -serial # 4. Create self-signed non-CA certificate with serial 0 echo "" -echo "[4/4] Creating self-signed non-CA certificate with serial number 0..." +echo "[4/5] Creating self-signed non-CA certificate with serial number 0..." openssl req -x509 -newkey rsa:2048 -keyout selfsigned_nonca_serial0_key.tmp \ -out selfsigned_nonca_serial0.pem -days 3650 -nodes \ -subj "/CN=Self-Signed Non-CA Serial 0/O=wolfSSL Test/C=US" \ @@ -82,6 +84,25 @@ rm -f selfsigned_nonca_serial0_key.tmp echo " Self-signed non-CA cert serial number:" openssl x509 -in selfsigned_nonca_serial0.pem -noout -serial +# 5. Create intermediate CA cert with serial 0, signed by root_serial0 +# (CA:TRUE but issuer != subject, so cert->selfSigned will be 0). +echo "" +echo "[5/5] Creating intermediate CA certificate with serial number 0..." +openssl req -newkey rsa:2048 -keyout intermediate_serial0_key.tmp \ + -out intermediate_serial0.csr.tmp -nodes \ + -subj "/CN=Intermediate CA Serial 0/O=wolfSSL Test/C=US" + +openssl x509 -req -in intermediate_serial0.csr.tmp \ + -CA root_serial0.pem -CAkey root_serial0_key.pem \ + -out intermediate_serial0.pem -days 3650 -set_serial 0 \ + -extfile <(echo "basicConstraints=critical,CA:TRUE +keyUsage=critical,keyCertSign,cRLSign") + +rm -f intermediate_serial0_key.tmp intermediate_serial0.csr.tmp + +echo " Intermediate CA cert serial number:" +openssl x509 -in intermediate_serial0.pem -noout -serial + echo "" echo "===================================================" echo "Certificate generation complete!" diff --git a/certs/test-serial0/include.am b/certs/test-serial0/include.am index cf20e1f8da..557e1da26c 100644 --- a/certs/test-serial0/include.am +++ b/certs/test-serial0/include.am @@ -7,5 +7,6 @@ EXTRA_DIST += certs/test-serial0/generate_certs.sh \ certs/test-serial0/root_serial0_key.pem \ certs/test-serial0/ee_serial0.pem \ certs/test-serial0/ee_normal.pem \ - certs/test-serial0/selfsigned_nonca_serial0.pem + certs/test-serial0/selfsigned_nonca_serial0.pem \ + certs/test-serial0/intermediate_serial0.pem diff --git a/certs/test-serial0/intermediate_serial0.pem b/certs/test-serial0/intermediate_serial0.pem new file mode 100644 index 0000000000..57bc4fa35c --- /dev/null +++ b/certs/test-serial0/intermediate_serial0.pem @@ -0,0 +1,21 @@ +-----BEGIN CERTIFICATE----- +MIIDaTCCAlGgAwIBAgIBADANBgkqhkiG9w0BAQsFADBEMR4wHAYDVQQDDBVUZXN0 +IFJvb3QgQ0EgU2VyaWFsIDAxFTATBgNVBAoMDHdvbGZTU0wgVGVzdDELMAkGA1UE +BhMCVVMwHhcNMjYwNDIzMjI0ODU3WhcNMzYwNDIwMjI0ODU3WjBHMSEwHwYDVQQD +DBhJbnRlcm1lZGlhdGUgQ0EgU2VyaWFsIDAxFTATBgNVBAoMDHdvbGZTU0wgVGVz +dDELMAkGA1UEBhMCVVMwggEiMA0GCSqGSIb3DQEBAQUAA4IBDwAwggEKAoIBAQC4 +vuEhBsjGh1kUXub+mnS+siPwFTUds5/wtCeJWeBZ1r7ks7jq0RMWHlPCnsB1RxFO +zKtRI1Vq4JlJmPN47VsWhRMm3RvQINf6L4dKhjVvYsJHWpJ0pEaopqogl4YlJPvM +yTuWm3O4FItS1iye9XdTBCdfsPOJgqHU8JxpFIBCijjReWYIqEP2wxQkO6/HcIm9 +eL4x3gaTzc8ye3ZBQcxRhjzg96AZ3N91pBFkyJAxzeBXME/L+j321svlsDNPQ1cn +pd39gEnGuhriy6R5UgOg5CptG7EGFrIpWI/+jexE3GM8zr6x+dXzUOccbJq40F1o +voEihRt+dXsScsp5oFMRAgMBAAGjYzBhMA8GA1UdEwEB/wQFMAMBAf8wDgYDVR0P +AQH/BAQDAgEGMB0GA1UdDgQWBBSJPt0qcSUhNus1U4l3OxjvkBdOszAfBgNVHSME +GDAWgBRh4Nck2fNzW7ljbZ7aY0JNA1WHwDANBgkqhkiG9w0BAQsFAAOCAQEAcO0A +3EOvpp9dMlQmt71HQ5dGtm2cerPe7yv9hiTWiL+s76eroaNoVptdoJvNnNcLGNdi +/5fGTnaCklbM/YW8xatxtvdiVMXQTVqVc+iZy3bc+j2eOJjl9qxsxv5mGjWefSL4 +DRrtZLQ1RqvZn2x1uVk3KIbCEfK1JpERN/rfHAsvR+adMxozUhNO5w6SMn5CnzNU +X6agw1zNXp/zaBuY8sXanRVRL71lHSEHYJSZtkBTcXH7/setEjVEVnC5Eq0OAB+P +ltJtU8Jrev2ABsy3di0rzC9jyZQMx+Bvw5LtIBRkONfe2bAsJVrgMznuANtLwDCV +nqmmSDLk6ODB/3Zo6g== +-----END CERTIFICATE----- diff --git a/tests/api.c b/tests/api.c index 2394378fc8..a986eef4c0 100644 --- a/tests/api.c +++ b/tests/api.c @@ -23773,67 +23773,69 @@ static int test_PathLenNoKeyUsage(void) return EXPECT_RESULT(); } -static int test_MakeCertWith0Ser(void) +/* Exhaustive matrix coverage of the serial-0 predicate in + * ParseCertRelative (asn.c). Inputs are openssl-generated PEM fixtures + * under certs/test-serial0/ — no cert-under-test data is generated by + * wolfSSL, so the test cannot pass for the wrong reason if wc_MakeCert + * encoding ever drifts. + * + * Predicate exempts only (CA_TYPE|TRUSTED_PEER_TYPE) && isCA && selfSigned. + * + * Fixture isCA selfSigned CERT_TYPE CA_TYPE + * root_serial0.pem 1 1 reject accept + * intermediate_serial0.pem 1 0 reject reject + * selfsigned_nonca_serial0.pem 0 1 reject reject + * ee_serial0.pem 0 0 reject reject + */ +static int test_ParseSerial0FixtureMatrix(void) { EXPECT_DECLS; -#if defined(WOLFSSL_CERT_REQ) && !defined(NO_ASN_TIME) && \ - defined(WOLFSSL_CERT_GEN) && !defined(NO_RSA) && \ - defined(WOLFSSL_KEY_GEN) && defined(WOLFSSL_ASN_TEMPLATE) - Cert cert; - DecodedCert decodedCert; - byte der[FOURK_BUF]; - int derSize = 0; - WC_RNG rng; - RsaKey key; - int ret; - - XMEMSET(&rng, 0, sizeof(WC_RNG)); - XMEMSET(&key, 0, sizeof(RsaKey)); - XMEMSET(&cert, 0, sizeof(Cert)); - XMEMSET(&decodedCert, 0, sizeof(DecodedCert)); - - ExpectIntEQ(wc_InitRng(&rng), 0); - ExpectIntEQ(wc_InitRsaKey(&key, NULL), 0); - ExpectIntEQ(wc_MakeRsaKey(&key, 2048, WC_RSA_EXPONENT, &rng), 0); - ExpectIntEQ(wc_InitCert(&cert), 0); - - (void)XSTRNCPY(cert.subject.country, "US", CTC_NAME_SIZE); - (void)XSTRNCPY(cert.subject.state, "state", CTC_NAME_SIZE); - (void)XSTRNCPY(cert.subject.locality, "Bozeman", CTC_NAME_SIZE); - (void)XSTRNCPY(cert.subject.org, "yourOrgNameHere", CTC_NAME_SIZE); - (void)XSTRNCPY(cert.subject.unit, "yourUnitNameHere", CTC_NAME_SIZE); - (void)XSTRNCPY(cert.subject.commonName, "www.yourDomain.com", - CTC_NAME_SIZE); - (void)XSTRNCPY(cert.subject.email, "yourEmail@yourDomain.com", - CTC_NAME_SIZE); - - cert.selfSigned = 1; - cert.isCA = 0; - cert.sigType = CTC_SHA256wRSA; - - /* set serial number to 0 */ - cert.serialSz = 1; - cert.serial[0] = 0; - - ExpectIntGE(wc_MakeCert(&cert, der, FOURK_BUF, &key, NULL, &rng), 0); - ExpectIntGE(derSize = wc_SignCert(cert.bodySz, cert.sigType, der, - FOURK_BUF, &key, NULL, &rng), 0); - - wc_InitDecodedCert(&decodedCert, der, (word32)derSize, NULL); - -#if !defined(WOLFSSL_NO_ASN_STRICT) && !defined(WOLFSSL_PYTHON) && \ +#if !defined(NO_CERTS) && !defined(NO_FILESYSTEM) && \ + defined(WOLFSSL_PEM_TO_DER) && !defined(WOLFSSL_NO_PEM) && \ + !defined(WOLFSSL_NO_ASN_STRICT) && !defined(WOLFSSL_PYTHON) && \ !defined(WOLFSSL_ASN_ALLOW_0_SERIAL) - ExpectIntEQ(wc_ParseCert(&decodedCert, CERT_TYPE, NO_VERIFY, NULL), - WC_NO_ERR_TRACE(ASN_PARSE_E)); -#else - ExpectIntEQ(wc_ParseCert(&decodedCert, CERT_TYPE, NO_VERIFY, NULL), 0); -#endif + struct { + const char* path; + int expectedCertType; /* expected wc_ParseCert(..., CERT_TYPE) */ + int expectedCaType; /* expected wc_ParseCert(..., CA_TYPE) */ + } cases[] = { + { "./certs/test-serial0/root_serial0.pem", + WC_NO_ERR_TRACE(ASN_PARSE_E), 0 }, + { "./certs/test-serial0/intermediate_serial0.pem", + WC_NO_ERR_TRACE(ASN_PARSE_E), WC_NO_ERR_TRACE(ASN_PARSE_E) }, + { "./certs/test-serial0/selfsigned_nonca_serial0.pem", + WC_NO_ERR_TRACE(ASN_PARSE_E), WC_NO_ERR_TRACE(ASN_PARSE_E) }, + { "./certs/test-serial0/ee_serial0.pem", + WC_NO_ERR_TRACE(ASN_PARSE_E), WC_NO_ERR_TRACE(ASN_PARSE_E) }, + }; + size_t i; - wc_FreeDecodedCert(&decodedCert); - ret = wc_FreeRsaKey(&key); - ExpectIntEQ(ret, 0); - ret = wc_FreeRng(&rng); - ExpectIntEQ(ret, 0); + for (i = 0; i < sizeof(cases) / sizeof(cases[0]); ++i) { + byte* pemBuf = NULL; + size_t pemSz = 0; + byte* derBuf = NULL; + int derSz = 0; + DecodedCert dc; + + ExpectIntEQ(load_file(cases[i].path, &pemBuf, &pemSz), 0); + ExpectNotNull(derBuf = (byte*)XMALLOC(pemSz, NULL, + DYNAMIC_TYPE_TMP_BUFFER)); + ExpectIntGE(derSz = wc_CertPemToDer(pemBuf, (int)pemSz, derBuf, + (int)pemSz, CERT_TYPE), 0); + + wc_InitDecodedCert(&dc, derBuf, (word32)derSz, NULL); + ExpectIntEQ(wc_ParseCert(&dc, CERT_TYPE, NO_VERIFY, NULL), + cases[i].expectedCertType); + wc_FreeDecodedCert(&dc); + + wc_InitDecodedCert(&dc, derBuf, (word32)derSz, NULL); + ExpectIntEQ(wc_ParseCert(&dc, CA_TYPE, NO_VERIFY, NULL), + cases[i].expectedCaType); + wc_FreeDecodedCert(&dc); + + XFREE(derBuf, NULL, DYNAMIC_TYPE_TMP_BUFFER); + XFREE(pemBuf, NULL, DYNAMIC_TYPE_TMP_BUFFER); + } #endif return EXPECT_RESULT(); } @@ -39135,7 +39137,7 @@ TEST_CASE testCases[] = { TEST_DECL(test_PathLenSelfIssuedAllowed), TEST_DECL(test_PathLenNoKeyUsage), TEST_DECL(test_NameConstraints_OtherName), - TEST_DECL(test_MakeCertWith0Ser), + TEST_DECL(test_ParseSerial0FixtureMatrix), TEST_DECL(test_MakeCertWithCaFalse), #ifdef WOLFSSL_CERT_SIGN_CB TEST_DECL(test_wc_SignCert_cb), diff --git a/tests/api/test_asn.c b/tests/api/test_asn.c index bcca20c513..4d48491489 100644 --- a/tests/api/test_asn.c +++ b/tests/api/test_asn.c @@ -1073,9 +1073,6 @@ int test_SerialNumber0_RootCA(void) wolfSSL_CertManagerFree(cm); cm = NULL; } - /* Balance the wolfSSL_Init refcount incremented by internal - * wolfSSL_CTX_new_ex calls in CertManagerLoadCA/Verify. */ - wolfSSL_Cleanup(); /* Test 4: Self-signed non-CA certificate with serial 0 should be rejected */ ExpectNotNull(cm = wolfSSL_CertManagerNew()); @@ -1086,7 +1083,21 @@ int test_SerialNumber0_RootCA(void) wolfSSL_CertManagerFree(cm); cm = NULL; } - wolfSSL_Cleanup(); + + /* Test 5: Intermediate CA (CA:TRUE but issuer != subject) with serial 0 + * must be rejected when loaded as CA_TYPE. Exercises the selfSigned + * half of the ParseCertRelative exemption predicate. */ + { + const char* intermediateSerial0File = + "./certs/test-serial0/intermediate_serial0.pem"; + ExpectNotNull(cm = wolfSSL_CertManagerNew()); + ExpectIntNE(wolfSSL_CertManagerLoadCA(cm, intermediateSerial0File, + NULL), WOLFSSL_SUCCESS); + if (cm != NULL) { + wolfSSL_CertManagerFree(cm); + cm = NULL; + } + } #endif /* !WOLFSSL_NO_ASN_STRICT && !WOLFSSL_PYTHON && !WOLFSSL_ASN_ALLOW_0_SERIAL && !WOLFSSL_TEST_APPLE_NATIVE_CERT_VALIDATION */ diff --git a/wolfcrypt/src/asn.c b/wolfcrypt/src/asn.c index 7f4c611dc7..82233ee85a 100644 --- a/wolfcrypt/src/asn.c +++ b/wolfcrypt/src/asn.c @@ -21395,6 +21395,13 @@ static int DecodeCertInternal(DecodedCert* cert, int verify, int* criticalExt, ret = badDate; } + /* Note: serial-0 rejection is performed in ParseCertRelative (after + * basicConstraints has been parsed and isCA is authoritative), not + * here. Checking isCA at this point would fail-open on a forged + * isCA flag. Callers that invoke DecodeCert/DecodeToKey/wc_GetPubX509 + * directly are pubkey-extraction paths and do not make trust + * decisions; trust-bearing flows go through ParseCertRelative. */ + return ret; } @@ -22797,18 +22804,28 @@ int ParseCertRelative(DecodedCert* cert, int type, int verify, void* cm, #if !defined(WOLFSSL_NO_ASN_STRICT) && !defined(WOLFSSL_PYTHON) && \ !defined(WOLFSSL_ASN_ALLOW_0_SERIAL) - /* Check for serial number of 0. RFC 5280 section 4.1.2.2 requires - * positive serial numbers. However, allow zero for self-signed CA - * certificates (root CAs) being loaded as trust anchors since they - * are explicitly trusted and some legacy root CAs in real-world - * trust stores have serial number 0. */ + /* RFC 5280 section 4.1.2.2 requires conforming CAs to issue + * positive serial numbers; the same section notes that verifiers + * SHOULD gracefully handle non-conforming certs with zero or + * negative serials. wolfSSL's policy is to reject as a security + * guard, with an exemption for self-signed CA certs loaded as + * explicitly-trusted anchors (some legacy real-world roots have + * serial 0). + * + * Note: cert->selfSigned is a subject/issuer name-hash compare + * (see DecodeCertInternal where it's set), not a validated + * self-signature. That is acceptable here because the trust + * decision is user-driven via CertManagerLoadCA; this check is + * only a structural sanity guard. */ if ((ret == 0) && (cert->serialSz == 1) && (cert->serial[0] == 0)) { - if (!((type == CA_TYPE || type == TRUSTED_PEER_TYPE) && - cert->isCA && cert->selfSigned) -#ifdef WOLFSSL_CERT_REQ - && !cert->isCSR -#endif - ) { + int isTrustAnchorLoad = + (type == CA_TYPE || type == TRUSTED_PEER_TYPE) + && cert->isCA && cert->selfSigned; + int isCsr = 0; + #ifdef WOLFSSL_CERT_REQ + isCsr = cert->isCSR; + #endif + if (!isTrustAnchorLoad && !isCsr) { WOLFSSL_MSG("Error serial number of 0 for non-root certificate"); return ASN_PARSE_E; }