From 68d5be36a0ead88af0fbc6a3f2c3e3c0fd115c52 Mon Sep 17 00:00:00 2001 From: "glm-5.3-flash" Date: Thu, 24 Sep 2026 18:20:31 +0000 Subject: [PATCH] rfc1928: the keep-or-cut sweep + the honest refusal vocabulary (review-002 N-1, N-4) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - N-1 cut: auth_method_enums! (macro + generated StandardAuthentication/ StandardAuthenticationStarted/allow_no_auth), skip_auth_this_is_not_ rfc_compliant, CheckResult's Option/Result impls (bool stays) — the producer-replaced machinery; ADR-013's no-speculative-surface posture - N-1 keep-notes made current: ConnectError (hand-port companion; the fired trigger + keep disposition recorded in stream.rs), transfer/wait_on_tcp (differential-suite-consumed), Bug (upstream- verbatim, display intact) - N-4: new SocksServerError::Refused(&'static str) — the honest log vocabulary for the five in-stream policy refusal sites (the BIND command gate + dial/egress/bind-factory/bind-accept seams) and the harness's minimal producer; wire bytes untouched (Bug keeps its display; AuthenticationRejected stays the typestate's auth-reject) - Provenance extended, not replaced (convention 17): server.rs module doc's owned-changes paragraph + rfc1928.md's owned-code post-fork changes list + last_updated - Differential :103 comment reworded to the RFC's ordering rationale; AuthMethod doc updated for the macro cut - Tests: refusal-site session-error asserts moved to Refused, the mapping-table loop grew the variant, a display/shape test pins it Verified: cargo test (356 green incl. --all-features), clippy/fmt/doc clean, both wasm checks clean. --- docs/architecture/rfc1928.md | 39 ++++- src/producer.rs | 34 ++-- src/rfc1928/server.rs | 171 +++++++------------- src/rfc1928/util/stream.rs | 9 +- tasks/implementation/fix-rfc1928-posture.md | 97 +++++++++-- tests/harness.rs | 2 +- tests/rfc1928-differential.rs | 6 +- 7 files changed, 211 insertions(+), 147 deletions(-) diff --git a/docs/architecture/rfc1928.md b/docs/architecture/rfc1928.md index fdf46ae..bfedbf3 100644 --- a/docs/architecture/rfc1928.md +++ b/docs/architecture/rfc1928.md @@ -1,10 +1,12 @@ --- status: draft -last_updated: 2026-09-22 (the targeted fork — ADR-013 supersedes -ADR-011's maintenance posture; the module is `src/rfc1928/`, the -fork point is `5b18d4d`, the lint-allow posture dissolves; -extraction review F-1: `ReplyError::from_u8`'s panic arm mapped to -`GeneralFailure`; Phase 1 initial draft 2026-09-20) +last_updated: 2026-09-24 (the keep-or-cut sweep + the refusal +vocabulary variant — the fork's owned post-fork changes section; +review 002 N-1/N-4; the targeted fork 2026-09-22 — ADR-013 +supersedes ADR-011's maintenance posture; the module is +`src/rfc1928/`, the fork point is `5b18d4d`, the lint-allow posture +dissolves; extraction review F-1: `ReplyError::from_u8`'s panic arm +mapped to `GeneralFailure`; Phase 1 initial draft 2026-09-20) --- # alksocks — The RFC 1928 Core (the targeted fork) @@ -113,6 +115,33 @@ four dissolve with the kernel-net region: Dissolved: the `net` feature gate (no net code in-tree) and the `run_udp_proxy_custom` bind seam (the handler isn't forked). +Plus the **owned-code post-fork changes** (the fork is first-class +code — extend this list, never replace it; convention 17): + +- **The keep-or-cut sweep** (review 002 N-1): the producer-replaced + machinery is cut from the typestate — the `auth_method_enums!` + macro (upstream's enum generator) with its generated + `StandardAuthentication` enum + `allow_no_auth`, + `skip_auth_this_is_not_rfc_compliant`, and `CheckResult`'s + `Option`/`Result` impls (`bool` stays; re-add with a + typed-rejection design if a door ever wants it). None had an + in-tree consumer once the producer drove `negotiate_auth` directly + (`&[NoAuthentication]`; per-door auth is ADR-005's, not the + typestate's). Kept with note (the hand-port map, ADR-013): + `ConnectError` (constructed nowhere in-tree; the dial-error → + reply-code mapping a future kernel-net port reuses) and + `transfer`/`wait_on_tcp` (differential-suite-consumed — their + semantics stay pinned there). The upstream `Bug` variant is + likewise kept, its display intact. +- **The refusal vocabulary variant** (review 002 N-4): + `SocksServerError::Refused(&'static str)` — the honest + log/observability vocabulary for the producer's in-stream policy + refusals (the command gate + the four seam-refusal sites). Upstream + has no such variant; it is an owned addition, never reaching the + wire (the RFC reply goes out first; the variant's + `to_reply_error` mapping is the `GeneralFailure` fall-through, + pinned by the mapping-table test). + Plus the provenance notes: the fork point (upstream v1.0.0, extraction `57b107b..5b18d4d`, fork at the review commit) and the license notice (MIT, extended — ADR-013's license mechanics). diff --git a/src/producer.rs b/src/producer.rs index a1a9960..6ebb4c8 100644 --- a/src/producer.rs +++ b/src/producer.rs @@ -228,11 +228,15 @@ where // The command gate (ADR-006): BIND mints inbound // listeners — an identity with proxy egress must not // imply ingress. RFC-faithful refusal (the command is - // known, the caller is not allowed). + // known, the caller is not allowed). The session + // error is the log/observability vocabulary (the wire + // got its reply already) — `Refused`, the honest + // shape (review 002 N-4; an ACL refusal is not an + // auth failure). proto.reply_error(&ReplyError::ConnectionNotAllowed).await?; - return Err(SocksError::ServerError( - SocksServerError::AuthenticationRejected, - )); + return Err(SocksError::ServerError(SocksServerError::Refused( + "bind command gate: not allowed", + ))); } serve_tcp_bind(proto, target, accept).await } @@ -262,7 +266,7 @@ where Ok(handle) => handle, Err(err) => { proto.reply_error(&err.to_reply_error()).await?; - return Err(SocksError::ServerError(SocksServerError::Bug( + return Err(SocksError::ServerError(SocksServerError::Refused( "dial refused in-stream", ))); } @@ -302,7 +306,7 @@ where Ok(egress) => egress, Err(err) => { proto.reply_error(&err.to_reply_error()).await?; - return Err(SocksError::ServerError(SocksServerError::Bug( + return Err(SocksError::ServerError(SocksServerError::Refused( "egress refused in-stream", ))); } @@ -495,7 +499,7 @@ where Ok(accept) => accept, Err(err) => { proto.reply_error(&err.to_reply_error()).await?; - return Err(SocksError::ServerError(SocksServerError::Bug( + return Err(SocksError::ServerError(SocksServerError::Refused( "bind factory refused in-stream", ))); } @@ -553,7 +557,7 @@ where .write_all(&new_reply(&err.to_reply_error(), sentinel_bind_peer_addr())) .await; let _ = inner.flush().await; - return Err(SocksError::ServerError(SocksServerError::Bug( + return Err(SocksError::ServerError(SocksServerError::Refused( "bind accept refused in-stream", ))); } @@ -919,7 +923,7 @@ mod tests { .expect_err("the session surfaces the refusal in-stream"); assert!(matches!( err, - SocksError::ServerError(SocksServerError::Bug("dial refused in-stream")) + SocksError::ServerError(SocksServerError::Refused("dial refused in-stream")) )); } @@ -1524,7 +1528,7 @@ mod tests { .expect_err("the refusal surfaces in-stream"); assert!(matches!( err, - SocksError::ServerError(SocksServerError::Bug("egress refused in-stream")) + SocksError::ServerError(SocksServerError::Refused("egress refused in-stream")) )); } @@ -1873,7 +1877,7 @@ mod tests { .expect_err("the refusal surfaces in-stream"); assert!(matches!( err, - SocksError::ServerError(SocksServerError::AuthenticationRejected) + SocksError::ServerError(SocksServerError::Refused("bind command gate: not allowed")) )); // CONNECT still serves the same identity (egress unaffected). @@ -2026,7 +2030,7 @@ mod tests { .expect_err("the refusal surfaces in-stream"); assert!(matches!( err, - SocksError::ServerError(SocksServerError::Bug("bind factory refused in-stream")) + SocksError::ServerError(SocksServerError::Refused("bind factory refused in-stream")) )); } @@ -2065,7 +2069,7 @@ mod tests { .expect_err("the refusal surfaces in-stream"); assert!(matches!( err, - SocksError::ServerError(SocksServerError::Bug("bind accept refused in-stream")) + SocksError::ServerError(SocksServerError::Refused("bind accept refused in-stream")) )); } @@ -2334,7 +2338,7 @@ mod tests { .expect_err("the refusal surfaces in-stream"); assert!(matches!( err, - SocksError::ServerError(SocksServerError::AuthenticationRejected) + SocksError::ServerError(SocksServerError::Refused("bind command gate: not allowed")) )); // CONNECT still serves the same producer (egress unaffected). @@ -2471,7 +2475,7 @@ mod tests { .expect_err("the refusal surfaces in-stream"); assert!(matches!( err, - SocksError::ServerError(SocksServerError::AuthenticationRejected) + SocksError::ServerError(SocksServerError::Refused("bind command gate: not allowed")) )); } diff --git a/src/rfc1928/server.rs b/src/rfc1928/server.rs index 3c339be..a37d5ef 100644 --- a/src/rfc1928/server.rs +++ b/src/rfc1928/server.rs @@ -16,15 +16,28 @@ //! inventory row 1 in `docs/architecture/rfc1928.md`. //! //! Extraction-time adjustments (mechanical, ADR-011 §Amendment): -//! `log` calls dropped; `anyhow::Context` dropped; the -//! `#[macro_export]` on `auth_method_enums!` converted to `pub` -//! scoping within this module (same adjustment as `read_exact!`); -//! `reply_error`'s `"0.0.0.0:0".parse().unwrap()` replaced with a -//! `const` parsed at compile time (no `unwrap` outside tests). -//! The upstream `Config`/`Authentication` impls that consumed +//! `log` calls dropped; `anyhow::Context` dropped; `reply_error`'s +//! `"0.0.0.0:0".parse().unwrap()` replaced with a `const` parsed at +//! compile time (no `unwrap` outside tests). The upstream +//! `Config`/`Authentication` impls that consumed //! `SocksServerError::AuthenticationRejected` are cut; the variant //! stays (the typestate's `accept_password_auth` produces it). //! +//! Post-fork owned changes (extend, never replace — convention 17): +//! the keep-or-cut sweep (review 002 N-1) cut the producer-replaced +//! machinery — the `auth_method_enums!` macro (upstream's enum +//! generator) with its generated `StandardAuthentication` enum + +//! `allow_no_auth`, `skip_auth_this_is_not_rfc_compliant`, and +//! `CheckResult`'s `Option`/`Result` impls (`bool` stays) — none had +//! an in-tree consumer once the producer drove `negotiate_auth` +//! directly. Kept with note (the hand-port map, ADR-013): +//! `transfer`/`wait_on_tcp` are differential-suite-consumed (their +//! semantics stay pinned there), and the error vocabulary keeps its +//! upstream variants. One variant was added, not absorbed: +//! `SocksServerError::Refused` — the honest refusal vocabulary for +//! the producer's in-stream policy refusals (review 002 N-4; never +//! reaches the wire). Recorded in docs/architecture/rfc1928.md. +//! //! The BIND note (ADR-007): BIND's driving is producer-side code — //! the typestate already returns `TCPBind` from `read_command()`, //! `reply_success(sock_addr)` carries reply#1's BND.ADDR, and @@ -71,6 +84,8 @@ pub enum SocksServerError { AddrError(#[from] AddrError), #[error("BUG: {0}")] // should be unreachable Bug(&'static str), + #[error("Refused: {0}")] + Refused(&'static str), #[error("Auth method unacceptable `{0:?}`.")] AuthMethodUnacceptable(Vec), #[error("Unsupported SOCKS version `{0}`.")] @@ -142,13 +157,11 @@ impl Socks5ServerProtocol { } /// Closure-result adapter for [`Socks5ServerProtocol -/// ::accept_password_auth`]'s credential checker: `bool`, -/// `Option`, and `Result` all work (`is_good`). In-tree -/// callers use `bool` only; the `Option`/`Result` impls anticipate -/// the producer layer's username/password mapping (typed rejections -/// want `Result`-returning checkers — review 001 N-3). Same -/// keep-or-cut decision as `StandardAuthentication` when the producer -/// layer lands. +/// ::accept_password_auth`]'s credential checker: `bool` (`is_good`). +/// The `Option`/`Result` impls upstream-adjacent shape were cut in +/// the keep-or-cut sweep (review 002 N-1) — every in-tree checker +/// returns `bool`; re-add with a typed-rejection design if a door +/// ever wants it. pub trait CheckResult { fn is_good(&self) -> bool; } @@ -159,18 +172,6 @@ impl CheckResult for bool { } } -impl CheckResult for Option { - fn is_good(&self) -> bool { - self.is_some() - } -} - -impl CheckResult for Result { - fn is_good(&self) -> bool { - self.is_ok() - } -} - impl Socks5ServerProtocol { /// Finish handling the authentication method-specific part of the protocol, /// returning back to the overall SOCKS5 flow. @@ -178,14 +179,6 @@ impl Socks5ServerProtocol { Self::new(auth.into_inner()) } - /// Wrap a socket in a SOCKS5 flow handler that's already marked as authenticated. - /// - /// This is not actually part of the official SOCKS5 protocol, but allows you to - /// only use the post-authentication subset of it. - pub fn skip_auth_this_is_not_rfc_compliant(inner: T) -> Self { - Self::new(inner) - } - /// Handle the SOCKS5 auth negotiation supporting only the `NoAuthentication` method. pub async fn accept_no_auth(inner: T) -> Result where @@ -244,8 +237,11 @@ pub trait AuthMethodSuccessState { /// A metadata trait for authentication methods, essentially binding an ID value /// (as used in the method negotiation) to an actual implementation of the method. /// -/// Use blank structs for individual protocol implementations and -/// enums for sets of supported protocols (you'll need a matching enum for the `Impl`). +/// Use blank structs for individual protocol implementations; +/// `negotiate_auth` takes any slice of `AuthMethod`s, so a custom +/// selection is a slice of the method structs (upstream's +/// enum-generator macro was cut — review 002 N-1; nothing in-tree +/// drove a multi-method slice). pub trait AuthMethod: Copy { type StartingState; fn method_id(self) -> u8; @@ -409,78 +405,6 @@ macro_rules! try_notify { }; } -macro_rules! auth_method_enums { - ( - $(#[$enum_meta:meta])* - $vis:vis enum $enum:ident / $(#[$state_enum_meta:meta])* $state_enum:ident<$state_enum_par:ident> { - $($method:ident($state:ty)),+ $(,)? - } - ) => { - $(#[$state_enum_meta])* - $vis enum $state_enum<$state_enum_par> { - $($method($state)),+ - } - - #[derive(Clone, Copy)] - $(#[$enum_meta])* - $vis enum $enum { - $($method($method)),+ - } - - impl AuthMethod for $enum { - type StartingState = $state_enum; - - fn method_id(self) -> u8 { - match self { - $($enum::$method(auth) => AuthMethod::::method_id(auth)),+ - } - } - - fn into_starting_state(self, inner: T) -> Self::StartingState { - match self { - $($enum::$method(auth) => $state_enum::$method(auth.into_starting_state(inner))),+ - } - } - } - }; -} - -auth_method_enums! { - /// The combination of all authentication methods supported by this crate out of the box, - /// as an enum appropriate for static dispatch. - /// - /// If you want to add your own custom methods, you can generate a similar enum using the `auth_method_enums` macro. - /// - /// Posture (review 001 C-2): currently uncalled in-tree — the - /// typestate's `negotiate_auth` is driven with the single-method - /// slices everywhere. Kept as the negotiation machinery's - /// combinator: the producer layer's per-door auth selection - /// (ADR-005's auth-split) is its anticipated consumer. If the - /// producer layer lands without consuming it, cut it then. - pub enum StandardAuthentication / StandardAuthenticationStarted { - NoAuthentication(NoAuthenticationImpl), - PasswordAuthentication(PasswordAuthenticationImpl), - } -} - -impl StandardAuthentication { - /// Return a slice containing either both supported methods or only `PasswordAuthentication`. - pub fn allow_no_auth(allow: bool) -> &'static [StandardAuthentication] { - if allow { - &[ - // The order of authentication methods can be tested by clients in sequence, - // so list more secure or preferred methods first - StandardAuthentication::PasswordAuthentication(PasswordAuthentication), - StandardAuthentication::NoAuthentication(NoAuthentication), - ] - } else { - &[StandardAuthentication::PasswordAuthentication( - PasswordAuthentication, - )] - } - } -} - impl Socks5ServerProtocol { /// Negotiate an authentication method from a list of supported ones and initialize it. /// @@ -685,9 +609,14 @@ mod tests { /// variant falls through to `GeneralFailure` — the mapping the /// producer's error surface relies on when mapping session errors /// to reply codes (`try_notify!`'s secondary-error write). The - /// fall-through arms (`Io`/`FromUtf8`/`EOF`/auth variants) are - /// exercised here directly, since driving each from the wire is - /// not one-test-cheap. Upstream `src/server.rs:62-70`. + /// fall-through arms (`Io`/`FromUtf8`/`EOF`/auth/refusal variants) + /// are exercised here directly, since driving each from the wire + /// is not one-test-cheap. Upstream `src/server.rs:62-70`. The + /// table also pins the vocabulary honesty (review 002 N-4): + /// `Bug` and `Refused` are log-vocabulary variants — both fall + /// through to `GeneralFailure` (they never reach the wire from + /// the typestate; the producer's refusal sites write the RFC + /// reply before the session error). #[test] fn to_reply_error_mapping_table() { assert!(matches!( @@ -706,6 +635,7 @@ mod tests { SocksServerError::AuthenticationRejected, SocksServerError::UnsupportedSocksVersion(0x04), SocksServerError::Bug("test"), + SocksServerError::Refused("test"), SocksServerError::AuthMethodUnacceptable(vec![0x02]), SocksServerError::UnexpectedUdpControlGarbage(0xde), SocksServerError::Io { @@ -724,6 +654,27 @@ mod tests { } } + /// The refusal vocabulary (review 002 N-4): `Refused` is the + /// honest display for the producer's in-stream policy refusals — + /// not `BUG:` (a working-as-designed refusal is not a bug) and + /// not "Authentication rejected" (an ACL refusal is not an auth + /// failure). It is log/observability vocabulary: never encoded + /// on the wire by the typestate (only `to_reply_error` maps it, + /// and the producer's refusal sites write the RFC reply before + /// the session error). Upstream has no such variant — the + /// fork's owned addition, recorded in the module doc. + #[test] + fn refused_variant_display_is_honest_and_wire_invisible() { + assert_eq!( + SocksServerError::Refused("policy says no").to_string(), + "Refused: policy says no" + ); + assert!(matches!( + SocksServerError::Refused("policy says no").to_reply_error(), + ReplyError::GeneralFailure + )); + } + #[test] fn new_reply_v4_form() { let addr: SocketAddr = "127.0.0.1:1080".parse().unwrap(); diff --git a/src/rfc1928/util/stream.rs b/src/rfc1928/util/stream.rs index dc9b22c..d7dbc84 100644 --- a/src/rfc1928/util/stream.rs +++ b/src/rfc1928/util/stream.rs @@ -14,12 +14,13 @@ //! //! [`ConnectError`] is currently constructed nowhere in-tree (review //! 001 C-1 — upstream's sole construction site was the cut -//! `tcp_connect` dialer). It is kept as the hand-port map's -//! companion: the dial-error → reply-code mapping +//! `tcp_connect` dialer). The keep-or-cut trigger (cut if the +//! producer layer landed without consuming it) fired and the +//! disposition is **keep** (review 002 N-1): it stays as the +//! hand-port map's companion — the dial-error → reply-code mapping //! (`to_reply_error`) is what a future kernel-net port (e.g. the //! dial-callback shape, ADR-004) would reuse, and its mapping is -//! pinned by the module's test. If the producer layer lands without -//! consuming it, cut it then. +//! pinned by the module's test. /// Easy to destructure bytes buffers by naming each fields: /// diff --git a/tasks/implementation/fix-rfc1928-posture.md b/tasks/implementation/fix-rfc1928-posture.md index c9ad974..eac1aed 100644 --- a/tasks/implementation/fix-rfc1928-posture.md +++ b/tasks/implementation/fix-rfc1928-posture.md @@ -1,7 +1,7 @@ --- id: implementation/fix-rfc1928-posture name: "Fix — rfc1928 keep-or-cut pass + honest refusal vocabulary (review-002 N-1, N-4)" -status: pending +status: completed depends_on: [] scope: narrow risk: medium @@ -86,20 +86,20 @@ display/shape if the vocabulary is table-tested elsewhere. ## Acceptance Criteria -- [ ] `StandardAuthentication`, `allow_no_auth`, the generated +- [x] `StandardAuthentication`, `allow_no_auth`, the generated enum, `skip_auth_this_is_not_rfc_compliant`, and `CheckResult`'s `Option`/`Result` impls are cut; `bool` stays -- [ ] `ConnectError`, `transfer`/`wait_on_tcp`, `Bug` kept — each +- [x] `ConnectError`, `transfer`/`wait_on_tcp`, `Bug` kept — each with its keep-note current (module docs name the posture) -- [ ] The differential suite stays green (the cut machinery was not +- [x] The differential suite stays green (the cut machinery was not load-bearing for it; the `:103` comment reworded) -- [ ] The five in-stream refusal sites carry the honest vocabulary +- [x] The five in-stream refusal sites carry the honest vocabulary (one variant or the renamed display — the choice recorded); the wire bytes are untouched (the reply-code assertions stay green) -- [ ] The fork's provenance notice is extended, not replaced, if the +- [x] The fork's provenance notice is extended, not replaced, if the carried-changes note names the new variant (convention 17) -- [ ] `cargo test` green; clippy/fmt clean; both wasm checks clean; +- [x] `cargo test` green; clippy/fmt clean; both wasm checks clean; `cargo doc --no-deps` clean (module docs changed) ## References @@ -116,8 +116,87 @@ display/shape if the vocabulary is table-tested elsewhere. ## Notes -> Agent fills this during implementation. +- **N-4 shape chosen: the dedicated variant** — + `SocksServerError::Refused(&'static str)`, display `"Refused: + {0}"` — not the `Bug` display rename. The review's sketch favored + the dedicated variant ("the honest shape"), and the vocabulary + split is real: `Bug` stays upstream-verbatim (`BUG: {0}`, its + "should be unreachable" contract intact) while policy refusals + get their own honest label. The five sites now read + `Refused("bind command gate: not allowed")`, + `Refused("dial refused in-stream")`, + `Refused("egress refused in-stream")`, + `Refused("bind factory refused in-stream")`, + `Refused("bind accept refused in-stream")` — same + `&'static str` payload shape as `Bug`, zero wire impact (the RFC + reply went out before each `Err`). +- **The harness's `Bug` site was migrated too** (the task said + "leave unless it wants the new variant"): `tests/harness.rs`'s + minimal CONNECT producer returned `Bug("dial refused in-stream")` + — the same working-as-designed refusal shape, so it takes the new + variant for vocabulary consistency across the log surface. No + test asserts that harness error's variant, so nothing else + moved. +- **The mapping-table test grew the new variant**: + `SocksServerError::Refused` rides the `GeneralFailure` + fall-through loop in `to_reply_error_mapping_table` (alongside + `Bug`), and a new test pins the display string and the + fall-through mapping (`refused_variant_display_is_honest_and_ + wire_invisible`) — the task's "one test pinning the new variant's + display/shape". +- **Provenance extended, not replaced** (convention 17): the + `server.rs` module doc gained a "Post-fork owned changes" + paragraph (the cuts, the keep-notes, the `Refused` addition); + `docs/architecture/rfc1928.md` gained the matching "owned-code + post-fork changes" list under the deliberate modifications and a + `last_updated` line. The bundled `src/rfc1928/LICENSE` notice + already carries the "modifications after the fork point" clause + generically — no change needed there. +- **Keep-notes made current**: `stream.rs`'s `ConnectError` note + records that the trigger fired and the disposition is **keep** + (was: "cut it then"); `transfer`/`wait_on_tcp`'s differential- + suite note lives in the `server.rs` module doc; `Bug`'s keep-note + is the same paragraph (upstream-verbatim, its display untouched). +- **The differential suite's `:103` comment** reworded to the + RFC-ordering rationale (the server picks by its own preference, + more-secure-first) — no cut-type reference remains anywhere. +- **The `AuthMethod` trait doc** was updated for the macro cut + (upstream's "use enums for sets" line → a custom selection is a + slice of method structs) and the module doc's extraction-time + adjustments line dropped the `#[macro_export]` note (the macro + itself is gone). ## Summary -> Agent fills this on completion. \ No newline at end of file +Both items landed. **N-1 (the cut)**: `auth_method_enums!` (the +macro, nothing else used it), `StandardAuthentication` + +`StandardAuthenticationStarted` + `allow_no_auth`, +`skip_auth_this_is_not_rfc_compliant`, and `CheckResult`'s +`Option`/`Result` impls are gone from `src/rfc1928/server.rs` — +the producer-replaced machinery, per ADR-013's +no-speculative-surface posture. Kept with notes: `ConnectError` +(hand-port companion, `stream.rs` note updated to record the fired +trigger + keep disposition), `transfer`/`wait_on_tcp` +(differential-suite-consumed, semantics pinned there), `Bug` +(upstream-verbatim, display intact). **N-4 (the vocabulary)**: new +`SocksServerError::Refused(&'static str)` variant, used at the five +in-stream refusal sites (the BIND command gate + the four seam +refusals) plus the harness's minimal producer; `Bug`'s display is +untouched (a real bug should still read `BUG:`); the wire bytes are +byte-identical (every reply-code assertion stayed green unchanged). +Tests: the three gate-site session-error asserts moved from +`AuthenticationRejected` to `Refused(...)`, the four `Bug(...)` +asserts to `Refused(...)`, the mapping-table loop grew the variant, +and a new display/shape test pins both. Docs: the `server.rs` +module doc's owned-changes paragraph, the `rfc1928.md` +"owned-code post-fork changes" list + `last_updated`, the +`stream.rs` keep-note, the `AuthMethod` doc, and the differential +comment. The review-002 N-1 coverage blocks resolve (the cuts were +the accepted-uncovered surface). + +Verified: `cargo test` (356 green across all suites incl. +`--all-features`), `cargo clippy --all-targets -- -D warnings` +clean, `cargo fmt --check` clean, `cargo doc --no-deps` clean, +`cargo check --target wasm32-unknown-unknown` + +`cargo clippy --target wasm32-unknown-unknown -- -D warnings` +clean. \ No newline at end of file diff --git a/tests/harness.rs b/tests/harness.rs index e8d4288..e669a88 100644 --- a/tests/harness.rs +++ b/tests/harness.rs @@ -384,7 +384,7 @@ where .reply_error(&err.to_reply_error()) .await .map_err(SocksError::ServerError)?; - return Err(SocksError::ServerError(SocksServerError::Bug( + return Err(SocksError::ServerError(SocksServerError::Refused( "dial refused in-stream", ))); } diff --git a/tests/rfc1928-differential.rs b/tests/rfc1928-differential.rs index 79bd7fe..5da24cf 100644 --- a/tests/rfc1928-differential.rs +++ b/tests/rfc1928-differential.rs @@ -100,9 +100,9 @@ async fn negotiation_server_picks_its_own_first_match() { /// The same contract under `accept_password_auth`: a client offering /// none-then-password is answered with the *server's* preferred -/// method (0x02, listed first upstream per the `StandardAuthentication -/// ::allow_no_auth` ordering note — `src/server.rs:455-469` upstream -/// / in-tree `server.rs` "more secure or preferred methods first"). +/// method (0x02 — `negotiate_auth` walks `server_methods` in server +/// order; RFC 1928 §3 lets the server pick by its own preference, so +/// the method list is ordered more-secure/preferred first). #[tokio::test] async fn negotiation_password_server_prefers_password_over_no_auth() { let (mut client, server) = tokio::io::duplex(64);