Skip to content

Commit 724b28b

Browse files
committed
Skoll review
1 parent a6f6094 commit 724b28b

6 files changed

Lines changed: 148 additions & 75 deletions

File tree

certs/test-serial0/generate_certs.sh

Lines changed: 25 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@
1111
# ee_serial0.pem - EE cert with serial 0 (rejected)
1212
# ee_normal.pem - Normal EE cert (serial 100)
1313
# selfsigned_nonca_serial0.pem - Self-signed non-CA, serial 0
14+
# intermediate_serial0.pem - Intermediate CA, serial 0
15+
# (CA:TRUE but issuer != subject)
1416

1517
set -e
1618

@@ -23,7 +25,7 @@ echo "==================================================="
2325

2426
# 1. Create Root CA with serial number 0
2527
echo ""
26-
echo "[1/4] Creating Root CA with serial number 0..."
28+
echo "[1/5] Creating Root CA with serial number 0..."
2729
openssl req -x509 -newkey rsa:2048 -keyout root_serial0_key.pem -out root_serial0.pem \
2830
-days 7300 -nodes -subj "/CN=Test Root CA Serial 0/O=wolfSSL Test/C=US" \
2931
-set_serial 0 \
@@ -35,7 +37,7 @@ openssl x509 -in root_serial0.pem -noout -serial
3537

3638
# 2. Create end-entity cert with serial 0 signed by root_serial0
3739
echo ""
38-
echo "[2/4] Creating end-entity certificate with serial number 0..."
40+
echo "[2/5] Creating end-entity certificate with serial number 0..."
3941
openssl req -newkey rsa:2048 -keyout ee_serial0_key.tmp -out ee_serial0.csr.tmp -nodes \
4042
-subj "/CN=End Entity Serial 0/O=wolfSSL Test/C=US"
4143

@@ -52,7 +54,7 @@ openssl x509 -in ee_serial0.pem -noout -serial
5254

5355
# 3. Create normal end-entity cert signed by root CA with serial 0
5456
echo ""
55-
echo "[3/4] Creating normal end-entity certificate (signed by serial 0 root)..."
57+
echo "[3/5] Creating normal end-entity certificate (signed by serial 0 root)..."
5658
openssl req -newkey rsa:2048 -keyout ee_normal_key.tmp -out ee_normal.csr.tmp -nodes \
5759
-subj "/CN=End Entity Normal/O=wolfSSL Test/C=US"
5860

@@ -69,7 +71,7 @@ openssl x509 -in ee_normal.pem -noout -serial
6971

7072
# 4. Create self-signed non-CA certificate with serial 0
7173
echo ""
72-
echo "[4/4] Creating self-signed non-CA certificate with serial number 0..."
74+
echo "[4/5] Creating self-signed non-CA certificate with serial number 0..."
7375
openssl req -x509 -newkey rsa:2048 -keyout selfsigned_nonca_serial0_key.tmp \
7476
-out selfsigned_nonca_serial0.pem -days 3650 -nodes \
7577
-subj "/CN=Self-Signed Non-CA Serial 0/O=wolfSSL Test/C=US" \
@@ -82,6 +84,25 @@ rm -f selfsigned_nonca_serial0_key.tmp
8284
echo " Self-signed non-CA cert serial number:"
8385
openssl x509 -in selfsigned_nonca_serial0.pem -noout -serial
8486

