mirror of
https://github.com/gravitational/teleport.git
synced 2026-10-12 02:15:01 +00:00
Julia Ogris
b887107e7f
* appresource: Add path tokenizer and validator Create the `lib/appresource` package with the request-path tokenizer and wire-form validator, promoted from the sketch branch. `Tokenize` validates a raw request path and splits it into segments without decoding the bytes it forwards. A separate decode is used only as a validation view. Tokenize rejects any byte outside the RFC 3986 path set less ";", admits the encoded separator `%2F`/`%2f` and percent-encoded non-ASCII content as the only escapes, rejects "." and ".." segments and "//" on a view that also decodes the encoded separator, and requires each segment, decoded once, to be valid UTF-8, NFKC-stable, and made of graphic runes. Together these prevent the canonicalization and fold bypasses where an upstream decodes or normalizes a path onto a segment the matcher never saw. Paths over 8 KiB are rejected. The package is a leaf so the app agent, role validation in `lib/services`, and tctl can all import it. Node matching, captures, and the encoded-slash grammar follow in later changes. * appresource: Validate path content per segment The NFKC and graphic-rune checks ran on the whole path body with the encoded separator `%2F` kept literal. A combining mark placed right after an encoded slash then composed with that literal `F` (`F` plus U+0307 forms `Ḟ`), so a legitimate path was rejected as not NFKC-normalized even though its real decoded content is normal. Move the content checks into `validateDecoded` so they run per segment on the decode-for-validation view, where `%2F` is a separator. An encoded slash and a raw `/` now validate the same way, and the merge drops a second `decodeSeparators` pass and split. Pin the non-breaking-space reject case to the NFKC rule, since the graphic check would otherwise let it pass for the wrong reason. * appresource: Address tokenizer review feedback Reject a segment that starts with a combining mark. The mark pairs with the previous rune, and net/http.ServeMux still splits on the literal slash, so upstreams may disagree on the segment boundary. RFC 5891 imposes the same rule on the start of a domain label. Re-add the rapid property tests. Unlike the fuzz target, which only replays its seed corpus under go test, they exercise the tokenizer invariants on every run. Register the tokenizer fuzz target in fuzz/oss-fuzz-build.sh and pin the encoded hash %23 (#) as rejected. * appresource: Allow the encoded space %20 in paths Allow %20 as content, alongside the non-ASCII escapes. A space has no raw form in a URL path, so the escape is its only representation, and rejecting it makes resources unreachable with no workaround. Jenkins serves job names with spaces at /job/My%20Job/lastBuild, and SharePoint serves its default library at Shared%20Documents. A space cannot decode into a separator, so it needs neither a rule-global grant nor a per-segment pattern, unlike the encoded separator %2F. Reject a segment that starts or ends with a space. Windows and IIS trim a trailing space from a segment, so ".. " would resolve as ".." and "secret " would resolve as "secret", escaping a carve-out. That is the same fold-to-a-different-segment problem the other content checks prevent, reached by trimming rather than normalization. Reject a segment of only dots and spaces, which folds the former "." and ".." equality check into one rule. A normalizer that strips spaces or a single trailing dot resolves ". ." and "..." the same way. Permit U+0020 alone in isGraphicRune, never the Zs category. U+1680 ogham space mark, U+2028, U+200B and U+0085 are NFKC-stable, so the graphic-rune check is the only place that rejects them. * appresource: Reject a segment's trailing dots Reject a segment whose trailing run of dots and spaces contains a dot. IIS and Windows trim trailing dots and spaces from a path component, so an upstream would resolve `secret.` and `secret%20.` as `secret`, a segment the matcher never saw. The edge-space rule already closes this trim vector for the space; close it for the dot the same way. A leading dot stays allowed, so `/.well-known` remains reachable. Fold the escape predicate shared by `validatePercentEscapes` and `decode` into `isAllowedEscape`, so the two functions visibly accept the same escape set. Clip paths and segments to 256 bytes in error messages, because a rejected path can be kilobytes long and the message survives into logs. Pin the acceptance of a combining mark riding a trailing interior space, which ends the segment with a mark rather than a space byte, so no upstream trim fires on it. * appresource: Reject a leading conjoining jamo Extend the leading-mark check with the NFKC boundary property. A conjoining Hangul jamo is a letter, not a mark, so unicode.IsMark misses it, yet it composes onto the character before it the same way. A segment of "ᆨ" after a segment ending in "가" joins to "각" once an upstream drops the separator, which is the fold the check prevents. Render an illegal byte as a one-byte slice instead of converting it to a rune, so a raw 0xC3 prints as "\xc3" rather than "Ã". * appresource: Simplify godocs and merge byte checks Drop the numbered rule list from the Tokenize godoc, along with the terms NFKC-stable, graphic rune, and conjoining Hangul jamo that a caller does not need in order to call the function correctly. Say what to pass, what comes back, and which paths are rejected, and keep the precise rules on the reject helpers next to the code that enforces them. Add an inline comment where the leading mark check needs both unicode.IsMark and the NFKC boundary property, since neither covers the other. Rename lengthCap to maxPathLength, which reads better at the call site, and merge validatePathBytes and validatePercentEscapes into validateRawBytes so the raw path is walked once rather than twice. Both loops were byte-level and neither depended on the other having finished, and the hex digits the escape branch skips are alphanumeric, so they were always legal path bytes anyway. Swap the formatted testify variants in the property tests for the plain ones, which take the same message and arguments.