diff --git a/crates/libid-transcript/src/ceremony.rs b/crates/libid-transcript/src/ceremony.rs index b0dfadf..229f23f 100644 --- a/crates/libid-transcript/src/ceremony.rs +++ b/crates/libid-transcript/src/ceremony.rs @@ -334,6 +334,26 @@ mod tests { const X_TOKEN_REQ: &[u8] = b"POST /2/oauth2/token HTTP/1.1\r\nhost: api.x.com\r\n\r\ngrant_type=authorization_code&client_id=abc&code_verifier=xyz"; + #[test] + fn a_spaced_access_token_delimiter_is_revealed_and_the_bearer_alone_committed() { + // A token service that pretty-prints puts whitespace inside the + // delimiter: the delimiter is revealed as served, and the bearer between + // the two reveals is what is committed -- never the whitespace with it. + for ws in [" ", "\t", "\r", "\n", " \t\r\n"] { + let prefix = format!("\"access_token\"{ws}:{ws}\""); + let recv = format!("HTTP/1.1 200 OK\r\n\r\n{{{prefix}SECRET\"}}"); + let recv = recv.as_bytes(); + let layout = Layout::token_response(recv).unwrap(); + assert!(tiles(&layout, recv.len())); + assert_eq!(&recv[layout.reveal[0].clone()], prefix.as_bytes()); + assert_eq!( + &recv[layout.reveal[0].end..layout.reveal[1].start], + b"SECRET" + ); + assert_eq!(&recv[layout.reveal[1].clone()], b"\""); + } + } + #[test] fn a_bearer_split_by_chunk_framing_is_refused() { // The session Rust actually runs. Framing inside the committed range @@ -633,7 +653,10 @@ mod tests { #[cfg(test)] mod tables { - use super::profiles; + use super::{ + profiles, + Layout, + }; /// The ceremony profiles and the identity system name the same platforms. /// @@ -661,4 +684,32 @@ mod tables { assert_eq!(profiles::GITHUB.platform, PLATFORM_GITHUB_DOMAIN); assert_eq!(profiles::GOOGLE.platform, PLATFORM_GOOGLE_DOMAIN); } + + #[test] + fn github_pretty_prints_and_the_layout_carries_the_whitespace() { + // The response GitHub serves for the profile's media type, and the + // two members the profile reads out of it, revealed as the wire + // carries them -- whitespace inside, at its offsets. + let body = + "{\n \"login\": \"octocat\",\n \"id\": 583231,\n \"node_id\": \"x\"\n}"; + let recv = format!( + "HTTP/1.1 200 OK\r\ncontent-type: application/json; charset=utf-8\r\ncontent-length: {}\r\n\r\n{body}", + body.len() + ); + let layout = Layout::identity_response( + recv.as_bytes(), + &profiles::GITHUB.identity.unwrap(), + ) + .unwrap(); + let revealed: Vec<&[u8]> = layout + .reveal + .iter() + .map(|range| &recv.as_bytes()[range.clone()]) + .collect(); + assert!( + revealed.contains(&&b"\"login\": \"octocat\""[..]), + "{revealed:?}" + ); + assert!(revealed.contains(&&b"\"id\": 583231,"[..]), "{revealed:?}"); + } } diff --git a/crates/libid-transcript/src/ranges.rs b/crates/libid-transcript/src/ranges.rs index 07fb189..58455bf 100644 --- a/crates/libid-transcript/src/ranges.rs +++ b/crates/libid-transcript/src/ranges.rs @@ -120,15 +120,19 @@ fn decode_chunked_body(raw: &[u8]) -> Result> { } /// The `"key":"value"` member, from the key's opening quote through the -/// value's closing quote. +/// value's closing quote, with whatever JSON whitespace sits between them. /// /// # The template is the reader's /// -/// `CeremonyFields.tryJsonString` matches the literal `"":"`, so this -/// matches the same bytes. Anything looser picks a range the reader cannot -/// read: a body written `"login" : "octocat"` would be revealed here and then -/// met with `FieldNotFound` on chain, which is the same refusal reported where -/// nobody can see why. Failing here fails it where the reason is visible. +/// `CeremonyFields.tryJsonString` removes the JSON whitespace beside a +/// structural byte from the bytes it is shown and then matches the literal +/// `"":"`, so this accepts exactly what that removal maps onto the +/// literal -- whitespace between the key and the colon, and between the colon +/// and the value -- and nothing else. Anything looser picks a range the reader +/// cannot read: a body written `"login" "octocat"`, no colon, would be +/// revealed here and then met with `FieldNotFound` on chain, which is the same +/// refusal reported where nobody can see why. Failing here fails it where the +/// reason is visible. /// /// Uniqueness is NOT checked here, and that is deliberate. The reader refuses /// a delimiter matching twice in the bytes it was shown (REQ-COMMON-19A), and @@ -173,9 +177,11 @@ impl JsonMember { /// is argued on [`find_json_snippet_range`], which is the public face of /// this scan. fn in_body(body: &[u8], field: &str) -> Option { - let needle = format!("\"{field}\":\""); - let start = find_first(body, needle.as_bytes())?; - let value = start.checked_add(needle.len())?; + // The key, then `:`, then the value's opening quote, with the + // whitespace JSON allows on either side of the colon kept inside the + // member -- [`key_and_value`] says why. + let (start, quote) = key_and_value(body, field, true)?; + let value = quote.checked_add(1)?; let close = body .get(value..)? .iter() @@ -228,6 +234,41 @@ fn find_first(haystack: &[u8], needle: &[u8]) -> Option { haystack.windows(needle.len()).position(|w| w == needle) } +/// The offset past the run of JSON whitespace starting at `at`, or `at`. +fn skip_json_whitespace(body: &[u8], mut at: usize) -> usize { + while let Some(b' ' | b'\t' | b'\n' | b'\r') = body.get(at) { + at += 1; + } + at +} + +/// The first `"field"` in `body` that a colon follows, as the offset of the +/// key's opening quote and the offset of the value's first byte -- past the +/// whitespace JSON allows on either side of the colon (RFC 8259 section 2), +/// and, when `quoted`, only where that byte is the value's opening quote. +/// +/// A `"login"` that is another member's value, or a key of another shape, is +/// passed over. GitHub pretty-prints its identity response, so a template +/// without the whitespace allowance matches nothing it serves. The whitespace +/// stays inside the member a caller cuts from these offsets: the verifier +/// reads the range as the wire carried it and removes that whitespace itself +/// before it compares, so the range has to carry it. +fn key_and_value(body: &[u8], field: &str, quoted: bool) -> Option<(usize, usize)> { + let key = format!("\"{field}\""); + let mut from = 0; + loop { + let start = find_first(body.get(from..)?, key.as_bytes())?.checked_add(from)?; + let colon = skip_json_whitespace(body, start.checked_add(key.len())?); + if body.get(colon) == Some(&b':') { + let value = skip_json_whitespace(body, colon.checked_add(1)?); + if !quoted || body.get(value) == Some(&b'"') { + return Some((start, value)); + } + } + from = start.checked_add(1)?; + } +} + /// The raw bytes are the member, and not the member with framing through it. /// /// A chunked body carries `\r\n\r\n` between chunks, and that framing @@ -253,16 +294,14 @@ fn require_contiguous(raw: &[u8], decoded: &[u8]) -> Option<()> { /// number; both terminators are included in the range (on-chain /// `tryJsonInteger` scans digits and stops at either). pub fn find_json_bare_snippet_range(body: &[u8], field: &str) -> Option> { - let needle = format!("\"{field}\":"); - let start = find_first(body, needle.as_bytes())?; - let from = start.checked_add(needle.len())?; + let (start, digits) = key_and_value(body, field, false)?; // Digits, then the byte that closes them -- the order `tryJsonInteger` // reads in. Scanning instead to the first `,` or `}` would accept // `"id":"7",`, a quoted value returned as though it were a number: the // chain then refuses it as noncanonical, which is the same answer given // where nobody can see the reason. - let rest = body.get(from..)?; + let rest = body.get(digits..)?; let width = rest.iter().take_while(|b| b.is_ascii_digit()).count(); if width == 0 { return None; @@ -274,8 +313,9 @@ pub fn find_json_bare_snippet_range(body: &[u8], field: &str) -> Option Some(start..term.checked_add(1)?), _ => None, @@ -384,15 +424,86 @@ mod tests { } #[test] - fn a_spaced_member_is_refused_because_the_reader_refuses_it() { - // The on-chain needle is the literal `"login":"`. Selecting a range - // here that the reader cannot read only moves the same refusal to - // where its reason is invisible. - let body = br#"{"login" : "octocat"}"#; - assert!(find_json_snippet_range(body, "login").is_none()); + fn a_spaced_member_is_found_with_its_whitespace_inside() { + // GitHub pretty-prints: a space after the colon, a newline and an + // indent before every key. The reader on chain removes the JSON + // whitespace beside a structural byte before it looks, so the member + // is found here and revealed with that whitespace at its offsets. + let body = b"{\n \"login\" : \"octocat\",\n \"id\": 583231\n}"; + let member = find_json_snippet_range(body, "login").unwrap(); + assert_eq!(&body[member], b"\"login\" : \"octocat\""); + let id = find_json_bare_snippet_range(body, "id").unwrap(); + assert_eq!(&body[id], b"\"id\": 583231\n}"); + } + + #[test] + fn a_key_that_is_only_a_value_is_passed_over() { + // `"login"` appears first as another member's value; the member is the + // one a colon and a quote follow. + let body = br#"{"name":"login","login":"octocat"}"#; + let member = find_json_snippet_range(body, "login").unwrap(); + assert_eq!(&body[member], br#""login":"octocat""#); + } + + #[test] + fn whitespace_inside_a_number_is_not_a_number() { + // `123 4` is two tokens where one is expected; the reader on chain + // keeps that space and refuses it as the terminator, and so nothing is + // revealed for it here. + assert_eq!(find_json_bare_snippet_range(b"{\"id\":123 4}", "id"), None); + } + + #[test] + fn every_json_whitespace_byte_is_kept_inside_the_member() { + // Each byte RFC 8259 section 2 calls whitespace, alone and as a run, + // on both sides of the colon and before the integer's terminator: the + // ranges are the members as the wire carries them. + for ws in [" ", "\t", "\n", "\r", " \t\r\n"] { + let member = format!("\"login\"{ws}:{ws}\"octocat\""); + let id = format!("\"id\"{ws}:{ws}123{ws},"); + let recv = format!("HTTP/1.1 200 OK\r\n\r\n{{{id}{member}}}"); + let recv = recv.as_bytes(); + assert_eq!( + &recv[compute_field_snippet_range(recv, "login").unwrap()], + member.as_bytes() + ); + assert_eq!( + &recv[compute_id_snippet_range(recv, "id", false).unwrap()], + id.as_bytes() + ); + } + } + + #[test] + fn a_byte_json_does_not_call_whitespace_is_not_skipped() { + // Vertical tab and form feed are whitespace to a text editor and not + // to RFC 8259; the reader on chain keeps them, so nothing is found + // over them here. + for ws in ["\u{000b}", "\u{000c}"] { + let body = format!("{{\"login\":{ws}\"octocat\",\"id\":{ws}123}}"); + assert_eq!(find_json_snippet_range(body.as_bytes(), "login"), None); + assert_eq!(find_json_bare_snippet_range(body.as_bytes(), "id"), None); + } + } + + #[test] + fn a_number_of_another_shape_is_not_a_number() { + // Digits and nothing else: an exponent or a fraction puts a byte where + // the terminator must be. + for value in ["1e3", "1.5"] { + let body = format!("{{\"id\": {value}}}"); + assert_eq!(find_json_bare_snippet_range(body.as_bytes(), "id"), None); + } + } - let bare = br#"{"id" : 123}"#; - assert!(find_json_bare_snippet_range(bare, "id").is_none()); + #[test] + fn whitespace_split_by_chunk_framing_is_not_a_contiguous_member() { + // The framing lands in the whitespace rather than in the value, and + // the member still spans two chunks: refused all the same. + let recv = straddling(r#"{"login" "#, r#": "alice"}"#); + assert!(compute_field_snippet_range(&recv, "login").is_none()); + let recv = straddling(r#"{"id": "#, r#"123}"#); + assert!(compute_id_snippet_range(&recv, "id", false).is_none()); } #[test]