87+
# 5. Create intermediate CA cert with serial 0, signed by root_serial0
88+
# (CA:TRUE but issuer != subject, so cert->selfSigned will be 0).
89+
echo ""
90+
echo "[5/5] Creating intermediate CA certificate with serial number 0..."
91+
openssl req -newkey rsa:2048 -keyout intermediate_serial0_key.tmp \
92+
-out intermediate_serial0.csr.tmp -nodes \
93+
-subj "/CN=Intermediate CA Serial 0/O=wolfSSL Test/C=US"
94+
95+
openssl x509 -req -in intermediate_serial0.csr.tmp \
96+
-CA root_serial0.pem -CAkey root_serial0_key.pem \
97+
-out intermediate_serial0.pem -days 3650 -set_serial 0 \
98+
-extfile <(echo "basicConstraints=critical,CA:TRUE
99+
keyUsage=critical,keyCertSign,cRLSign")
100+
101+
rm -f intermediate_serial0_key.tmp intermediate_serial0.csr.tmp
102+
103+
echo " Intermediate CA cert serial number:"
104+
openssl x509 -in intermediate_serial0.pem -noout -serial
105+
85106
echo ""
86107
echo "==================================================="
87108
echo "Certificate generation complete!"

certs/test-serial0/include.am

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,5 +7,6 @@ EXTRA_DIST += certs/test-serial0/generate_certs.sh \
77
certs/test-serial0/root_serial0_key.pem \
88
certs/test-serial0/ee_serial0.pem \
99
certs/test-serial0/ee_normal.pem \
10-
certs/test-serial0/selfsigned_nonca_serial0.pem
10+
certs/test-serial0/selfsigned_nonca_serial0.pem \
11+
certs/test-serial0/intermediate_serial0.pem
1112

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
-----BEGIN CERTIFICATE-----
2+
MIIDaTCCAlGgAwIBAgIBADANBgkqhkiG9w0BAQsFADBEMR4wHAYDVQQDDBVUZXN0
3+
IFJvb3QgQ0EgU2VyaWFsIDAxFTATBgNVBAoMDHdvbGZTU0wgVGVzdDELMAkGA1UE
4+
BhMCVVMwHhcNMjYwNDIzMjI0ODU3WhcNMzYwNDIwMjI0ODU3WjBHMSEwHwYDVQQD
5+
DBhJbnRlcm1lZGlhdGUgQ0EgU2VyaWFsIDAxFTATBgNVBAoMDHdvbGZTU0wgVGVz
6+
dDELMAkGA1UEBhMCVVMwggEiMA0GCSqGSIb3DQEBAQUAA4IBDwAwggEKAoIBAQC4
7+
vuEhBsjGh1kUXub+mnS+siPwFTUds5/wtCeJWeBZ1r7ks7jq0RMWHlPCnsB1RxFO
8+
zKtRI1Vq4JlJmPN47VsWhRMm3RvQINf6L4dKhjVvYsJHWpJ0pEaopqogl4YlJPvM
9+
yTuWm3O4FItS1iye9XdTBCdfsPOJgqHU8JxpFIBCijjReWYIqEP2wxQkO6/HcIm9
10+
eL4x3gaTzc8ye3ZBQcxRhjzg96AZ3N91pBFkyJAxzeBXME/L+j321svlsDNPQ1cn
11+
pd39gEnGuhriy6R5UgOg5CptG7EGFrIpWI/+jexE3GM8zr6x+dXzUOccbJq40F1o
12+
voEihRt+dXsScsp5oFMRAgMBAAGjYzBhMA8GA1UdEwEB/wQFMAMBAf8wDgYDVR0P
13+
AQH/BAQDAgEGMB0GA1UdDgQWBBSJPt0qcSUhNus1U4l3OxjvkBdOszAfBgNVHSME
14+
GDAWgBRh4Nck2fNzW7ljbZ7aY0JNA1WHwDANBgkqhkiG9w0BAQsFAAOCAQEAcO0A
15+
3EOvpp9dMlQmt71HQ5dGtm2cerPe7yv9hiTWiL+s76eroaNoVptdoJvNnNcLGNdi
16+
/5fGTnaCklbM/YW8xatxtvdiVMXQTVqVc+iZy3bc+j2eOJjl9qxsxv5mGjWefSL4
17+
DRrtZLQ1RqvZn2x1uVk3KIbCEfK1JpERN/rfHAsvR+adMxozUhNO5w6SMn5CnzNU
18+
X6agw1zNXp/zaBuY8sXanRVRL71lHSEHYJSZtkBTcXH7/setEjVEVnC5Eq0OAB+P
19+
ltJtU8Jrev2ABsy3di0rzC9jyZQMx+Bvw5LtIBRkONfe2bAsJVrgMznuANtLwDCV
20+
nqmmSDLk6ODB/3Zo6g==
21+
-----END CERTIFICATE-----

tests/api.c

Lines changed: 57 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -22124,67 +22124,69 @@ static int test_PathLenNoKeyUsage(void)
2212422124
return EXPECT_RESULT();
2212522125
}
2212622126

