Repository navigation
socks: support CURLAUTH_ONLY in CURLOPT_SOCKS5_AUTH - #23003
Closed
MegaManSec wants to merge 1 commit into
Closed
MegaManSec wants to merge 1 commit into
MegaManSec wants to merge 1 commit into
Conversation
MegaManSec
force-pushed
the
socks5-auth-doc-noauth
branch
from
September 18, 2026 22:26
4677194 to
bf64669
Compare
MegaManSec
changed the title
The SOCKS5 method negotiation always offered "no authentication" and accepted it, so the bitmask could not express that the connection has to be authenticated. CURLAUTH_NONE is zero and cannot say it either. With CURLAUTH_ONLY set, the method is neither offered nor accepted, and a connection that did not authenticate is not reused. Document both, and add tests 1742, 1743 and 1744.
MegaManSec
force-pushed
the
socks5-auth-doc-noauth
branch
from
September 18, 2026 22:34
bf64669 to
33cf1f3
Compare
bagder
approved these changes
Sep 19, 2026
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The implementation consistently enforces the documented authentication requirement and includes focused regression coverage.
Review effort: Balanced
Findings: None
What changed in this PR
Adds opt-in enforcement of SOCKS5 authentication through CURLAUTH_ONLY, including safe connection reuse and documentation.
Changes:
- Rejects unauthenticated SOCKS5 negotiation when authentication is required.
- Prevents reuse of unauthenticated SOCKS5 connections.
- Adds coverage for validation, negotiation, and reuse behavior.
| File | Description |
|---|---|
lib/setopt.c |
Accepts and validates CURLAUTH_ONLY. |
lib/socks.c |
Enforces authenticated negotiation. |
lib/url.c |
Restricts connection reuse appropriately. |
lib/urldata.h |
Tracks authentication requirements and connection state. |
docs/libcurl/opts/CURLOPT_SOCKS5_AUTH.md |
Documents the new behavior. |
tests/libtest/lib1742.c |
Tests validation and required authentication. |
tests/libtest/lib1744.c |
Tests unauthenticated connection reuse prevention. |
tests/libtest/Makefile.inc |
Registers new libtests. |
tests/data/test1742 |
Covers excluding no-auth from negotiation. |
tests/data/test1743 |
Covers rejecting proxy-selected no-auth. |
tests/data/test1744 |
Covers reuse behavior. |
tests/data/Makefile.am |
Registers new test cases. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
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.
socks5_req0_init()always advertises method 0 andsocks5_check_resp0()always accepts it, so a SOCKS5 proxy can select "no authentication" whatever the bitmask says. The bitmask cannot express "authentication required" either, sinceCURLAUTH_NONEis zero:CURLAUTH_BASIC | CURLAUTH_NONEis justCURLAUTH_BASIC. That matters most for GSS-API, which asks forGSS_C_MUTUAL_FLAG: answering method 0 strips the proxy's authentication to the client.CURLAUTH_ONLYis currently rejected by setopt for this option, so accepting it cannot change any existing application. When set:CURLE_PROXYurl_match_proxy_use()otherwise matches on proxy type, destination and credentials only, and the handshake does not re-run on reuseCURLAUTH_ONLYwithoutCURLAUTH_BASICorCURLAUTH_GSSAPIreturnsCURLE_BAD_FUNCTION_ARGUMENTThe man page is updated to say that no-auth is otherwise always offered and accepted, which it did not before.
Tests: 1742 has the test proxy require exactly one advertised method, 1743 has it answer method 0, 1744 does an unauthenticated transfer first and then requires auth on the same handle. Each fails without the change. Tests 700-799 pass.