Use wolfSSL's EmbedOcspLookup in non-blocking async OCSP example - #604
Use wolfSSL's EmbedOcspLookup in non-blocking async OCSP example#604night1rider wants to merge 3 commits into
Conversation
wolfSSL-Fenrir-bot
left a comment
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #604
Scan targets checked: wolfssl-examples-bugs, wolfssl-examples-src
Findings: 1
1 finding(s) posted as inline comments (see file-level comments below)
This review was generated automatically by Fenrir. Findings are non-blocking.
c7c49f3 to
8d7c242
Compare
wolfSSL-Fenrir-bot
left a comment
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #604
Scan targets checked: wolfssl-examples-bugs, wolfssl-examples-src
Findings: 1
1 finding(s) posted as inline comments (see file-level comments below)
This review was generated automatically by Fenrir. Findings are non-blocking.
8d7c242 to
4011953
Compare
wolfSSL-Fenrir-bot
left a comment
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #604
Scan targets checked: wolfssl-examples-bugs, wolfssl-examples-src
Findings: 1
1 finding(s) posted as inline comments (see file-level comments below)
This review was generated automatically by Fenrir. Findings are non-blocking.
wolfSSL-Fenrir-bot
left a comment
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #604
Scan targets checked: wolfssl-examples-bugs, wolfssl-examples-src
Findings: 1
1 finding(s) posted as inline comments (see file-level comments below)
This review was generated automatically by Fenrir. Findings are non-blocking.
There was a problem hiding this comment.
Pull request overview
This PR hardens the non-blocking async OCSP example by removing execution of an untrusted OCSP responder URL via system()/curl, instead using wolfSSL’s built-in OCSP HTTP fetch API, and updates server connection setup to resolve the hostname dynamically via getaddrinfo().
Changes:
- Replace
system("curl ... <untrusted-url>")OCSP fetching withEmbedOcspLookup()+EmbedOcspRespFree(). - Resolve
SERVER_NAMEviagetaddrinfo()instead of using a hard-coded IP address. - Update README sample output to remove the printed curl command flow.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| ocsp/ocsp_nonblock/ocsp_nonblock_async.c | Switch OCSP callback to EmbedOcspLookup() and resolve server address via getaddrinfo(). |
| ocsp/ocsp_nonblock/README.md | Remove curl-command/temporary-file output lines from the example run log. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Overall this looks good and an improvement over the original system(3) based example.
Two requested changes:
- While you're updating the
README.mdwe should remove this outdated section:
NOTE: Before building this example the asynchronous support must be obtained from (https://github.com/wolfSSL/wolfAsyncCrypt) and installed into wolfSSL by following the instructions in the README file contained in the wolfAsyncCrypt repository.
The wolfAsyncCrypt repo is deprecated, and the async code is upstreamed into wolfssl now.
- It would be nice to just include a cert to use for this
ocsp_nonblock_asyncexample.
note: the google.pem used by ocsp_nonblock.c example is expired, but that's outside scope of this PR.
wolfSSL-Fenrir-bot
left a comment
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #604
Scan targets checked: wolfssl-examples-bugs, wolfssl-examples-src
Fenrir result: Approved ✅
No new issues found in the changed files.
Advisory only — this automated result does not count as a GitHub approval.
8edbabb to
de2be79
Compare
wolfSSL-Fenrir-bot
left a comment
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #604
Scan targets checked: wolfssl-examples-bugs, wolfssl-examples-src
Findings: 3
3 finding(s) posted as inline comments (see file-level comments below)
This review was generated automatically by Fenrir. Reported findings require changes before merge.
de2be79 to
770dfcd
Compare
wolfSSL-Fenrir-bot
left a comment
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #604
Scan targets checked: wolfssl-examples-bugs, wolfssl-examples-src
Findings: 1
1 finding(s) posted as inline comments (see file-level comments below)
This review was generated automatically by Fenrir. Reported findings require changes before merge.
3425718 to
25ff2ff
Compare
wolfSSL-Fenrir-bot
left a comment
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #604
Scan targets checked: wolfssl-examples-bugs, wolfssl-examples-src
Findings: 2
2 finding(s) posted as inline comments (see file-level comments below)
This review was generated automatically by Fenrir. Reported findings require changes before merge.
Replace the system("curl ... <untrusted responder URL>") OCSP fetch in the async
example with wolfSSL's EmbedOcspLookup, so the certificate-supplied URL is fetched
as data over a socket instead of executed.
Both examples now target www.digicert.com (Google dropped OCSP): ocsp_nonblock
fetches the leaf live over TLS rather than a bundled cert that expires, and the
async example uses wolfSSL's native async API. Bundle the DigiCert CAs and drop
the expired google.pem and stale GTS certs.
The rewritten clients need asynccrypt/opensslextra/sni/alpn and WOLFSSL_NONBLOCK_OCSP, which the shared ocsp profile lacks; add a dedicated ocsp-nonblock profile.
25ff2ff to
a4d68f7
Compare
wolfSSL-Fenrir-bot
left a comment
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #604
Scan targets checked: wolfssl-examples-bugs, wolfssl-examples-src
Findings: 2
2 finding(s) posted as inline comments (see file-level comments below)
This review was generated automatically by Fenrir. Reported findings require changes before merge.
| return NULL; | ||
| } | ||
|
|
||
| ret = wolfSSL_connect(ssl); |
There was a problem hiding this comment.
TLS certificate fetch is not covered by the configured timeout · Logic errors
DEFAULT_TIMEOUT_SEC applies only to wolfIO_TcpConnect; wolfSSL_connect has no deadline, so a peer that accepts TCP but stalls TLS blocks indefinitely. Unlike known #12575, this is a handshake timeout, not an OCSP socket leak.
Related known finding #12575 (similar but distinct): Both affect the OCSP nonblocking example's connection flow, but this fault is a blocking wolfSSL_connect operation without a handshake deadline; #12575 leaks and reuses the callback's socket descriptor. Their root causes and fixes—deadline-driven handshake retries versus descriptor cleanup/reset—are separate.
Suggested fix: Make the socket and ssl non-blocking, then retry WANT_READ/WANT_WRITE only until DEFAULT_TIMEOUT_SEC expires.
Basis: wolfSSL API documentation states that wolfIO_TcpConnect times out the TCP connection, while blocking wolfSSL_connect returns only after handshake completion or an error.
| /* Whether a failed wolfSSL_connect() should be retried. */ | ||
| static int is_retryable(int errCode) | ||
| { | ||
| if (errCode == SSL_ERROR_WANT_READ) { |
There was a problem hiding this comment.
Nonblocking handshake drops WANT_WRITE retries · Incorrect wolfSSL/wolfCrypt API usage
is_retryable() omits SSL_ERROR_WANT_WRITE, so wolfSSL_connect() treats a documented retry state as fatal and aborts valid handshakes. Distinct from known #4597, this driver drops a retry state rather than returning the wrong callback sentinel.
Suggested fix: Handle SSL_ERROR_WANT_WRITE as retryable and repeat wolfSSL_connect() until the socket is ready or the deadline expires.
Basis: wolfSSL Manual, wolfSSL_connect(): nonblocking I/O can yield SSL_ERROR_WANT_READ or SSL_ERROR_WANT_WRITE, requiring the caller to repeat the call when I/O is ready.
Set SO_RCVTIMEO/SO_SNDTIMEO to the fetch timeout before wolfSSL_connect so a peer that accepts TCP but stalls TLS fails instead of blocking forever, and treat SSL_ERROR_WANT_WRITE as retryable in the async client's handshake loop, per PR wolfSSL#604 review.
Replace the system("curl ... ") OCSP fetch in the async example with wolfSSL's EmbedOcspLookup, so the certificate-supplied URL is fetched as data over a socket instead of executed.
Both examples now target www.digicert.com (Google dropped OCSP): ocsp_nonblock
fetches the leaf live over TLS rather than a bundled cert that expires, and the async example uses wolfSSL's native async API. Bundle the DigiCert CAs and drop the expired google.pem and stale GTS certs.