fix(esp-wolfssl): send SNI and skip OCSP when skip_common_name is set#30
Open
hrushikesh430 wants to merge 2 commits into
Open
fix(esp-wolfssl): send SNI and skip OCSP when skip_common_name is set#30hrushikesh430 wants to merge 2 commits into
hrushikesh430 wants to merge 2 commits into
Conversation
Migrated from esp-idf PR #14684. When esp_tls_cfg_t.skip_common_name is set, peer-certificate hostname verification is disabled, but: - SNI must still be sent so virtual-host / CDN servers return the certificate for the requested host. Previously SNI was set only inside the !skip_common_name branch, so it was wrongly skipped along with the hostname check. - OCSP status checking is now skipped too, so the relaxed-verification request is not defeated by an OCSP failure. Verified on ESP32-C5: a client with skip_common_name=true connecting to an SNI-dependent host (www.howsmyssl.com) completes the handshake, which only succeeds if SNI was sent so the chain verifies against the provided root CA.
Revert the OCSP part of the skip_common_name change. OCSP revocation checking is identified by the certificate's issuer + serial number (RFC 6960 sec 4.1.1), not by the server hostname/CN, so it is independent of common-name matching and does not 'fail because the CN does not match'. OCSP remains governed solely by CONFIG_WOLFSSL_HAVE_OCSP. The SNI change is kept: SNI (RFC 6066) selects which certificate the server returns and is independent of hostname verification, matching common TLS client behaviour (e.g. curl -k still sends SNI).
Collaborator
Author
|
@mahavirj @AdityaHPatwardhan PTAL. |
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
1 participant
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.
Background
Migrated from esp-idf PR #14684, which proposed two behaviour changes to the wolfSSL backend for
esp_tls_cfg_t.skip_common_name. After review, only the SNI change is kept; the OCSP change is dropped.✅ Kept: send SNI even when
skip_common_nameis setWhat: when
skip_common_name = true, still send the SNI extension (the server hostname) so the server returns the certificate for the requested host.Why it's correct:
curl -k/--insecurestill sends SNI based on the URL host.SSL_set_tlsext_host_name()(SNI) vsX509_VERIFY_PARAM_set1_host()(verification) — https://docs.openssl.org/master/man3/SSL_set_tlsext_host_name/wolfSSL_UseSNI()vswolfSSL_check_domain_name()), so we can send SNI without doing the CN check.skip_common_namedisables SNI too (esp_tls.h L201-208); this PR keeps SNI on for wolfSSL, so the two backends differ here — an intentional, standards-aligned choice.❌ Dropped: skipping OCSP when
skip_common_nameis setThe original PR also disabled OCSP status checking when
skip_common_nameis set, reasoning that "OCSP checks cannot succeed when the server's domain name does not match its certificate's CN." That reasoning does not hold at the protocol level, so the change is dropped:CertID), with no reference to the server hostname/CN.CONFIG_WOLFSSL_HAVE_OCSP(off by default). Apps that don't want OCSP simply leave it disabled.Net change
A single hunk in
port/esp_tls_wolfssl.c: whenskip_common_nameis set, send SNI (with a NULL/length guard) instead of skipping the whole block. OCSP behaviour is unchanged from upstream.