Skip to main content

max / makenotwork

synckit: add constant-time OAuth state check, tighten loopback TLS exemption (ultra-fuzz Run #1 Auth) - oauth: states_match (constant-time) + doc that the consumer must validate the echoed OAuth state before code exchange. - requires_https: drop 0.0.0.0 from the loopback exemption (it's the unspecified bind-all address, not a loopback to connect to), so plaintext to it enforces TLS. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Author: Max Johnson <me@maxj.phd> · 2026-06-19 22:36 UTC
Commit: 64c640f600d49df97a3dd23678faf3ff8914e52f
Parent: 98c5776
3 files changed, +37 insertions, -2 deletions
M shared/synckit-client/src/client/mod.rs +5 -1
@@ -308,9 +308,11 @@ fn requires_https(server_url: &str) -> bool {
308 308 // https (or any non-plaintext scheme): enforcing https_only is a no-op.
309 309 return true;
310 310 };
311 + // Strictly loopback only — `0.0.0.0` is the unspecified/bind-all address, not
312 + // a loopback you'd connect *to*, so it does not belong in the plaintext-exempt
313 + // set.
311 314 let is_loopback = rest.starts_with("localhost")
312 315 || rest.starts_with("127.")
313 - || rest.starts_with("0.0.0.0")
314 316 || rest.starts_with("[::1]")
315 317 || rest.starts_with("::1");
316 318 !is_loopback
@@ -359,6 +361,8 @@ mod tests {
359 361 assert!(requires_https("https://makenot.work"));
360 362 assert!(requires_https("http://makenot.work"));
361 363 assert!(requires_https("http://192.168.1.5:7766"));
364 + // 0.0.0.0 is the unspecified address, not loopback — must enforce TLS.
365 + assert!(requires_https("http://0.0.0.0:8080"));
362 366 // loopback is exempt (local dev + mock-server tests use http://127.0.0.1).
363 367 assert!(!requires_https("http://127.0.0.1:8080"));
364 368 assert!(!requires_https("http://localhost:3000"));
M shared/synckit-client/src/lib.rs +1 -1
@@ -54,7 +54,7 @@ pub mod types;
54 54 // Re-exports for convenience
55 55 pub use client::{validate_api_key, SessionInfo, SyncKitClient, SyncKitConfig, SyncNotifyStream};
56 56 pub use client::{OtaArtifactUpload, OtaManifest, OtaRelease};
57 - pub use oauth::{generate_oauth_state, generate_pkce, Pkce};
57 + pub use oauth::{generate_oauth_state, generate_pkce, states_match, Pkce};
58 58 pub use client::subscription::{
59 59 AccountInfo, AppPricing, BillingInterval, CheckoutResponse, PriceQuote, SubscriptionStatus,
60 60 };
M shared/synckit-client/src/oauth.rs +31
@@ -36,17 +36,48 @@ pub fn generate_pkce() -> Pkce {
36 36 }
37 37
38 38 /// Generate a random opaque `state` value for CSRF protection on the OAuth flow.
39 + ///
40 + /// The caller must hold onto the returned value and, when the authorization
41 + /// redirect arrives at the localhost listener, confirm the echoed `state`
42 + /// matches via [`states_match`] **before** calling `authenticate_with_code`.
43 + /// Skipping that check reopens the CSRF hole this value exists to close (PKCE
44 + /// still binds the code to this client, so this is defense-in-depth).
39 45 pub fn generate_oauth_state() -> String {
40 46 let mut bytes = [0u8; 16];
41 47 rand::rng().fill_bytes(&mut bytes);
42 48 URL_SAFE_NO_PAD.encode(bytes)
43 49 }
44 50
51 + /// Constant-time comparison of an OAuth `state` value against the one returned
52 + /// on the redirect. Constant-time (over equal-length inputs) so a timing side
53 + /// channel can't be used to forge a matching `state` byte by byte.
54 + pub fn states_match(expected: &str, actual: &str) -> bool {
55 + let (a, b) = (expected.as_bytes(), actual.as_bytes());
56 + if a.len() != b.len() {
57 + return false;
58 + }
59 + let mut diff = 0u8;
60 + for (x, y) in a.iter().zip(b) {
61 + diff |= x ^ y;
62 + }
63 + diff == 0
64 + }
65 +
45 66 #[cfg(test)]
46 67 mod tests {
47 68 use super::*;
48 69
49 70 #[test]
71 + fn states_match_only_on_equal_values() {
72 + let s = generate_oauth_state();
73 + assert!(states_match(&s, &s.clone()));
74 + assert!(!states_match(&s, "different"));
75 + assert!(!states_match("a", "ab")); // length mismatch
76 + assert!(!states_match("abc", "abd"));
77 + assert!(states_match("", ""));
78 + }
79 +
80 + #[test]
50 81 fn verifier_is_43_chars_unreserved() {
51 82 let p = generate_pkce();
52 83 assert_eq!(p.verifier.len(), 43); // 32 bytes base64url-nopad