22127-
static int test_MakeCertWith0Ser(void)
22127+
/* Exhaustive matrix coverage of the serial-0 predicate in
22128+
* ParseCertRelative (asn.c). Inputs are openssl-generated PEM fixtures
22129+
* under certs/test-serial0/ — no cert-under-test data is generated by
22130+
* wolfSSL, so the test cannot pass for the wrong reason if wc_MakeCert
22131+
* encoding ever drifts.
22132+
*
22133+
* Predicate exempts only (CA_TYPE|TRUSTED_PEER_TYPE) && isCA && selfSigned.
22134+
*
22135+
* Fixture isCA selfSigned CERT_TYPE CA_TYPE
22136+
* root_serial0.pem 1 1 reject accept
22137+
* intermediate_serial0.pem 1 0 reject reject
22138+
* selfsigned_nonca_serial0.pem 0 1 reject reject
22139+
* ee_serial0.pem 0 0 reject reject
22140+
*/
22141+
static int test_ParseSerial0FixtureMatrix(void)
2212822142
{
2212922143
EXPECT_DECLS;
22130-
#if defined(WOLFSSL_CERT_REQ) && !defined(NO_ASN_TIME) && \
22131-
defined(WOLFSSL_CERT_GEN) && !defined(NO_RSA) && \
22132-
defined(WOLFSSL_KEY_GEN) && defined(WOLFSSL_ASN_TEMPLATE)
22133-
Cert cert;
22134-
DecodedCert decodedCert;
22135-
byte der[FOURK_BUF];
22136-
int derSize = 0;
22137-
WC_RNG rng;
22138-
RsaKey key;
22139-
int ret;
22140-
22141-
XMEMSET(&rng, 0, sizeof(WC_RNG));
22142-
XMEMSET(&key, 0, sizeof(RsaKey));
22143-
XMEMSET(&cert, 0, sizeof(Cert));
22144-
XMEMSET(&decodedCert, 0, sizeof(DecodedCert));
22145-
22146-
ExpectIntEQ(wc_InitRng(&rng), 0);
22147-
ExpectIntEQ(wc_InitRsaKey(&key, NULL), 0);
22148-
ExpectIntEQ(wc_MakeRsaKey(&key, 2048, WC_RSA_EXPONENT, &rng), 0);
22149-
ExpectIntEQ(wc_InitCert(&cert), 0);
22150-
22151-
(void)XSTRNCPY(cert.subject.country, "US", CTC_NAME_SIZE);
22152-
(void)XSTRNCPY(cert.subject.state, "state", CTC_NAME_SIZE);
22153-
(void)XSTRNCPY(cert.subject.locality, "Bozeman", CTC_NAME_SIZE);
22154-
(void)XSTRNCPY(cert.subject.org, "yourOrgNameHere", CTC_NAME_SIZE);
22155-
(void)XSTRNCPY(cert.subject.unit, "yourUnitNameHere", CTC_NAME_SIZE);
22156-
(void)XSTRNCPY(cert.subject.commonName, "www.yourDomain.com",
22157-
CTC_NAME_SIZE);
22158-
(void)XSTRNCPY(cert.subject.email, "yourEmail@yourDomain.com",
22159-
CTC_NAME_SIZE);
22144+
#if !defined(NO_CERTS) && !defined(NO_FILESYSTEM) && \
22145+
defined(WOLFSSL_PEM_TO_DER) && !defined(WOLFSSL_NO_PEM) && \
22146+
!defined(WOLFSSL_NO_ASN_STRICT) && !defined(WOLFSSL_PYTHON) && \
22147+
!defined(WOLFSSL_ASN_ALLOW_0_SERIAL)
22148+
struct {
22149+
const char* path;
22150+
int expectedCertType; /* expected wc_ParseCert(..., CERT_TYPE) */
22151+
int expectedCaType; /* expected wc_ParseCert(..., CA_TYPE) */
22152+
} cases[] = {
22153+
{ "./certs/test-serial0/root_serial0.pem",
22154+
WC_NO_ERR_TRACE(ASN_PARSE_E), 0 },
22155+
{ "./certs/test-serial0/intermediate_serial0.pem",
22156+
WC_NO_ERR_TRACE(ASN_PARSE_E), WC_NO_ERR_TRACE(ASN_PARSE_E) },
22157+
{ "./certs/test-serial0/selfsigned_nonca_serial0.pem",
22158+
WC_NO_ERR_TRACE(ASN_PARSE_E), WC_NO_ERR_TRACE(ASN_PARSE_E) },
22159+
{ "./certs/test-serial0/ee_serial0.pem",
22160+
WC_NO_ERR_TRACE(ASN_PARSE_E), WC_NO_ERR_TRACE(ASN_PARSE_E) },
22161+
};
22162+
size_t i;
2216022163

22161-
cert.selfSigned = 1;
22162-
cert.isCA = 0;
22163-
cert.sigType = CTC_SHA256wRSA;
22164+
for (i = 0; i < sizeof(cases) / sizeof(cases[0]); ++i) {
22165+
byte* pemBuf = NULL;
22166+
size_t pemSz = 0;
22167+
byte* derBuf = NULL;
22168+
int derSz = 0;
22169+
DecodedCert dc;
2216422170

22165-
/* set serial number to 0 */
22166-
cert.serialSz = 1;
22167-
cert.serial[0] = 0;
22171+
ExpectIntEQ(load_file(cases[i].path, &pemBuf, &pemSz), 0);
22172+
ExpectNotNull(derBuf = (byte*)XMALLOC(pemSz, NULL,
22173+
DYNAMIC_TYPE_TMP_BUFFER));
22174+
ExpectIntGE(derSz = wc_CertPemToDer(pemBuf, (int)pemSz, derBuf,
22175+
(int)pemSz, CERT_TYPE), 0);
2216822176

22169-
ExpectIntGE(wc_MakeCert(&cert, der, FOURK_BUF, &key, NULL, &rng), 0);
22170-
ExpectIntGE(derSize = wc_SignCert(cert.bodySz, cert.sigType, der,
22171-
FOURK_BUF, &key, NULL, &rng), 0);
22177+
wc_InitDecodedCert(&dc, derBuf, (word32)derSz, NULL);
22178+
ExpectIntEQ(wc_ParseCert(&dc, CERT_TYPE, NO_VERIFY, NULL),
22179+
cases[i].expectedCertType);
22180+
wc_FreeDecodedCert(&dc);
2217222181

22173-
wc_InitDecodedCert(&decodedCert, der, (word32)derSize, NULL);
22182+
wc_InitDecodedCert(&dc, derBuf, (word32)derSz, NULL);
22183+
ExpectIntEQ(wc_ParseCert(&dc, CA_TYPE, NO_VERIFY, NULL),
22184+
cases[i].expectedCaType);
22185+
wc_FreeDecodedCert(&dc);
2217422186

22175-
#if !defined(WOLFSSL_NO_ASN_STRICT) && !defined(WOLFSSL_PYTHON) && \
22176-
!defined(WOLFSSL_ASN_ALLOW_0_SERIAL)
22177-
ExpectIntEQ(wc_ParseCert(&decodedCert, CERT_TYPE, NO_VERIFY, NULL),
22178-
WC_NO_ERR_TRACE(ASN_PARSE_E));
22179-
#else
22180-
ExpectIntEQ(wc_ParseCert(&decodedCert, CERT_TYPE, NO_VERIFY, NULL), 0);
22181-
#endif
22182-
22183-
wc_FreeDecodedCert(&decodedCert);
22184-
ret = wc_FreeRsaKey(&key);
22185-
ExpectIntEQ(ret, 0);
22186-
ret = wc_FreeRng(&rng);
22187-
ExpectIntEQ(ret, 0);
22187+
XFREE(derBuf, NULL, DYNAMIC_TYPE_TMP_BUFFER);
22188+
XFREE(pemBuf, NULL, DYNAMIC_TYPE_TMP_BUFFER);
22189+
}
2218822190
#endif
2218922191
return EXPECT_RESULT();
2219022192
}
@@ -37008,7 +37010,7 @@ TEST_CASE testCases[] = {
3700837010
TEST_DECL(test_PathLenSelfIssued),
3700937011
TEST_DECL(test_PathLenSelfIssuedAllowed),
3701037012
TEST_DECL(test_PathLenNoKeyUsage),
37011-
TEST_DECL(test_MakeCertWith0Ser),
37013+
TEST_DECL(test_ParseSerial0FixtureMatrix),
3701237014
TEST_DECL(test_MakeCertWithCaFalse),
3701337015
#ifdef WOLFSSL_CERT_SIGN_CB
3701437016
TEST_DECL(test_wc_SignCert_cb),

tests/api/test_asn.c

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1073,9 +1073,6 @@ int test_SerialNumber0_RootCA(void)
10731073
wolfSSL_CertManagerFree(cm);
10741074
cm = NULL;
10751075
}
1076-
/* Balance the wolfSSL_Init refcount incremented by internal
1077-
* wolfSSL_CTX_new_ex calls in CertManagerLoadCA/Verify. */
1078-
wolfSSL_Cleanup();
10791076

