-
Notifications
You must be signed in to change notification settings - Fork 996
Add public alt-name list APIs #10768
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1035,6 +1035,12 @@ WOLFSSL_TEST_SUBROUTINE wc_test_ret_t certext_test(void); | |
| defined(WOLFSSL_CERT_EXT) && defined(WOLFSSL_CERT_GEN) | ||
| WOLFSSL_TEST_SUBROUTINE wc_test_ret_t decodedCertCache_test(void); | ||
| #endif | ||
| #if defined(WOLFSSL_CERT_GEN) && defined(WOLFSSL_ALT_NAMES) && \ | ||
| defined(WOLFSSL_ASN_TEMPLATE) && \ | ||
| (defined(WOLFSSL_TEST_CERT) || defined(OPENSSL_EXTRA) || \ | ||
| defined(OPENSSL_EXTRA_X509_SMALL) || defined(WOLFSSL_PUBLIC_ASN)) | ||
| WOLFSSL_TEST_SUBROUTINE wc_test_ret_t flattenAltNames_test(void); | ||
| #endif | ||
| WOLFSSL_TEST_SUBROUTINE wc_test_ret_t memory_test(void); | ||
| #if defined(WOLFSSL_PUBLIC_MP) && \ | ||
| ((defined(WOLFSSL_SP_MATH_ALL) && !defined(WOLFSSL_RSA_VERIFY_ONLY)) || \ | ||
|
|
@@ -3089,6 +3095,16 @@ options: [-s max_relative_stack_bytes] [-m max_relative_heap_memory_bytes]\n\ | |
| TEST_PASS("DECODED CERT CACHE test passed!\n"); | ||
| #endif | ||
|
|
||
| #if defined(WOLFSSL_CERT_GEN) && defined(WOLFSSL_ALT_NAMES) && \ | ||
| defined(WOLFSSL_ASN_TEMPLATE) && \ | ||
| (defined(WOLFSSL_TEST_CERT) || defined(OPENSSL_EXTRA) || \ | ||
| defined(OPENSSL_EXTRA_X509_SMALL) || defined(WOLFSSL_PUBLIC_ASN)) | ||
| if ( (ret = flattenAltNames_test()) != 0) | ||
| TEST_FAIL("FLATTEN ALT NAMES test failed!\n", ret); | ||
| else | ||
| TEST_PASS("FLATTEN ALT NAMES test passed!\n"); | ||
| #endif | ||
|
|
||
| #ifdef HAVE_CURVE25519 | ||
| if ( (ret = curve25519_test()) != 0) | ||
| TEST_FAIL("CURVE25519 test failed!\n", ret); | ||
|
|
@@ -26485,6 +26501,118 @@ WOLFSSL_TEST_SUBROUTINE wc_test_ret_t decodedCertCache_test(void) | |
| #endif /* defined(WOLFSSL_CERT_GEN_CACHE) && defined(WOLFSSL_TEST_CERT) && | ||
| defined(WOLFSSL_CERT_EXT) && defined(WOLFSSL_CERT_GEN) */ | ||
|
|
||
| #if defined(WOLFSSL_CERT_GEN) && defined(WOLFSSL_ALT_NAMES) && \ | ||
| defined(WOLFSSL_ASN_TEMPLATE) && \ | ||
| (defined(WOLFSSL_TEST_CERT) || defined(OPENSSL_EXTRA) || \ | ||
| defined(OPENSSL_EXTRA_X509_SMALL) || defined(WOLFSSL_PUBLIC_ASN)) | ||
| /* Exercise the public wc_SetDNSEntry() + wc_FlattenAltNames() pair: build an | ||
| * alt-name list and encode it into a GeneralNames SEQUENCE. The order entries | ||
| * land in depends on build config (OPENSSL_EXTRA appends, otherwise prepends), | ||
| * so presence checks are order-independent. Also exercise the | ||
| * wc_SetAltNamesFromList() convenience that encodes straight into a Cert. */ | ||
| WOLFSSL_TEST_SUBROUTINE wc_test_ret_t flattenAltNames_test(void) | ||
| { | ||
| wc_test_ret_t ret = 0; | ||
| DNS_entry* list = NULL; | ||
| Cert* cert = NULL; | ||
| byte out[256]; | ||
| int len; | ||
| /* dNSName "example.com" -> [2] IMPLICIT IA5String */ | ||
| static const byte dnsTlv[] = { | ||
| 0x82, 0x0B, 'e','x','a','m','p','l','e','.','c','o','m' | ||
| }; | ||
| /* iPAddress 10.0.0.7 -> [7] IMPLICIT OCTET STRING */ | ||
| static const byte ipTlv[] = { 0x87, 0x04, 0x0A, 0x00, 0x00, 0x07 }; | ||
| static const byte ip[] = { 0x0A, 0x00, 0x00, 0x07 }; | ||
| const int innerSz = (int)sizeof(dnsTlv) + (int)sizeof(ipTlv); /* 19 */ | ||
| const int expSz = 2 + innerSz; /* 0x30,len + body */ | ||
| int i, foundDns = 0, foundIp = 0; | ||
|
|
||
| WOLFSSL_ENTER("flattenAltNames_test"); | ||
|
|
||
| /* A NULL list encodes to nothing. */ | ||
| len = wc_FlattenAltNames(out, sizeof(out), NULL); | ||
| if (len != 0) | ||
| ret = WC_TEST_RET_ENC_EC(len); | ||
|
|
||
| if (ret == 0) { | ||
| ret = wc_SetDNSEntry(HEAP_HINT, "example.com", 11, ASN_DNS_TYPE, &list); | ||
| if (ret != 0) | ||
| ret = WC_TEST_RET_ENC_EC(ret); | ||
| } | ||
| if (ret == 0) { | ||
| ret = wc_SetDNSEntry(HEAP_HINT, (const char*)ip, (int)sizeof(ip), | ||
| ASN_IP_TYPE, &list); | ||
| if (ret != 0) | ||
| ret = WC_TEST_RET_ENC_EC(ret); | ||
| } | ||
| if (ret == 0) { | ||
| len = wc_FlattenAltNames(out, sizeof(out), list); | ||
| if (len != expSz) | ||
| ret = WC_TEST_RET_ENC_EC(len); | ||
| } | ||
| if (ret == 0 && (out[0] != ASN_SEQUENCE + ASN_CONSTRUCTED || | ||
| out[1] != (byte)innerSz)) | ||
| ret = WC_TEST_RET_ENC_NC; | ||
| /* Both GeneralName TLVs must be present, regardless of order. */ | ||
| for (i = 0; ret == 0 && i + (int)sizeof(dnsTlv) <= len; i++) { | ||
| if (XMEMCMP(out + i, dnsTlv, sizeof(dnsTlv)) == 0) | ||
| foundDns = 1; | ||
| } | ||
| for (i = 0; ret == 0 && i + (int)sizeof(ipTlv) <= len; i++) { | ||
| if (XMEMCMP(out + i, ipTlv, sizeof(ipTlv)) == 0) | ||
| foundIp = 1; | ||
| } | ||
| if (ret == 0 && (!foundDns || !foundIp)) | ||
| ret = WC_TEST_RET_ENC_NC; | ||
| /* NULL output is rejected. */ | ||
| if (ret == 0) { | ||
| len = wc_FlattenAltNames(NULL, sizeof(out), list); | ||
| if (len != WC_NO_ERR_TRACE(BAD_FUNC_ARG)) | ||
| ret = WC_TEST_RET_ENC_EC(len); | ||
| } | ||
| /* Output one byte too small is rejected with BUFFER_E. */ | ||
| if (ret == 0) { | ||
| len = wc_FlattenAltNames(out, (word32)expSz - 1, list); | ||
| if (len != WC_NO_ERR_TRACE(BUFFER_E)) | ||
| ret = WC_TEST_RET_ENC_EC(len); | ||
| } | ||
|
|
||
| /* wc_SetAltNamesFromList() encodes the same list straight into a Cert and | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟠 [Medium] Test does not cover documented NULL-names path of wc_SetAltNamesFromList or wc_SetDNSEntry error paths The new test exercises the success paths, NULL-output, BAD_FUNC_ARG, BUFFER_E, and the NULL-cert path well. However, Fix: Add an assertion that |
||
| * records the length; the result must match the standalone encoding. */ | ||
| if (ret == 0) { | ||
| cert = (Cert*)XMALLOC(sizeof(Cert), HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER); | ||
| if (cert == NULL) | ||
| ret = WC_TEST_RET_ENC_EC(MEMORY_E); | ||
| } | ||
| if (ret == 0) { | ||
| ret = wc_InitCert_ex(cert, HEAP_HINT, devId); | ||
| if (ret != 0) | ||
| ret = WC_TEST_RET_ENC_EC(ret); | ||
| } | ||
| if (ret == 0) { | ||
| ret = wc_SetAltNamesFromList(cert, list); | ||
| if (ret != 0) | ||
| ret = WC_TEST_RET_ENC_EC(ret); | ||
| } | ||
| if (ret == 0 && (cert->altNamesSz != expSz || | ||
| XMEMCMP(cert->altNames, out, (size_t)expSz) != 0)) | ||
| ret = WC_TEST_RET_ENC_NC; | ||
| /* NULL cert is rejected. */ | ||
| if (ret == 0) { | ||
| int r = wc_SetAltNamesFromList(NULL, list); | ||
| if (r != WC_NO_ERR_TRACE(BAD_FUNC_ARG)) | ||
| ret = WC_TEST_RET_ENC_EC(r); | ||
| } | ||
|
|
||
| XFREE(cert, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER); | ||
| FreeAltNames(list, HEAP_HINT); | ||
| return ret; | ||
| } | ||
| #endif /* WOLFSSL_CERT_GEN && WOLFSSL_ALT_NAMES && WOLFSSL_ASN_TEMPLATE && | ||
| * (WOLFSSL_TEST_CERT || OPENSSL_EXTRA || OPENSSL_EXTRA_X509_SMALL || | ||
| * WOLFSSL_PUBLIC_ASN) */ | ||
|
|
||
| #define RSA_TEST_BYTES (RSA_MAX_SIZE / 8) | ||
|
|
||
| #if !defined(NO_ASN) && !defined(WOLFSSL_RSA_PUBLIC_ONLY) && \ | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2390,6 +2390,11 @@ WOLFSSL_LOCAL int StreamOctetString(const byte* inBuf, word32 inBufSz, | |
| WOLFSSL_ASN_API void FreeAltNames(DNS_entry* altNames, void* heap); | ||
| WOLFSSL_ASN_API DNS_entry* AltNameNew(void* heap); | ||
| WOLFSSL_ASN_API DNS_entry* AltNameDup(DNS_entry* from, void* heap); | ||
| #if defined(WOLFSSL_ASN_TEMPLATE) && \ | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔵 [Low] API availability mismatch: wc_SetDNSEntry can build a list that cannot be flattened in some build configs
Fix: Consider aligning the guards (or documenting the gap) so the wc_SetDNSEntry -> wc_FlattenAltNames flow advertised in the doxygen is available together. Mirroring the internal availability is acceptable, but a short note in the docs would avoid confusion. |
||
| (defined(WOLFSSL_CERT_GEN) || !defined(NO_CERTS)) | ||
| WOLFSSL_ASN_API int wc_SetDNSEntry(void* heap, const char* str, int strLen, | ||
| int type, DNS_entry** entries); | ||
| #endif | ||
| #ifndef IGNORE_NAME_CONSTRAINTS | ||
| WOLFSSL_ASN_API void FreeNameSubtrees(Base_entry* names, void* heap); | ||
| #endif /* IGNORE_NAME_CONSTRAINTS */ | ||
|
|
@@ -2677,6 +2682,11 @@ WOLFSSL_API int wc_DhPublicKeyDecode(const byte* input, word32* inOutIdx, | |
| #endif | ||
| WOLFSSL_LOCAL int FlattenAltNames(byte* output, word32 outputSz, | ||
| const DNS_entry* names); | ||
| #if defined(WOLFSSL_CERT_GEN) && defined(WOLFSSL_ALT_NAMES) | ||
| WOLFSSL_ASN_API int wc_FlattenAltNames(byte* output, word32 outputSz, | ||
| const DNS_entry* names); | ||
| WOLFSSL_ASN_API int wc_SetAltNamesFromList(Cert* cert, const DNS_entry* names); | ||
| #endif | ||
|
|
||
| WOLFSSL_LOCAL int wc_EncodeName(EncodedName* name, const char* nameStr, | ||
| char nameType, byte type); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🟠 [Medium] New public wc_SetDNSEntry() does no argument validation, unlike its sibling wrappers
This PR promotes the previously
staticinternalSetDNSEntry()to a public API via a bare pass-through wrapper. The other two wrappers added in this same PR validate their pointer arguments (wc_SetAltNamesFromListcheckscert == NULL, andwc_FlattenAltNames/FlattenAltNameschecksoutput == NULL), butwc_SetDNSEntryperforms no validation at all. Because it now accepts caller-supplied values from outside the library, malformed inputs reachSetDNSEntry()directly: a negativestrLenflows intoXMALLOC((size_t)strLen + 1, ...)andXMEMCPY(dnsEntry_name, str, (size_t)strLen)where the negative value casts to a hugesize_t(out-of-bounds copy / crash); a NULLentriesis dereferenced inAddDNSEntryToList(*lst...); and a NULLstrwith non-zerostrLencauses a NULL read inXMEMCPY. The doxygen for this function does not document anyBAD_FUNC_ARGreturn for these cases. wolfSSL convention is for publicwc_functions to return a non-void error and validate inputs at the boundary.Fix: Add boundary validation (NULL
str, NULLentries, negativestrLen) in the public wrapper and document theBAD_FUNC_ARGreturn in the doxygen, to be consistent with the other two wrappers added in this PR and with wolfSSL public-API conventions.