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.
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.
- clu_client_setup.c takes -verify_hostname <name> and -verify_ip <ip> and forwards them to the inner client. Either one sets verify, leaving out -d; a missing value is rejected. - The client help lists both, and the no-verification warning names all three options that turn verification 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, with ssl and ctx freed and err_sys() called on failure. - server-test.py adds six tests for matching and mismatching names and addresses, each rejection checked for the mismatch wolfSSL reports; client-test.py covers the help entries, a missing value, and the no-verification warning. - wolfssl-s_client.1 documents both options with examples, rewords the -CAfile note, and separates chain verification from the host check. Issue: F-9853
c8bc81c to
fa38c7b
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
Fenrir result: Approved ✅
No new issues found in the changed files.
Advisory only — this automated result does not count as a GitHub approval.
Fenrir's latest completed scan found no issues; clearing the prior automated change request.
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.