10801077
/* Test 4: Self-signed non-CA certificate with serial 0 should be rejected */
10811078
ExpectNotNull(cm = wolfSSL_CertManagerNew());
@@ -1086,7 +1083,21 @@ int test_SerialNumber0_RootCA(void)
10861083
wolfSSL_CertManagerFree(cm);
10871084
cm = NULL;
10881085
}
1089-
wolfSSL_Cleanup();
1086+
1087+
/* Test 5: Intermediate CA (CA:TRUE but issuer != subject) with serial 0
1088+
* must be rejected when loaded as CA_TYPE. Exercises the selfSigned
1089+
* half of the ParseCertRelative exemption predicate. */
1090+
{
1091+
const char* intermediateSerial0File =
1092+
"./certs/test-serial0/intermediate_serial0.pem";
1093+
ExpectNotNull(cm = wolfSSL_CertManagerNew());
1094+
ExpectIntNE(wolfSSL_CertManagerLoadCA(cm, intermediateSerial0File,
1095+
NULL), WOLFSSL_SUCCESS);
1096+
if (cm != NULL) {
1097+
wolfSSL_CertManagerFree(cm);
1098+
cm = NULL;
1099+
}
1100+
}
10901101
#endif /* !WOLFSSL_NO_ASN_STRICT && !WOLFSSL_PYTHON &&
10911102
!WOLFSSL_ASN_ALLOW_0_SERIAL &&
10921103
!WOLFSSL_TEST_APPLE_NATIVE_CERT_VALIDATION */

