s_client: add -verify_hostname and -verify_ip - #293
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
The manpage DESCRIPTION and the IP SAN support probe in the new tests have correctness issues that can mislead users and/or cause unintended test skipping.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds opt-in certificate identity verification to wolfssl s_client (hostname and/or IP) so that a CA-trusted certificate for the wrong identity is rejected, aligning behavior more closely with openssl s_client when explicitly requested.
Changes:
- Add
-verify_hostname <name>and-verify_ip <ip>options in the CLI wrapper and forward them to the inner client. - Implement pre-connect configuration for identity checking via
wolfSSL_check_domain_name()/wolfSSL_check_ip_address()with fail-closed handling. - Add server/client tests covering hostname and IP identity match/mismatch cases, plus help-menu and manpage updates.
File summaries
| File | Description |
|---|---|
| wolfclu/clu_optargs.h | Adds option IDs for the new CLI flags. |
| src/client/clu_client_setup.c | Parses -verify_hostname / -verify_ip, forwards to inner client, and ensures peer verification is not disabled when these are used. |
| src/client/client.c | Adds long options and configures wolfSSL identity checks prior to wolfSSL_connect(), checking return values. |
| tests/server/server-test.py | Adds integration tests exercising hostname/IP identity acceptance and rejection. |
| tests/client/client-test.py | Ensures new options appear in s_client -help. |
| manpages/wolfssl-s_client.1 | Documents new options and clarifies verification vs identity checking. |
Review details
- Files reviewed: 6/6 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
- clu_client_setup.c takes -verify_hostname <name> and -verify_ip <ip> and forwards them to the inner client after the parse loop. Either one sets verify, leaving out -d. - The client help lists both under a shared line saying either one turns on peer verification, and the warning printed when verification is off names all three options that turn it on. - clu_optargs.h gains WOLFCLU_VERIFY_HOSTNAME and WOLFCLU_VERIFY_IP. - client.c gains the --verify_hostname and --verify_ip long options. --verify_hostname keeps its name in a checkDomain of its own, which wolfSSL_check_domain_name() takes in place of domain; that call and the new wolfSSL_check_ip_address() have their returns tested, freeing ssl and ctx and calling err_sys() on failure. - server-test.py adds the _run_identity_check() and _assert_rejected() helpers and six tests for matching and mismatching names and addresses, each rejection checked for the mismatch wolfSSL reports; client-test.py checks both options appear in the help output. - wolfssl-s_client.1 documents both options with examples, rewords the -CAfile note, and separates chain verification from the host check. Issue: F-9853
25b816a to
c8bc81c
Compare
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #293
Scan targets checked: wolfclu-bugs, wolfclu-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.
| break; | ||
|
|
||
| case WOLFCLU_VERIFY_HOSTNAME: | ||
| verifyHost = optarg; |
There was a problem hiding this comment.
-verify_hostname/-verify_ip silently ignored when optarg is NULL · Logic errors
wolfCLU_GetOpt leaves optarg NULL when a required_argument option is the last token (clu_funcs.c:1295-1300). Storing it unchecked makes the later verifyHost != NULL / verifyIp != NULL blocks skip, so verify stays 0, -d is emitted, and the run proceeds unverified while printing that no verify option was given. Adjacent to known finding #8069, which is the NULL-optarg crash in the -connect handler of this function; this branch does not crash and instead disables the requested check.
Related known finding #8069 (similar but distinct): Both are in wolfCLU_Client and stem from required-option optarg being NULL. #8069 dereferences optarg in the -connect handler via XSTRSTR and crashes; this finding assigns optarg in the -verify_hostname/-verify_ip handlers, causing verification to be skipped. Different switch cases and separate validation patches are required.
Fix: Reject with WOLFCLU_FATAL_ERROR and a usage message when optarg is NULL in the WOLFCLU_VERIFY_HOSTNAME and WOLFCLU_VERIFY_IP cases.
Problem
wolfssl s_clientvalidates the certificate chain but never checks that the certificate belongs to the host in-connect.matchNamein the inner client defaults to 0 and is set only by-m, whichwolfCLU_Client()never emits, sowolfSSL_check_domain_name()is unreachable from the CLI. With-CAfile ca.pem -verify_return_error, a CA-trusted certificate issued for any other name passes — an attacker with network position can present a certificate forattacker.exampleon a connection tovictim.exampleand be accepted. Certificate validation bypass. Closes f-9853.Fix (
src/client/clu_client_setup.c)Adds
-verify_hostname <name>and-verify_ip <ip>, followingopenssl s_client, which likewise checks no identity unless told to.-d, so the check cannot be silently skipped.-verify_return_erroris not also needed.--verify_hostname/--verify_ip.In
src/client/client.c:--verify_hostnamewolfSSL_check_domain_name()--verify_ipwolfSSL_check_ip_address()Both returns are tested and fail closed before
wolfSSL_connect(); the pre-existing-mcall discarded its return.--verify_hostnamestores into its owncheckDomainrather than thedomainthat-halso writes, so the connect host cannot displace the requested name.Tests (
tests/server/server-test.py)Six tests against a local
s_serverwithserver-cert.pem(CN=www.wolfssl.com,SAN: DNS:example.com, IP:127.0.0.1):-verify_hostname example.com-verify_hostname attacker.example-verify_ip 127.0.0.1-verify_ipwith10.0.0.1,::1,not-an-ipEvery rejection is checked for the reason wolfSSL reports —
peer subject name mismatch(-322) orpeer ip address mismatch(-325) — so an unready server or a dropped connection cannot satisfy a security test. None pass-verify_return_error, so they also cover each option enabling verification alone.client-test.pychecks both options appear in-help.Verification
make check25/25, 0 skipped.s_clientaccepts the wrong-host certificate and exactly the mismatch tests fail.