Conversation
|
certNameSubject[] is indexed by id - 3 and holds 20 entries, covering DN tag ids 3 through 22. The four entries WOLFSSL_CERT_NAME_ALL appends are labelled name, givenName, initials and dnQualifier - ids 41, 42, 43 and 46 - but they sit at array positions that make them ids 19, 20, 21 and 22. Two consequences. GetRDN() finds no table entry for the real ids, leaves typeStr NULL and never calls SetSubject(), so subjectN, subjectGN, subjectI and subjectDNQ are never populated and those RDNs are dropped from a parsed certificate or CSR. And a certificate carrying 2.5.4.19 through 2.5.4.22 - physicalDeliveryOfficeName, telephoneNumber, teletexTerminalIdentifier, x121Address - matches those entries instead and is decoded under the wrong name, including into the wrong DecodedCert field and the wrong X509_NAME NID. Drop the four misplaced entries and give the ids their own branches in GetRDN() and SetSubject(), beside the ASN_X500_UNIQUE_ID branch already there for a component whose id falls outside the contiguous range. Without WOLFSSL_CERT_NAME_ALL the table already stopped at id 18, so ids 19 to 22 go back to being unrecognised there, as they were. Add test_wc_CertNameAllSubject(), which generates a self-signed certificate carrying all four RDNs and asserts they come back out of wc_ParseCert() in the matching DecodedCert fields and in the subject string. Nothing covered these paths before: no certificate under certs/ carries any of the four OIDs, and no test referenced the fields. The test fails against the pre-fix decoder. The WOLFSSL_NAME, WOLFSSL_GIVEN_NAME, WOLFSSL_INITIALS and WOLFSSL_DNQUALIFIER macros GetRDN() needs are already defined in asn.h under the same guard, so no new definitions are required.
Frauschi
force-pushed
the
fix-cert-name-all-subject-decode
branch
from
September 14, 2026 06:27
c06fb93 to
49da941
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
certNameSubject[]is indexed byid - 3and, withoutWOLFSSL_CERT_NAME_ALL, covers DN tag ids 3 through 18. The four entriesWOLFSSL_CERT_NAME_ALLappends are labelled name, givenName, initials and dnQualifier - ids 41, 42, 43 and 46 - but appending them puts them at positions that make them ids 19, 20, 21 and 22.Two consequences, in opposite directions:
GetRDN()finds no entry for the real ids, leavestypeStrNULL and never callsSetSubject(), sosubjectN,subjectGN,subjectIandsubjectDNQare never populated.physicalDeliveryOfficeName,telephoneNumber,teletexTerminalIdentifier,x121Address- match those entries instead, landing in the wrongDecodedCertfield, under the wrong tag string, with the wrongX509_NAMENID.Affects
WOLFSSL_CERT_NAME_ALLbuilds:--enable-alland the openvpn, nginx, haproxy, hitch, bind and rsyslog recipes.Fix
Drop the four misplaced entries and give the ids their own branches in
GetRDN()andSetSubject(), beside theASN_X500_UNIQUE_IDbranch already there for a component outside the contiguous range. The tag-string macros are already defined inasn.hunder the same guard. WithoutWOLFSSL_CERT_NAME_ALLthe table already stopped at id 18, so ids 19 to 22 go back to being unrecognised there, as they were.Testing
Adds
test_wc_CertNameAllSubject()to theasngroup: generates a self-signed cert carrying all four RDNs, re-parses it, and asserts each lands in the matchingDecodedCertfield and appears indCert.subject. Nothing covered these paths before - no cert undercerts/carries any of the four OIDs, and no test referenced the fields.make checkgreen under--enable-all, and the new test fails whenasn.cis restored from master, so it pins the indexing rather than passing vacuously.Note for the reviewer
The subject string uses
/N=and/GN=, matching the legacyasn_orig.cdecoder, whileRebuildFullName()regenerates it fromobj->sName("name","givenName"). That divergence is pre-existing and mirrors theSNvssurnamesplit inssl_asn1.c, but this change makes it reachable in template builds for the first time. Left alone since aligning them is a user-visible output change; happy to do it separately.