wolfcrypt/src/asn.c

Lines changed: 28 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -20988,6 +20988,13 @@ static int DecodeCertInternal(DecodedCert* cert, int verify, int* criticalExt,
2098820988
ret = badDate;
2098920989
}
2099020990

20991+
/* Note: serial-0 rejection is performed in ParseCertRelative (after
20992+
* basicConstraints has been parsed and isCA is authoritative), not
20993+
* here. Checking isCA at this point would fail-open on a forged
20994+
* isCA flag. Callers that invoke DecodeCert/DecodeToKey/wc_GetPubX509
20995+
* directly are pubkey-extraction paths and do not make trust
20996+
* decisions; trust-bearing flows go through ParseCertRelative. */
20997+
2099120998
return ret;
2099220999
}
2099321000

@@ -22390,18 +22397,28 @@ int ParseCertRelative(DecodedCert* cert, int type, int verify, void* cm,
2239022397

2239122398
#if !defined(WOLFSSL_NO_ASN_STRICT) && !defined(WOLFSSL_PYTHON) && \
2239222399
!defined(WOLFSSL_ASN_ALLOW_0_SERIAL)
22393-
/* Check for serial number of 0. RFC 5280 section 4.1.2.2 requires
22394-
* positive serial numbers. However, allow zero for self-signed CA
22395-
* certificates (root CAs) being loaded as trust anchors since they
22396-
* are explicitly trusted and some legacy root CAs in real-world
22397-
* trust stores have serial number 0. */
22400+
/* RFC 5280 section 4.1.2.2 requires conforming CAs to issue
22401+
* positive serial numbers; the same section notes that verifiers
22402+
* SHOULD gracefully handle non-conforming certs with zero or
22403+
* negative serials. wolfSSL's policy is to reject as a security
22404+
* guard, with an exemption for self-signed CA certs loaded as
22405+
* explicitly-trusted anchors (some legacy real-world roots have
22406+
* serial 0).
22407+
*
22408+
* Note: cert->selfSigned is a subject/issuer name-hash compare
22409+
* (see DecodeCertInternal where it's set), not a validated
22410+
* self-signature. That is acceptable here because the trust
22411+
* decision is user-driven via CertManagerLoadCA; this check is
22412+
* only a structural sanity guard. */
2239822413
if ((ret == 0) && (cert->serialSz == 1) && (cert->serial[0] == 0)) {
22399-
if (!((type == CA_TYPE || type == TRUSTED_PEER_TYPE) &&
22400-
cert->isCA && cert->selfSigned)
22401-
#ifdef WOLFSSL_CERT_REQ
22402-
&& !cert->isCSR
22403-
#endif
22404-
) {
22414+
int isTrustAnchorLoad =
22415+
(type == CA_TYPE || type == TRUSTED_PEER_TYPE)
22416+
&& cert->isCA && cert->selfSigned;
22417+
int isCsr = 0;
22418+
#ifdef WOLFSSL_CERT_REQ
22419+
isCsr = cert->isCSR;
22420+
#endif
22421+
if (!isTrustAnchorLoad && !isCsr) {
2240522422
WOLFSSL_MSG("Error serial number of 0 for non-root certificate");
2240622423
return ASN_PARSE_E;
2240722424
}

0 commit comments

Comments
 (0)