From 7571fbdbfb1ab17ed617183282c146d179903501 Mon Sep 17 00:00:00 2001 From: John Safranek Date: Wed, 8 Jan 2020 16:39:30 -0800 Subject: [PATCH 1/4] Maintenance: X509 1. Fix for issue #2718. Added a flag to the X509 structure when someone sets the issuer name. 2. When making a certificate out of the X509, if the issuer name is set clear the self-signed flag in the cert. 3. Propigate the flat X509_NAMEs to the string the cert building code uses. --- src/ssl.c | 9 ++++++++- wolfssl/internal.h | 1 + 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/src/ssl.c b/src/ssl.c index a04a10bbf..99da25cd0 100644 --- a/src/ssl.c +++ b/src/ssl.c @@ -37157,6 +37157,8 @@ void* wolfSSL_GetDhAgreeCtx(WOLFSSL* ssl) } /* copy over Name structures */ + if (x509->issuerSet) + cert->selfSigned = 0; if ((ret = CopyX509NameToCertName(&(x509->issuer), &(cert->issuer))) != WOLFSSL_SUCCESS) { WOLFSSL_MSG("Error copying over issuer names"); @@ -38517,7 +38519,7 @@ err: if (dName->fullName != NULL) XFREE(dName->fullName, NULL, DYNAMIC_TYPE_X509); dName->fullName = fullName; - dName->fullNameLen = idx; + dName->fullNameLen = idx + 1; return 0; } @@ -47919,6 +47921,8 @@ int wolfSSL_X509_set_subject_name(WOLFSSL_X509 *cert, WOLFSSL_X509_NAME *name) wolfSSL_X509_NAME_add_entry(&cert->subject, ne, i, 1); } cert->subject.x509 = cert; + cert->subject.name = cert->subject.fullName.fullName; + cert->subject.sz = cert->subject.fullName.fullNameLen; return WOLFSSL_SUCCESS; } @@ -47949,6 +47953,9 @@ int wolfSSL_X509_set_issuer_name(WOLFSSL_X509 *cert, WOLFSSL_X509_NAME *name) wolfSSL_X509_NAME_add_entry(&cert->issuer, ne, i, 1); } cert->issuer.x509 = cert; + cert->issuer.name = cert->issuer.fullName.fullName; + cert->issuer.sz = cert->issuer.fullName.fullNameLen; + cert->issuerSet = 1; return WOLFSSL_SUCCESS; } diff --git a/wolfssl/internal.h b/wolfssl/internal.h index 2b00a3749..0521a7dad 100644 --- a/wolfssl/internal.h +++ b/wolfssl/internal.h @@ -3646,6 +3646,7 @@ struct WOLFSSL_X509 { WOLFSSL_X509_ALGOR algor; WOLFSSL_X509_PUBKEY key; #endif + byte issuerSet:1; }; From 5dcffa6b4060eee79b644b672aebadb8c3964695 Mon Sep 17 00:00:00 2001 From: John Safranek Date: Thu, 9 Jan 2020 15:34:45 -0800 Subject: [PATCH 2/4] Maintenance: X509 1. Fix for issue #2724. When making a certificate out of an X.509 structure, the subject alt names weren't getting correctly copied. 2. Added a function to flatten the DNS_entries into a sequence of GeneralNames. 3. Put the proper certificate extension wrapping around the flattened general names. --- src/ssl.c | 19 +--------- wolfcrypt/src/asn.c | 82 ++++++++++++++++++++++++++++++++++++++--- wolfssl/wolfcrypt/asn.h | 1 + 3 files changed, 79 insertions(+), 23 deletions(-) diff --git a/src/ssl.c b/src/ssl.c index 99da25cd0..882c05bc0 100644 --- a/src/ssl.c +++ b/src/ssl.c @@ -37084,24 +37084,9 @@ void* wolfSSL_GetDhAgreeCtx(WOLFSSL* ssl) cert->afterDateSz = 0; } - /* copy over alt names */ - { - int idx = 0; - DNS_entry* dns = x509->altNames; + cert->altNamesSz = FlattenAltNames(cert->altNames, + sizeof(cert->altNames), x509->altNames); - while (dns != NULL) { - int sz = (int)XSTRLEN(dns->name); - - if (sz < 0 || sz + idx > CTC_MAX_ALT_SIZE) { - WOLFSSL_MSG("Issue with copying over alt names"); - return WOLFSSL_FAILURE; - } - XMEMCPY(cert->altNames, dns->name, sz); - idx += sz; - dns = dns->next; - } - cert->altNamesSz = idx; - } #endif /* WOLFSSL_ALT_NAMES */ cert->sigType = wolfSSL_X509_get_signature_type(x509); diff --git a/wolfcrypt/src/asn.c b/wolfcrypt/src/asn.c index 44887763f..9923c700b 100644 --- a/wolfcrypt/src/asn.c +++ b/wolfcrypt/src/asn.c @@ -12155,21 +12155,91 @@ static int SetCertificatePolicies(byte *output, } #endif /* WOLFSSL_CERT_EXT */ + #ifdef WOLFSSL_ALT_NAMES + /* encode Alternative Names, return total bytes written */ -static int SetAltNames(byte *out, word32 outSz, byte *input, word32 length) +static int SetAltNames(byte *output, word32 outSz, + const byte *input, word32 length) { - if (out == NULL || input == NULL) + byte san_len[1 + MAX_LENGTH_SZ]; + int idx = 0, san_lenSz; + static const byte san_oid[] = { 0x06, 0x03, 0x55, 0x1d, 0x11 }; + + if (output == NULL || input == NULL) return BAD_FUNC_ARG; if (outSz < length) return BUFFER_E; - /* Alternative Names come from certificate or computed by - * external function, so already encoded. Just copy value */ - XMEMCPY(out, input, length); - return length; + /* Octet String header */ + san_lenSz = SetOctetString(length, san_len); + + if (outSz < MAX_SEQ_SZ) + return BUFFER_E; + + idx = SetSequence(length + sizeof(san_oid) + san_lenSz, output); + + if ((length + sizeof(san_oid) + san_lenSz) > outSz) + return BUFFER_E; + + /* put oid */ + XMEMCPY(output+idx, san_oid, sizeof(san_oid)); + idx += sizeof(san_oid); + + /* put octet header */ + XMEMCPY(output+idx, san_len, san_lenSz); + idx += san_lenSz; + + /* put value */ + XMEMCPY(output+idx, input, length); + idx += length; + + return idx; } + + +#ifdef WOLFSSL_CERT_GEN + +int FlattenAltNames(byte* output, word32 outputSz, const DNS_entry* names) +{ + word32 idx; + const DNS_entry* curName; + word32 namesSz = 0; + + if (output == NULL) + return BAD_FUNC_ARG; + + if (names == NULL) + return 0; + + curName = names; + do { + namesSz += curName->len + 2 + + ((curName->len < ASN_LONG_LENGTH) ? 0 + : BytePrecision(curName->len)); + curName = curName->next; + } while (curName != NULL); + + if (outputSz < MAX_SEQ_SZ + namesSz) + return BUFFER_E; + + idx = SetSequence(namesSz, output); + + curName = names; + do { + output[idx++] = ASN_CONTEXT_SPECIFIC | curName->type; + idx += SetLength(curName->len, output + idx); + XMEMCPY(output + idx, curName->name, curName->len); + idx += curName->len; + curName = curName->next; + } while (curName != NULL); + + return idx; +} + +#endif /* WOLFSSL_CERT_GEN */ + #endif /* WOLFSL_ALT_NAMES */ /* Encodes one attribute of the name (issuer/subject) diff --git a/wolfssl/wolfcrypt/asn.h b/wolfssl/wolfcrypt/asn.h index 95d1c9429..72ac9e108 100644 --- a/wolfssl/wolfcrypt/asn.h +++ b/wolfssl/wolfcrypt/asn.h @@ -1161,6 +1161,7 @@ WOLFSSL_LOCAL int GetNameHash(const byte* source, word32* idx, byte* hash, int maxIdx); WOLFSSL_LOCAL int wc_CheckPrivateKey(byte* key, word32 keySz, DecodedCert* der); WOLFSSL_LOCAL int StoreDHparams(byte* out, word32* outLen, mp_int* p, mp_int* g); +WOLFSSL_LOCAL int FlattenAltNames( byte*, word32, const DNS_entry*); #ifdef HAVE_ECC /* ASN sig helpers */ From 8d1b20706c493f259e283fdcdbdfcb637dcbeb0f Mon Sep 17 00:00:00 2001 From: John Safranek Date: Fri, 10 Jan 2020 16:06:26 -0800 Subject: [PATCH 3/4] Maintenance: X509 1. Add a test for the new alt name handling. 2. Added an API to set altnames in a WOLFSSL_X509 struct. Just adds DNS_entries. 3. Removed the "static" from a bunch of constant byte arrays used inside some of the ASN.1 code. --- src/ssl.c | 40 ++++++++++++++++++++++++++++++++++++++++ tests/api.c | 21 +++++++++++++++++++++ wolfcrypt/src/asn.c | 30 +++++++++++++++--------------- wolfssl/ssl.h | 1 + 4 files changed, 77 insertions(+), 15 deletions(-) diff --git a/src/ssl.c b/src/ssl.c index 882c05bc0..af27625c9 100644 --- a/src/ssl.c +++ b/src/ssl.c @@ -9743,6 +9743,46 @@ err: return NULL; } + +int wolfSSL_X509_add_altname(WOLFSSL_X509* x509, const char* name, int type) +{ + DNS_entry* newAltName = NULL; + char* nameCopy = NULL; + word32 nameSz; + + if (x509 == NULL) + return WOLFSSL_FAILURE; + + if (name == NULL) + return WOLFSSL_SUCCESS; + + nameSz = (word32)XSTRLEN(name); + if (nameSz == 0) + return WOLFSSL_SUCCESS; + + newAltName = (DNS_entry*)XMALLOC(sizeof(DNS_entry), + x509->heap, DYNAMIC_TYPE_ALTNAME); + if (newAltName == NULL) + return WOLFSSL_FAILURE; + + nameCopy = (char*)XMALLOC(nameSz + 1, x509->heap, DYNAMIC_TYPE_ALTNAME); + if (nameCopy == NULL) { + XFREE(newAltName, x509->heap, DYNAMIC_TYPE_ALTNAME); + return WOLFSSL_FAILURE; + } + + XSTRNCPY(nameCopy, name, nameSz); + + newAltName->next = x509->altNames; + newAltName->type = type; + newAltName->len = nameSz; + newAltName->name = nameCopy; + x509->altNames = newAltName; + + return WOLFSSL_SUCCESS; +} + + #ifndef NO_WOLFSSL_STUB int wolfSSL_X509_add_ext(WOLFSSL_X509 *x509, WOLFSSL_X509_EXTENSION *ext, int loc) { diff --git a/tests/api.c b/tests/api.c index 1d24b0114..ce1478ad9 100644 --- a/tests/api.c +++ b/tests/api.c @@ -22443,6 +22443,22 @@ static void test_wolfSSL_X509_sign(void) /* Set subject name, add pubkey, and sign certificate */ AssertIntEQ(X509_set_subject_name(x509, name), SSL_SUCCESS); AssertIntEQ(X509_set_pubkey(x509, pub), SSL_SUCCESS); +#ifdef WOLFSSL_ALT_NAMES + /* Add some subject alt names */ + AssertIntNE(wolfSSL_X509_add_altname(NULL, + NULL, ASN_DNS_TYPE), SSL_SUCCESS); + AssertIntEQ(wolfSSL_X509_add_altname(x509, + NULL, ASN_DNS_TYPE), SSL_SUCCESS); + AssertIntEQ(wolfSSL_X509_add_altname(x509, + "sphygmomanometer", + ASN_DNS_TYPE), SSL_SUCCESS); + AssertIntEQ(wolfSSL_X509_add_altname(x509, + "supercalifragilisticexpialidocious", + ASN_DNS_TYPE), SSL_SUCCESS); + AssertIntEQ(wolfSSL_X509_add_altname(x509, + "Llanfairpwllgwyngyllgogerychwyrndrobwllllantysiliogogogoch", + ASN_DNS_TYPE), SSL_SUCCESS); +#endif /* WOLFSSL_ALT_NAMES */ /* Test invalid parameters */ AssertIntEQ(X509_sign(NULL, priv, EVP_sha256()), 0); AssertIntEQ(X509_sign(x509, NULL, EVP_sha256()), 0); @@ -22461,8 +22477,13 @@ static void test_wolfSSL_X509_sign(void) XFCLOSE(tmpFile); #endif +#ifndef WOLFSSL_ALT_NAMES /* Valid case - size should be 798 */ AssertIntEQ(ret, 798); +#else /* WOLFSSL_ALT_NAMES */ + /* Valid case - size should be 927 */ + AssertIntEQ(ret, 927); +#endif /* WOLFSSL_ALT_NAMES */ X509_NAME_free(name); EVP_PKEY_free(priv); diff --git a/wolfcrypt/src/asn.c b/wolfcrypt/src/asn.c index 9923c700b..f45e03e24 100644 --- a/wolfcrypt/src/asn.c +++ b/wolfcrypt/src/asn.c @@ -5408,7 +5408,7 @@ WOLFSSL_API int EccEnumToNID(int n) #if defined(OPENSSL_EXTRA) || defined(OPENSSL_EXTRA_X509_SMALL) WOLFSSL_LOCAL int wc_OBJ_sn2nid(const char *sn) { - static const struct { + const struct { const char *sn; int nid; } sn2nid[] = { @@ -11867,7 +11867,7 @@ static int SetExtensionsHeader(byte* out, word32 outSz, int extSz) /* encode CA basic constraint true, return total bytes written */ static int SetCa(byte* out, word32 outSz) { - static const byte ca[] = { 0x30, 0x0c, 0x06, 0x03, 0x55, 0x1d, 0x13, 0x04, + const byte ca[] = { 0x30, 0x0c, 0x06, 0x03, 0x55, 0x1d, 0x13, 0x04, 0x05, 0x30, 0x03, 0x01, 0x01, 0xff }; if (out == NULL) @@ -11916,7 +11916,7 @@ static int SetSKID(byte* output, word32 outSz, const byte *input, word32 length) byte skid_len[1 + MAX_LENGTH_SZ]; byte skid_enc_len[MAX_LENGTH_SZ]; int idx = 0, skid_lenSz, skid_enc_lenSz; - static const byte skid_oid[] = { 0x06, 0x03, 0x55, 0x1d, 0x0e, 0x04 }; + const byte skid_oid[] = { 0x06, 0x03, 0x55, 0x1d, 0x0e, 0x04 }; if (output == NULL || input == NULL) return BAD_FUNC_ARG; @@ -11962,8 +11962,8 @@ static int SetAKID(byte* output, word32 outSz, { byte *enc_val; int ret, enc_valSz; - static const byte akid_oid[] = { 0x06, 0x03, 0x55, 0x1d, 0x23, 0x04 }; - static const byte akid_cs[] = { 0x80 }; + const byte akid_oid[] = { 0x06, 0x03, 0x55, 0x1d, 0x23, 0x04 }; + const byte akid_cs[] = { 0x80 }; (void)heap; @@ -11995,7 +11995,7 @@ static int SetKeyUsage(byte* output, word32 outSz, word16 input) { byte ku[5]; int idx; - static const byte keyusage_oid[] = { 0x06, 0x03, 0x55, 0x1d, 0x0f, + const byte keyusage_oid[] = { 0x06, 0x03, 0x55, 0x1d, 0x0f, 0x01, 0x01, 0xff, 0x04}; if (output == NULL) return BAD_FUNC_ARG; @@ -12023,7 +12023,7 @@ static int SetOjectIdValue(byte* output, word32 outSz, int* idx, static int SetExtKeyUsage(Cert* cert, byte* output, word32 outSz, byte input) { int idx = 0, oidListSz = 0, totalSz, ret = 0; - static const byte extkeyusage_oid[] = { 0x06, 0x03, 0x55, 0x1d, 0x25 }; + const byte extkeyusage_oid[] = { 0x06, 0x03, 0x55, 0x1d, 0x25 }; if (output == NULL) return BAD_FUNC_ARG; @@ -12115,8 +12115,8 @@ static int SetCertificatePolicies(byte *output, word32 outSz, i = 0, der_oidSz[MAX_CERTPOL_NB]; int ret; - static const byte certpol_oid[] = { 0x06, 0x03, 0x55, 0x1d, 0x20, 0x04 }; - static const byte oid_oid[] = { 0x06 }; + const byte certpol_oid[] = { 0x06, 0x03, 0x55, 0x1d, 0x20, 0x04 }; + const byte oid_oid[] = { 0x06 }; if (output == NULL || input == NULL || nb_certpol > MAX_CERTPOL_NB) return BAD_FUNC_ARG; @@ -12164,7 +12164,7 @@ static int SetAltNames(byte *output, word32 outSz, { byte san_len[1 + MAX_LENGTH_SZ]; int idx = 0, san_lenSz; - static const byte san_oid[] = { 0x06, 0x03, 0x55, 0x1d, 0x11 }; + const byte san_oid[] = { 0x06, 0x03, 0x55, 0x1d, 0x11 }; if (output == NULL || input == NULL) return BAD_FUNC_ARG; @@ -12240,7 +12240,7 @@ int FlattenAltNames(byte* output, word32 outputSz, const DNS_entry* names) #endif /* WOLFSSL_CERT_GEN */ -#endif /* WOLFSL_ALT_NAMES */ +#endif /* WOLFSSL_ALT_NAMES */ /* Encodes one attribute of the name (issuer/subject) * @@ -13127,10 +13127,10 @@ int wc_MakeNtruCert(Cert* cert, byte* derBuffer, word32 derSz, static int SetReqAttrib(byte* output, char* pw, int pwPrintableString, int extSz) { - static const byte cpOid[] = + const byte cpOid[] = { ASN_OBJECT_ID, 0x09, 0x2a, 0x86, 0x48, 0x86, 0xf7, 0x0d, 0x01, 0x09, 0x07 }; - static const byte erOid[] = + const byte erOid[] = { ASN_OBJECT_ID, 0x09, 0x2a, 0x86, 0x48, 0x86, 0xf7, 0x0d, 0x01, 0x09, 0x0e }; @@ -14929,7 +14929,7 @@ int wc_EccPrivateKeyDecode(const byte* input, word32* inOutIdx, ecc_key* key, #ifdef WOLFSSL_CUSTOM_CURVES static void ByteToHex(byte n, char* str) { - static const char hexChar[] = { '0', '1', '2', '3', '4', '5', '6', '7', + const char hexChar[] = { '0', '1', '2', '3', '4', '5', '6', '7', '8', '9', 'a', 'b', 'c', 'd', 'e', 'f' }; str[0] = hexChar[n >> 4]; @@ -16116,7 +16116,7 @@ int OcspResponseDecode(OcspResponse* resp, void* cm, void* heap, int noVerify) word32 EncodeOcspRequestExtensions(OcspRequest* req, byte* output, word32 size) { - static const byte NonceObjId[] = { 0x2b, 0x06, 0x01, 0x05, 0x05, 0x07, + const byte NonceObjId[] = { 0x2b, 0x06, 0x01, 0x05, 0x05, 0x07, 0x30, 0x01, 0x02 }; byte seqArray[5][MAX_SEQ_SZ]; word32 seqSz[5], totalSz = (word32)sizeof(NonceObjId); diff --git a/wolfssl/ssl.h b/wolfssl/ssl.h index 2c5c53dc0..79fec7237 100644 --- a/wolfssl/ssl.h +++ b/wolfssl/ssl.h @@ -2076,6 +2076,7 @@ WOLFSSL_API int wolfSSL_X509_version(WOLFSSL_X509*); WOLFSSL_API int wolfSSL_cmp_peer_cert_to_file(WOLFSSL*, const char*); WOLFSSL_ABI WOLFSSL_API char* wolfSSL_X509_get_next_altname(WOLFSSL_X509*); +WOLFSSL_API int wolfSSL_X509_add_altname(WOLFSSL_X509*, const char*, int); WOLFSSL_API WOLFSSL_X509* wolfSSL_d2i_X509(WOLFSSL_X509** x509, const unsigned char** in, int len); From c69bd5169f44beaa07f42f5d84061669b48c2cc1 Mon Sep 17 00:00:00 2001 From: John Safranek Date: Fri, 10 Jan 2020 16:17:27 -0800 Subject: [PATCH 4/4] Switch strncpy to memcpy in the altname store function. --- src/ssl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ssl.c b/src/ssl.c index af27625c9..8cf4f63dd 100644 --- a/src/ssl.c +++ b/src/ssl.c @@ -9771,7 +9771,7 @@ int wolfSSL_X509_add_altname(WOLFSSL_X509* x509, const char* name, int type) return WOLFSSL_FAILURE; } - XSTRNCPY(nameCopy, name, nameSz); + XMEMCPY(nameCopy, name, nameSz + 1); newAltName->next = x509->altNames; newAltName->type = type;