Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 8 additions & 8 deletions .plan/marshal.json
Original file line number Diff line number Diff line change
Expand Up @@ -122,13 +122,6 @@
"ce_wait_timeout_seconds": 600,
"lane": "standard"
},
"default:lessons-capture": {
"lane": "minimal"
},
"default:finalize-step-preference-emitter": {
"preference_min_recurrence": 2,
"lane": "minimal"
},
"default:adr-propose": {
"lane": "off"
},
Expand All @@ -144,6 +137,13 @@
"pre_merge_comment_barrier": "fail_into_loopback",
"lane": "minimal"
},
"default:lessons-capture": {
"lane": "off"
},
"default:finalize-step-preference-emitter": {
"preference_min_recurrence": 2,
"lane": "minimal"
},
"default:record-metrics": {
"lane": "minimal"
},
Expand Down Expand Up @@ -324,7 +324,7 @@
"no_plan_body_days": 7,
"build_results_days": 5
},
"provisioned_version": "0.1.1286",
"provisioned_version": "0.1.1288",
"config_seed_fingerprint": "714f8058"
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -400,6 +400,20 @@ private static String requireNonBlank(String cookieName) {

/**
* The successful outcome of {@link #unseal(String)}: the authenticated session payload.
* <p>
* <strong>Load-bearing — do not remove.</strong> This record is the payload wrapper of
* {@link #unseal(String)}'s return type, and its production consumer is the
* {@code readSealedValue(…).flatMap(codec::unseal).filter(…).map(…)} chain in
* {@code CookieSessionBinding.resolve}. That call site uses the <em>method-reference</em> form
* {@code codec::unseal}, so a reachability search for the call form {@code unseal(} alone reports
* this API as having no production consumer — a false "unused" verdict that would justify an
* unsafe removal. Search both forms before re-opening the question.
* <p>
* The {@code Optional<Unsealed>} return type is <em>not</em> a residue of the PLAN-36 sweep:
* {@link de.cuioss.sheriff.gateway.bff.cookie.SealedSessionCookieCodec#unseal(String)} is a
* computed method return, and ADR-0033 retires {@code Optional} only from <em>stored</em>
* positions — fields, declared parameters and record components. A computed return is explicitly
* sanctioned, so a future sweep should not re-flag it.
*
* @param payload the authenticated session payload
* @author API Sheriff Team
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,13 +63,6 @@ public final class FramingGate {

private final boolean allowGetWithContentLengthBody;

/**
* Creates a gate with the strict default posture — a body on {@code GET} is rejected.
*/
public FramingGate() {
this(false);
}

/**
* Creates a gate with the boot-resolved {@code GET}-body posture.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,41 +81,103 @@ public Result parse(byte[] bytes) {
ByteArrayOutputStream handshake = new ByteArrayOutputStream();
int pos = 0;
while (true) {
if (bytes.length - pos < RECORD_HEADER_LENGTH) {
return overBound(pos) ? Result.parsed(null) : Result.needMoreData();
}
if ((bytes[pos] & UINT8_MASK) != RECORD_TYPE_HANDSHAKE) {
// Not a TLS handshake record — fail closed to the terminated-strict path.
return Result.parsed(null);
Result headerVerdict = recordHeaderVerdict(bytes, pos);
if (headerVerdict != null) {
return headerVerdict;
}
int recordLength = uint16(bytes, pos + 3);
int recordEnd = pos + RECORD_HEADER_LENGTH + recordLength;
if (recordEnd > MAX_CLIENT_HELLO_BYTES) {
return Result.parsed(null);
}
if (bytes.length < recordEnd) {
return Result.needMoreData();
Result bodyVerdict = recordBodyVerdict(bytes, recordEnd);
if (bodyVerdict != null) {
return bodyVerdict;
}
handshake.write(bytes, pos + RECORD_HEADER_LENGTH, recordLength);
pos = recordEnd;

byte[] handshakeBytes = handshake.toByteArray();
HandshakeSpan span = completeHandshake(handshakeBytes);
if (span == HandshakeSpan.MALFORMED) {
return Result.parsed(null);
}
if (span == HandshakeSpan.COMPLETE) {
return Result.parsed(extractServerName(handshakeBytes));
}
// span == INCOMPLETE: keep reading records if any remain, else ask for more bytes.
if (bytes.length - pos < RECORD_HEADER_LENGTH) {
return overBound(pos) ? Result.parsed(null) : Result.needMoreData();
Result reassembledVerdict = reassembledVerdict(handshake.toByteArray());
if (reassembledVerdict != null) {
return reassembledVerdict;
}
// The handshake is still incomplete: loop back and consume the next record. The
// "is another record header even present?" question is the loop head's own first check,
// so it is asked there rather than repeated here.
}
}

/**
* The terminal verdict, if any, implied by the record header at {@code pos}: too few bytes for a
* header (keep buffering, or fail closed once the bound is passed), or a record that is not a TLS
* handshake at all.
* <p>
* <strong>The give-up test is anchored on {@code bytes.length}</strong> — the bytes already
* buffered — rather than on {@code pos}, the bytes already consumed. {@link #MAX_CLIENT_HELLO_BYTES}
* declares its bound on the accumulated buffer, and in this branch {@code pos} trails
* {@code bytes.length} by up to {@code RECORD_HEADER_LENGTH - 1}: a position-anchored test would
* therefore still answer {@link Result#needMoreData()} for a buffer holding up to
* {@code MAX_CLIENT_HELLO_BYTES + 3} bytes, telling the caller to keep buffering something that
* has already passed the declared hard bound. Since {@code pos <= bytes.length} always holds
* ({@link #recordBodyVerdict(byte[], int)} advances {@code pos} only to a {@code recordEnd} it has
* confirmed is within the buffer), the buffer-anchored test subsumes the position-anchored one.
*
* @param bytes the accumulated connection bytes
* @param pos the offset of the record header being examined
* @return the verdict to return from {@code parse}, or {@code null} to consume this record
*/
private static @Nullable Result recordHeaderVerdict(byte[] bytes, int pos) {
if (bytes.length - pos < RECORD_HEADER_LENGTH) {
return overBound(bytes.length) ? Result.parsed(null) : Result.needMoreData();
}
Comment thread
cuioss-oliver marked this conversation as resolved.
if ((bytes[pos] & UINT8_MASK) != RECORD_TYPE_HANDSHAKE) {
// Not a TLS handshake record — fail closed to the terminated-strict path.
return Result.parsed(null);
}
return null;
}

/**
* The terminal verdict, if any, implied by the declared record body: a record running past the
* size bound fails closed, and a body that has not fully arrived means keep buffering.
*
* @param bytes the accumulated connection bytes
* @param recordEnd the offset one past the declared end of this record
* @return the verdict to return from {@code parse}, or {@code null} to consume this record
*/
private static @Nullable Result recordBodyVerdict(byte[] bytes, int recordEnd) {
if (recordEnd > MAX_CLIENT_HELLO_BYTES) {
return Result.parsed(null);
}
if (bytes.length < recordEnd) {
return Result.needMoreData();
}
return null;
}

/**
* The terminal verdict, if any, implied by the handshake bytes reassembled so far: a wrong
* handshake type or an over-bound body fails closed, a complete {@code client_hello} yields the
* extracted SNI.
*
* @param handshakeBytes the handshake bytes reassembled across the records consumed so far
* @return the verdict to return from {@code parse}, or {@code null} while the body is incomplete
*/
private static @Nullable Result reassembledVerdict(byte[] handshakeBytes) {
HandshakeSpan span = completeHandshake(handshakeBytes);
if (span == HandshakeSpan.MALFORMED) {
return Result.parsed(null);
}
if (span == HandshakeSpan.COMPLETE) {
return Result.parsed(extractServerName(handshakeBytes));
}
return null;
}

private static boolean overBound(int pos) {
return pos >= MAX_CLIENT_HELLO_BYTES;
/**
* Whether {@code byteCount} has reached the reassembly bound. Callers pass the number of bytes
* <em>buffered</em>, never the number consumed — see
* {@link #recordHeaderVerdict(byte[], int)} for why the distinction is load-bearing.
*/
private static boolean overBound(int byteCount) {
return byteCount >= MAX_CLIENT_HELLO_BYTES;
}

/**
Expand Down Expand Up @@ -168,7 +230,7 @@ private static HandshakeSpan completeHandshake(byte[] handshake) {
cursor.seek(next);
}
return null;
} catch (MalformedHelloException e) {
} catch (MalformedHelloException _) {
return null;
}
}
Expand Down Expand Up @@ -202,10 +264,50 @@ private static HandshakeSpan completeHandshake(byte[] handshake) {
return null;
}

/**
* Reads the big-endian {@code uint16} at {@code offset}.
* <p>
* <strong>Bounds contract.</strong> Both call sites establish {@code 0 <= offset} and
* {@code offset + 1 < data.length} before calling, so neither read can run past the array:
* <ul>
* <li>{@link #parse(byte[])} evaluates {@code uint16(bytes, pos + 3)} only after
* {@link #recordHeaderVerdict(byte[], int)} returned {@code null}, which happens solely when
* {@code bytes.length - pos >= RECORD_HEADER_LENGTH} — so {@code pos + 4 <= bytes.length - 1}.
* {@code pos} starts at {@code 0} and only ever advances to a {@code recordEnd} that
* {@link #recordBodyVerdict(byte[], int)} already bounded by {@link #MAX_CLIENT_HELLO_BYTES}, so
* it is never negative and never overflows.</li>
* <li>{@link Cursor#readUint16()} evaluates {@code uint16(data, position)} only after
* {@link Cursor#require(int)} asserted {@code position + 2 <= data.length}, raising
* {@link MalformedHelloException} otherwise. {@code position} starts at
* {@link #HANDSHAKE_HEADER_LENGTH} and never decreases ({@link Cursor#seek(int)} refuses a
* backwards target), so it is never negative.</li>
* </ul>
* Both guards live in extracted helpers rather than inline in the reading method, which is why
* the symbolic-execution engine cannot see them; {@code ClientHelloSniParserTest} pins the
* {@code parse} guard at the exact one-byte-short-of-a-record-header boundary, on the first
* record and on the loop-back to a later record.
*
* @param data the buffer to read from
* @param offset the offset of the high-order byte; the caller guarantees {@code offset + 1} is
* within {@code data}
* @return the unsigned 16-bit big-endian value at {@code offset}
*/
private static int uint16(byte[] data, int offset) {
return ((data[offset] & UINT8_MASK) << 8) | (data[offset + 1] & UINT8_MASK);
int high = data[offset] & UINT8_MASK; // NOSONAR javabugs:S6466 - caller-guarded, see Javadoc
int low = data[offset + 1] & UINT8_MASK; // NOSONAR javabugs:S6466 - caller-guarded, see Javadoc
return (high << 8) | low;
}

/**
* Reads the big-endian {@code uint24} at {@code offset}. The sole caller
* {@link #completeHandshake(byte[])} has already returned {@link HandshakeSpan#INCOMPLETE} unless
* {@code handshake.length >= HANDSHAKE_HEADER_LENGTH}, so offsets {@code 1..3} are always within
* the buffer.
*
* @param data the buffer to read from
* @param offset the offset of the most significant byte
* @return the unsigned 24-bit big-endian value at {@code offset}
*/
private static int uint24(byte[] data, int offset) {
return ((data[offset] & UINT8_MASK) << 16)
| ((data[offset + 1] & UINT8_MASK) << 8)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,17 +61,6 @@ class AuthenticationStageTest {
private static final String SESSION_ID = "opaque-session-id";
private static final String MEDIATED_TOKEN = "mediated-access-token";

@Test
@DisplayName("passes a require:none route without inspecting any token")
void passesRequireNone() {
// Arrange
AuthenticationStage stage = stageFor(TestTokenGenerators.accessTokens().next());
PipelineRequest request = request(authConfig("none", List.of()), Map.of());

// Act + Assert
assertDoesNotThrow(() -> stage.process(request));
}

@Test
@DisplayName("passes a require:none route without ever resolving the lazy validator")
void passesRequireNoneWithoutResolvingValidator() {
Expand Down
Loading
Loading