mirror of
https://github.com/gravitational/teleport.git
synced 2026-10-25 10:40:02 +08:00
master
4
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
f433c29963 |
appresource: Add rule and decision types (#195)
Export-Source-Commit:
|
||
|
|
4c3cb314ec |
app: Add audit event for role v9 HTTP deny (#194)
Export-Source-Commit:
|
||
|
|
b887107e7f |
appresource: Add path tokenizer and validator (#68795)
* 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. |
||
|
|
2c92419557 |
appresource: Compile and evaluate where clauses (#68796)
* appresource: Compile and evaluate where clauses Create the leaf package `lib/appresource` with the predicate language for HTTP app access. Carry over the where environment from the sketch branch `julia/app/policy-matcher-sketch`. That environment covers the `user.name`, `user.roles`, `user.traits`, and `request.method` bindings, the `true` and `false` constants, and the list and string helpers (`set`, `contains`, `lower`, `upper`, `has_prefix`, `has_suffix`, `has_substring`). Export `CompileWhere` and `Where.Evaluate` as the entry point, so a caller can compile a where clause and match it against a request and a caller identity. Keep the parser unexported. Name the function for the clause it compiles, because a follow-up PR adds a second entry point for the desugared expression form. Pass the evaluation environment as one `Env` struct, so the `vars.` capture namespace can be added as a new field without changing the `Evaluate` signature. Validate the environment before evaluation and reject any method outside the canonical upper-case HTTP method list, so a case-varied method can neither slip past a negated comparison nor match a clause while the backend receives a different spelling. Aggregate `trace.BadParameter` with the parse error in `CompileWhere`, matching the sibling path tokenizer, so a caller can tell a malformed clause in a role from an internal fault without losing the parse error chain. Report no match when evaluation fails, so a caller that drops the error denies rather than allows. Bind `true` and `false` explicitly, because `typical` has no boolean literal, so a bare literal would otherwise fail as an unknown identifier. Leave the `allow_code` and `deny_hint` audit wrappers to the PR that adds the rule types. Both wrappers record against the rule that matched, and checking that a wrapper sits where its record can be attributed correctly needs the rule types to define what one rule's expression is. Compile clauses over identity and method only. Leave path matching and the `vars.` capture namespace to follow-up PRs as well. * appresource: Use net/http method constants Replace the hand-written method strings in `validMethods` with the `net/http` constants, so the list pins the canonical spellings instead of repeating them by hand. * appresource: Add property-based where clause tests Add rapid property tests over generated clauses and environments. The properties are that `CompileWhere` never panics on arbitrary input, that a clause and its negation never agree, that any method outside the canonical list is denied for every clause, that `contains` over `set` matches `slices.Contains`, and that evaluation is deterministic. * appresource: Address review comments Add `NewEnv` so the canonical HTTP method check runs once at the request boundary. A `where` clause is optional on a rule, so a rule with only `methods` and `paths` never builds a `Where` and never reached the check, which meant adding a no-op `where: "true"` to an otherwise identical rule changed the answer for a request spelled `get`. `Evaluate` repeats the check, so an environment a caller assembled itself still cannot authorize such a request. Rename `validateEnv` to `validateMethod`, because the request method is the only thing it inspects. Record why `CompileWhere` builds an aggregate rather than wrapping. The aggregate classifies the result as `BadParameter` for the caller and keeps `typical`'s typed error for `errors.As`, and `Parse` does not return a `BadParameter` for every failure. Drop the claim that `contains` matches the role where-clause function of the same name. `predicate.Contains` type-asserts its first argument to `[]string` and returns false for a string, while `typical` coerces a string to a one-element list, so the same clause text disagrees across the two languages. State that a trait key the identity does not have reads as an empty list, since a mistyped key under a negation then matches every caller. State that the `Evaluate` result is only meaningful when the error is nil, and that sharing an `Env` across evaluations requires the caller not to mutate its slices or map. Cover the clauses that type-check to something other than a bool, operator precedence, and an identity with no traits. Widen the `strconv.Quote` round-trip generator to arbitrary strings, since the restricted alphabet never exercised the escaping the property is about. Rename the tests that did not name the behaviour they check, and check that `CompileWhere` never returns a `Where` with a nil expression. * appresource: Address review feedback on tests Draw a well-formed clause for half the cases in `TestCompileReturnsValueOrError`. Arbitrary strings essentially never parse, so the success branch never ran and the test asserted nothing. Measured 0 successful compiles in 20000 draws of `rapid.String()`, against 2496 in 5000 draws of the mixed generator. Assert that a compile failure returns no `Where`, which no other test checks, since `TestInvalidWhereClause` discards the value. Keep the generator choice in a variable so a well-formed clause that fails to compile is a test failure rather than an early return. Drop `TestEvaluateDeterministic`. Evaluation holds no state and no map iteration reaches the result, so the property cannot fail. The cached parser path it exercised is already reached by the other property tests, which compile the same clause more than once. Use `identifierText` directly rather than aliasing it per call, now that the generator is built once for the package. Say why the two assertions after the `TestEnvBindings` table sit outside it, since they cover a non-GET method and a nil traits map that the fixed request and identity cannot express. Check the error class in `TestUnsupportedMethodRejected` as `TestNewEnv` already does. |