ssl.c split: cleanup - #11022
Open
SparkiDev wants to merge 1 commit into
Open
Conversation
|
SparkiDev
force-pushed
the
ssl_c_split_cleanup_1
branch
2 times, most recently
from
July 31, 2026 00:52
7271051 to
8379b37
Compare
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #11022
Scan targets checked: wolfcrypt-rs-bugs, wolfssl-bugs, wolfssl-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.
SparkiDev
force-pushed
the
ssl_c_split_cleanup_1
branch
from
July 31, 2026 01:48
8379b37 to
40bc196
Compare
SparkiDev
commented
Jul 31, 2026
Contributor
Author
|
Jenkins: retest this please |
SparkiDev
force-pushed
the
ssl_c_split_cleanup_1
branch
2 times, most recently
from
July 31, 2026 05:30
847c5a3 to
ce7dfd9
Compare
Frauschi
requested changes
Jul 31, 2026
Frauschi
left a comment
Contributor
There was a problem hiding this comment.
🐺 Skoll Code Review
Overall recommendation: REQUEST_CHANGES
Findings: 21 total — 14 posted, 7 skipped
Posted findings
- [High] New rw and hs tests call wolfTLSv1_2_*_method() without a !WOLFSSL_NO_TLS12 guard - build break in TLS 1.3-only builds —
tests/api/test_ssl_rw.c:82 - [High] wolfSSL_OCSP_parse_url now rejects URLs with an explicit port and a ':' in the path —
src/ssl_api_crl_ocsp.c:1339-1349 - [High] New !sentNotify branch in wolfSSL_shutdown overwrites WOLFSSL_SUCCESS and changes ssl->error on a pre-existing path —
src/ssl_api_rw.c:960-976 - [High] Guard mismatches after the split leave three static functions unused or undefined under -Werror —
src/ssl_api_hs.c:89-92 - [Medium] wolfSSL_writev: small-stack allocation regression, ssl dereferenced before validation, and stale guard indentation —
src/ssl_api_rw.c:1077-1114 - [Medium] None of the three fixes this PR claims is covered by a test that could fail without it —
src/ssl_api_rw.c:965-976 - [Medium] set0_verify_cert_store still leaks the handed-over reference on the sibling early-return path —
src/ssl_api_cert.c:1740-1762 - [Medium] Doxygen blocks in ssl_api_crl_ocsp.c copy-pasted from neighbouring functions describe the wrong parameter —
src/ssl_api_crl_ocsp.c:1172-1177 - [Medium] wolfSSL_UseMaxFragment overloads ret with three incompatible conventions —
src/ssl_api_ext.c:247-278 - [Low] wolfSSL_SendUserCanceled doc omits WOLFSSL_FATAL_ERROR —
doc/dox_comments/header_files/ssl.h:2542-2575 - [Low] UnloadIntermediateCerts NULL check hidden behind an unrelated WOLFSSL_TRUST_PEER_CERT guard —
tests/api/test_ssl_cert.c:1179-1182 - [Low] Leftovers from the ssl_api_hs.c de-indent: an indented preprocessor block and three dead (void) casts —
src/ssl_api_hs.c:1459-1478 - [Low] Comment in _Rehandshake describes the branch that is not taken —
src/ssl_api_ext.c:988-993 - [Info] wolfSSL_get_ocsp_response is now the only unguarded function in a hardened file —
src/ssl_api_crl_ocsp.c:780-793
Skipped findings
- [Medium] ChangeLog.md not updated for the user-visible behaviour changes in this PR
- [Low] Five bare scope blocks introduced in test_ssl_hs.c
- [Low] Three new tests carry comments that claim more than the assertions can prove
- [Low] ALPN test allocates the peer buffer with the wrong dynamic type
- [Low] Handshake state enums moved to file scope pollute the ssl.c amalgamation namespace with very generic names
- [Low] Flush progress messages switch from WOLFSSL_MSG to WOLFSSL_MSG_EX, dropping them where XVSNPRINTF is unavailable
- [Info] Inconsistent backslash alignment in the extended DECLS macros
Review generated by Skoll via Claude/Codex
SparkiDev
force-pushed
the
ssl_c_split_cleanup_1
branch
from
August 2, 2026 20:39
ce7dfd9 to
d002b93
Compare
Clean up includes single return point from every function, consistent formatting function block comment and tests added. ssl_api_rw.c: Cleanup; 6 helpers extracted from write_dup/shutdown; fixed a SendBuffered corner case that returned 0 instead of an error. ssl_api_ext.c: Cleanup; extracted wolfssl_ticket_key_cb_process, wolfssl_rehandshake_prepare. ssl_api_hs.c: Cleanup + de-indent; 9 helpers extracted, incl. shared wolfssl_handshake_flush/_done between connect and accept (~100 duplicated lines removed). wolfSSL_connect/accept left multi-exit. ssl_api_cert.c: Cleanup of newer functions at end of file; extracted PushPeerCertToChain; fixed a double free in CreatePeerCertChain; fixed 5 wrong @param names. ssl_api_crl_ocsp.c: Full cleanup of all 49 functions; wolfSSL_OCSP_parse_url rewritten; fixed a URL with no host returning success with the rest of the URL as the host; added IPv6 literal support; fixed 4 wrong WOLFSSL_ENTER names. ssl.c: Moved the x509GetIssuerFromCM forward declaration here for clarity.
SparkiDev
force-pushed
the
ssl_c_split_cleanup_1
branch
from
August 2, 2026 22:04
d002b93 to
e1ca20b
Compare
Contributor
Author
|
Jenkins: retest this please FIPS aborted |
SparkiDev
requested review from
wolfSSL-Fenrir-bot
and removed request for
wolfSSL-Fenrir-bot
August 3, 2026 01:28
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #11022
Scan targets checked: wolfcrypt-rs-bugs, wolfssl-bugs, wolfssl-src
No new issues found in the changed files. ✅
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
Clean up includes single return point from every function, consistent formatting function block comment and tests added.
ssl_api_rw.c: Cleanup; 6 helpers extracted from write_dup/shutdown; fixed a SendBuffered corner case that returned 0 instead of an error.
ssl_api_ext.c: Cleanup; extracted wolfssl_ticket_key_cb_process, wolfssl_rehandshake_prepare.
ssl_api_hs.c: Cleanup + de-indent; 9 helpers extracted, incl. shared wolfssl_handshake_flush/_done between connect and accept (~100 duplicated lines removed). wolfSSL_connect/accept left multi-exit.
ssl_api_cert.c: Cleanup of newer functions at end of file; extracted PushPeerCertToChain; fixed a double free in CreatePeerCertChain; fixed 5 wrong @param names.
ssl_api_crl_ocsp.c: Full cleanup of all 49 functions; wolfSSL_OCSP_parse_url rewritten; fixed a URL with no host returning success with the rest of the URL as the host; added IPv6 literal support; fixed 4 wrong WOLFSSL_ENTER names.
ssl.c: Moved the x509GetIssuerFromCM forward declaration here for clarity.
Testing
Maximal config.
OpenSSL API regression testing.
Tests added.