Skip to content

Commit 4bc4036

Browse files
committed
Keep the gateway's own attribution out of the substitution
The fix for the reserved-name collision moved any name equal to `admin-token` or `cluster-token` aside, and applied that to every name reaching the key ID — including the gateway's own. A session driven by the admin API token started reporting `warpgate:admin-token_:<session>`, so the fix written to make that string trustworthy changed it. Two guards caught it by failing their baseline, which is what a baseline is for. The two kinds of name are indistinguishable as strings — that is the defect, not an accident of this code — so the distinction is carried as data. `IdentityHint` is `Gateway` or `Person`; `key_id_field` does the colon substitution for both, and `user_key_id_field` adds the reserved-name substitution for names a person chose. `username()` was already `None` for exactly the two token variants, so the line was already drawn and only needed carrying. The test had checked that the substitution fires and nothing had checked that it does not fire where it must not. Both directions now.
1 parent 37e1c34 commit 4bc4036

5 files changed

Lines changed: 179 additions & 36 deletions

File tree

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
thread 'rustc' panicked at /rustc-dev/14cae681329a63c622a6e1fbe1d30f9374bc51d8/compiler/rustc_middle/src/verify_ich.rs:82:9:
2+
Found unstable fingerprints for evaluate_obligation(fa22464e1fa5c580-d3361cab409b9437): Ok(EvaluatedToOk)
3+
stack backtrace:
4+
0: 0x10c1b919c - <std[44b9ac3670375cbd]::backtrace::Backtrace>::create
5+
1: 0x10a0a9b24 - std[44b9ac3670375cbd]::panicking::update_hook::<alloc[430db6043f5c203]::boxed::Box<rustc_driver_impl[a855bf5576bf6386]::install_ice_hook::{closure#1}>>::{closure#0}
6+
2: 0x10c1cbd78 - std[44b9ac3670375cbd]::panicking::panic_with_hook
7+
3: 0x10c1aee8c - std[44b9ac3670375cbd]::panicking::panic_handler::{closure#0}
8+
4: 0x10c1a37f8 - std[44b9ac3670375cbd]::sys::backtrace::__rust_end_short_backtrace::<std[44b9ac3670375cbd]::panicking::panic_handler::{closure#0}, !>
9+
5: 0x10c1b03b4 - __rustc[f6d2528ad773d790]::rust_begin_unwind
10+
6: 0x10c2960cc - core[8f87d4b81545bdd6]::panicking::panic_fmt
11+
7: 0x10c3777f0 - rustc_middle[b52d7b3deda38e89]::verify_ich::incremental_verify_ich_failed
12+
8: 0x10b683780 - rustc_middle[b52d7b3deda38e89]::verify_ich::incremental_verify_ich::<rustc_middle[b52d7b3deda38e89]::query::erase::ErasedData<[u8; 2usize]>>
13+
9: 0x10b609080 - rustc_query_impl[1ad07c722c830f54]::execution::try_execute_query::<rustc_middle[b52d7b3deda38e89]::query::caches::DefaultCache<rustc_type_ir[d843d4337cf6179c]::canonical::CanonicalQueryInput<rustc_middle[b52d7b3deda38e89]::ty::context::TyCtxt, rustc_middle[b52d7b3deda38e89]::ty::ParamEnvAnd<rustc_middle[b52d7b3deda38e89]::ty::predicate::Predicate>>, rustc_middle[b52d7b3deda38e89]::query::erase::ErasedData<[u8; 2usize]>>, true>
14+
10: 0x10b7aaa4c - rustc_query_impl[1ad07c722c830f54]::query_impl::evaluate_obligation::execute_query_incr::__rust_end_short_backtrace
15+
11: 0x10bf5a79c - <rustc_infer[74f459790732029d]::infer::InferCtxt as rustc_trait_selection[9ed77c9c529de812]::traits::query::evaluate_obligation::InferCtxtExt>::evaluate_obligation
16+
12: 0x10bf5ab58 - <rustc_infer[74f459790732029d]::infer::InferCtxt as rustc_trait_selection[9ed77c9c529de812]::traits::query::evaluate_obligation::InferCtxtExt>::evaluate_obligation_no_overflow
17+
13: 0x10bec403c - <rustc_trait_selection[9ed77c9c529de812]::traits::fulfill::FulfillProcessor>::process_trait_obligation
18+
14: 0x10bf77914 - <rustc_trait_selection[9ed77c9c529de812]::traits::fulfill::FulfillProcessor as rustc_data_structures[a421bec0e96cd1d8]::obligation_forest::ObligationProcessor>::process_obligation
19+
15: 0x10bce8f0c - <rustc_data_structures[a421bec0e96cd1d8]::obligation_forest::ObligationForest<rustc_trait_selection[9ed77c9c529de812]::traits::fulfill::PendingPredicateObligation>>::process_obligations::<rustc_trait_selection[9ed77c9c529de812]::traits::fulfill::FulfillProcessor>
20+
16: 0x10bf64920 - <rustc_trait_selection[9ed77c9c529de812]::traits::fulfill::FulfillmentContext<rustc_trait_selection[9ed77c9c529de812]::traits::FulfillmentError> as rustc_infer[74f459790732029d]::traits::engine::TraitEngine<rustc_trait_selection[9ed77c9c529de812]::traits::FulfillmentError>>::try_evaluate_obligations
21+
17: 0x10c004bb0 - <rustc_trait_selection[9ed77c9c529de812]::traits::fulfill::FulfillmentContext<rustc_trait_selection[9ed77c9c529de812]::traits::FulfillmentError> as rustc_infer[74f459790732029d]::traits::engine::TraitEngine<rustc_trait_selection[9ed77c9c529de812]::traits::FulfillmentError>>::evaluate_obligations_error_on_ambiguity
22+
18: 0x10a41ce28 - rustc_hir_analysis[24136b8774b253f]::check::check::check_coroutine_obligations
23+
19: 0x10b7bdc7c - rustc_query_impl[1ad07c722c830f54]::query_impl::check_coroutine_obligations::invoke_provider_fn::__rust_begin_short_backtrace
24+
20: 0x10b5a4810 - rustc_query_impl[1ad07c722c830f54]::execution::try_execute_query::<rustc_data_structures[a421bec0e96cd1d8]::vec_cache::VecCache<rustc_span[b5a8eb030444d333]::def_id::LocalDefId, rustc_middle[b52d7b3deda38e89]::query::erase::ErasedData<[u8; 1usize]>, rustc_middle[b52d7b3deda38e89]::dep_graph::graph::DepNodeIndex>, true>
25+
21: 0x10b659714 - rustc_query_impl[1ad07c722c830f54]::execution::force_query_dep_node::<rustc_data_structures[a421bec0e96cd1d8]::vec_cache::VecCache<rustc_span[b5a8eb030444d333]::def_id::LocalDefId, rustc_middle[b52d7b3deda38e89]::query::erase::ErasedData<[u8; 1usize]>, rustc_middle[b52d7b3deda38e89]::dep_graph::graph::DepNodeIndex>>
26+
22: 0x10b8e3a8c - <rustc_query_impl[1ad07c722c830f54]::dep_kind_vtables::make_dep_kind_vtable_for_query<rustc_query_impl[1ad07c722c830f54]::query_impl::check_coroutine_obligations::VTableGetter>::{closure#0} as core[8f87d4b81545bdd6]::ops::function::FnOnce<(rustc_middle[b52d7b3deda38e89]::ty::context::TyCtxt, rustc_middle[b52d7b3deda38e89]::dep_graph::dep_node::DepNode, rustc_middle[b52d7b3deda38e89]::dep_graph::serialized::SerializedDepNodeIndex)>>::call_once
27+
23: 0x10ad10294 - <rustc_middle[b52d7b3deda38e89]::dep_graph::graph::DepGraphData>::try_mark_previous_green
28+
24: 0x10ad10224 - <rustc_middle[b52d7b3deda38e89]::dep_graph::graph::DepGraphData>::try_mark_previous_green
29+
25: 0x10ad0ffd0 - <rustc_middle[b52d7b3deda38e89]::dep_graph::graph::DepGraphData>::try_mark_green
30+
26: 0x10b65acec - rustc_query_impl[1ad07c722c830f54]::execution::ensure_can_skip_execution::<rustc_data_structures[a421bec0e96cd1d8]::vec_cache::VecCache<rustc_hir_id[8338dc4af2e58abe]::OwnerId, rustc_middle[b52d7b3deda38e89]::query::erase::ErasedData<[u8; 8usize]>, rustc_middle[b52d7b3deda38e89]::dep_graph::graph::DepNodeIndex>>
31+
27: 0x10b7c437c - rustc_query_impl[1ad07c722c830f54]::query_impl::mir_drops_elaborated_and_const_checked::execute_query_incr::__rust_end_short_backtrace
32+
28: 0x10a9307dc - <rustc_middle[b52d7b3deda38e89]::ty::context::TyCtxt>::par_hir_body_owners::<rustc_interface[59eb29e6d15410d7]::passes::run_required_analyses::{closure#2}::{closure#0}>::{closure#0}
33+
29: 0x10a98a9a4 - <rustc_thread_pool[9f0ce17141eacb56]::job::HeapJob<<rustc_thread_pool[9f0ce17141eacb56]::scope::Scope>::spawn<rustc_data_structures[a421bec0e96cd1d8]::sync::parallel::par_slice<&rustc_span[b5a8eb030444d333]::def_id::LocalDefId, rustc_data_structures[a421bec0e96cd1d8]::sync::parallel::par_for_each_in<&rustc_span[b5a8eb030444d333]::def_id::LocalDefId, &[rustc_span[b5a8eb030444d333]::def_id::LocalDefId], <rustc_middle[b52d7b3deda38e89]::ty::context::TyCtxt>::par_hir_body_owners<rustc_interface[59eb29e6d15410d7]::passes::run_required_analyses::{closure#2}::{closure#0}>::{closure#0}>::{closure#0}::{closure#0}>::{closure#1}::{closure#0}>::{closure#0}> as rustc_thread_pool[9f0ce17141eacb56]::job::Job>::execute
34+
30: 0x10c41efbc - <rustc_thread_pool[9f0ce17141eacb56]::registry::WorkerThread>::wait_or_steal_until_cold
35+
31: 0x10bcd3bb4 - <rustc_thread_pool[9f0ce17141eacb56]::registry::ThreadBuilder>::run
36+
32: 0x10a0a030c - std[44b9ac3670375cbd]::sys::backtrace::__rust_begin_short_backtrace::<<rustc_thread_pool[9f0ce17141eacb56]::ThreadPoolBuilder>::build_scoped<rustc_interface[59eb29e6d15410d7]::util::run_in_thread_pool_with_globals<rustc_interface[59eb29e6d15410d7]::interface::run_compiler<(), rustc_driver_impl[a855bf5576bf6386]::run_compiler::{closure#0}>::{closure#1}, ()>::{closure#5}::{closure#0}::{closure#0}, rustc_interface[59eb29e6d15410d7]::util::run_in_thread_pool_with_globals<rustc_interface[59eb29e6d15410d7]::interface::run_compiler<(), rustc_driver_impl[a855bf5576bf6386]::run_compiler::{closure#0}>::{closure#1}, ()>::{closure#5}::{closure#0}::{closure#1}, ()>::{closure#0}::{closure#0}::{closure#0}, ()>
37+
33: 0x10a0afa40 - <std[44b9ac3670375cbd]::thread::lifecycle::spawn_unchecked<<rustc_thread_pool[9f0ce17141eacb56]::ThreadPoolBuilder>::build_scoped<rustc_interface[59eb29e6d15410d7]::util::run_in_thread_pool_with_globals<rustc_interface[59eb29e6d15410d7]::interface::run_compiler<(), rustc_driver_impl[a855bf5576bf6386]::run_compiler::{closure#0}>::{closure#1}, ()>::{closure#5}::{closure#0}::{closure#0}, rustc_interface[59eb29e6d15410d7]::util::run_in_thread_pool_with_globals<rustc_interface[59eb29e6d15410d7]::interface::run_compiler<(), rustc_driver_impl[a855bf5576bf6386]::run_compiler::{closure#0}>::{closure#1}, ()>::{closure#5}::{closure#0}::{closure#1}, ()>::{closure#0}::{closure#0}::{closure#0}, ()>::{closure#1} as core[8f87d4b81545bdd6]::ops::function::FnOnce<()>>::call_once::{shim:vtable#0}
38+
34: 0x10c1d6ed8 - <std[44b9ac3670375cbd]::sys::thread::unix::Thread>::new::thread_start
39+
35: 0x18861fc58 - __pthread_cond_wait
40+
41+
42+
rustc version: 1.99.0-nightly (14cae6813 2026-07-08)
43+
platform: aarch64-apple-darwin
44+
45+
query stack during panic:
46+
#0 [evaluate_obligation] evaluating trait selection obligation `alloc::boxed::Box<warpgate_vault::error::VaultError>: core::marker::Sync`
47+
#1 [check_coroutine_obligations] verify auto trait bounds for coroutine interior type `server::session::<impl at warpgate-protocol-ssh/src/server/session.rs:156:1: 156:19>::handle_event`
48+
#2 [analysis] running analysis passes on crate `warpgate_protocol_ssh`
49+
end of query stack

tests/mutation_matrix.py

Lines changed: 55 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -282,16 +282,19 @@ def _crates_from_mutations() -> tuple[str, ...]:
282282
(
283283
"certificate: a username cannot shift the key ID fields",
284284
"warpgate-protocol-ssh/src/client/mod.rs",
285-
# Repointed when the attribution substitution joined this function: the
286-
# colon replacement stopped being the whole body and became a binding.
287-
' let field = username.replace(\':\', "_");',
288-
" let field = username.to_owned();",
285+
# Repointed twice: once when the attribution substitution joined this
286+
# function, and again when it left for `user_key_id_field` because it
287+
# was renaming the gateway itself.
288+
" name.replace(':', \"_\")",
289+
" name.to_owned()",
289290
),
290291
(
291292
"certificate: a host-key check names the admin who asked",
292293
"warpgate-protocol-ssh/src/client/mod.rs",
293-
".or_else(|| self.identity_hint.clone())",
294-
".or_else(|| None)",
294+
# Repointed when the hint stopped being a bare string: the gateway's own
295+
# attribution and a person's name are now carried apart.
296+
" None => self.identity_hint.as_ref().map(|hint| match hint {",
297+
" None => None.map(|hint: &IdentityHint| match hint {",
295298
),
296299
(
297300
# And names it honestly: a token is not a person, and the first fix
@@ -846,16 +849,47 @@ def check_replacements_build(mutations, in_flight):
846849
whose mutation does not build has not been measured, whatever the tally
847850
says.
848851
849-
Checked in one pass, not one per guard: every selected replacement is
850-
applied at once and the packages they touch are checked together, so the
851-
whole matrix costs a single `cargo check` rather than forty-three. The
852-
binary crate is included here even though it holds no discriminating tests,
853-
because "it compiles" is a claim about all of the code, not the tested part.
854-
855-
Applying them together also catches a pair that cannot coexist: once the
856-
first has been applied the second's anchor is gone, and two mutations that
857-
overlap in the same span cannot both be measured.
852+
Checked in as few passes as the matrix allows, not one per guard: the
853+
replacements are applied together and the packages they touch are checked
854+
in one `cargo check`, so the whole matrix costs two builds rather than
855+
forty-seven. The binary crate is included even though it holds no
856+
discriminating tests, because "it compiles" is a claim about all of the
857+
code, not the tested part.
858+
859+
More than one pass is needed because some guards deliberately share an
860+
anchor. `certificate: pinned critical options must be present` and
861+
`certificate: a bare name permits without requiring` disable the same line
862+
in opposite directions — one drops the requirement, the other makes it
863+
unconditional — and neither is redundant. Two mutations over one span
864+
cannot be applied at once, so they go in different rounds. The first
865+
version of this check applied everything in one pass and refused the whole
866+
matrix on exactly that, calling a deliberate pair a collision.
867+
868+
An anchor that vanishes *within* a round is still a refusal: that is an
869+
overlap nobody declared, and two guards that quietly rewrite each other
870+
cannot both be measured.
858871
"""
872+
# Guards sharing an anchor go in separate rounds; everything else rides
873+
# along in the first.
874+
rounds: list[list] = []
875+
seen: dict[tuple[str, str], int] = {}
876+
for mutation in mutations:
877+
_, path, old, _ = mutation
878+
turn = seen.get((path, old), 0)
879+
seen[(path, old)] = turn + 1
880+
while len(rounds) <= turn:
881+
rounds.append([])
882+
rounds[turn].append(mutation)
883+
884+
packages = []
885+
for crate in sorted({path.split("/")[0] for _, path, _, _ in mutations}):
886+
packages += ["-p", _package_of(crate)]
887+
888+
for round_number, batch in enumerate(rounds, start=1):
889+
_check_one_round(batch, packages, in_flight, round_number, len(rounds))
890+
891+
892+
def _check_one_round(mutations, packages, in_flight, round_number, rounds):
859893
touched: dict[Path, str] = {}
860894
collided = []
861895
try:
@@ -873,22 +907,20 @@ def check_replacements_build(mutations, in_flight):
873907
if collided:
874908
lines = "\n".join(f" {name}\n in {path}" for name, path in collided)
875909
raise SystemExit(
876-
f"{len(collided)} anchor(s) vanished once their neighbours were "
877-
f"applied, so those guards overlap and cannot both be "
878-
f"measured:\n{lines}"
910+
f"{len(collided)} anchor(s) vanished once their neighbours in "
911+
f"the same round were applied, so those guards overlap without "
912+
f"saying so and cannot both be measured:\n{lines}"
879913
)
880914

881-
packages = []
882-
for crate in sorted({path.split("/")[0] for _, path, _, _ in mutations}):
883-
packages += ["-p", _package_of(crate)]
884915
built = run(["cargo", "check", "--all-targets", *packages])
885916
if built.returncode != 0:
886917
errors = [
887918
line for line in built.stderr.splitlines() if line.startswith("error")
888919
]
889920
raise SystemExit(
890-
"a replacement does not compile, so the guard it belongs to "
891-
"cannot be measured:\n"
921+
f"a replacement does not compile (round {round_number} of "
922+
f"{rounds}), so the guard it belongs to cannot be "
923+
f"measured:\n"
892924
+ "\n".join(f" {line}" for line in errors[:10])
893925
+ "\n\nRepoint it at something that builds before trusting any "
894926
"number from this script."

warpgate-admin/src/api/ssh_connection_test.rs

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ use poem_openapi::{ApiResponse, Object, OpenApi};
33
use russh::keys::PublicKeyBase64;
44
use uuid::Uuid;
55
use warpgate_common::{AdminPermission, WarpgateError};
6-
use warpgate_protocol_ssh::{ConnectionError, RCCommand, RCEvent, RemoteClient, resolve_ssh_chain};
6+
use warpgate_protocol_ssh::{IdentityHint, ConnectionError, RCCommand, RCEvent, RemoteClient, resolve_ssh_chain};
77

88
use super::AdminContext;
99

@@ -56,7 +56,11 @@ impl Api {
5656
// By identity. Which hop answers is decided by which target was
5757
// asked about, not by which happens to be last.
5858
target_id: body.target_id,
59-
requested_by: admin.auth.attribution().to_owned(),
59+
requested_by: if admin.auth.attribution_is_gateway() {
60+
IdentityHint::Gateway(admin.auth.attribution().to_owned())
61+
} else {
62+
IdentityHint::Person(admin.auth.attribution().to_owned())
63+
},
6064
},
6165
None,
6266
));

warpgate-common-http/src/auth.rs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -258,6 +258,16 @@ impl RequestAuthorization {
258258
}
259259
}
260260

261+
/// Whether `attribution()` names the gateway rather than a person.
262+
///
263+
/// `username()` already draws this line — it is `None` for exactly the two
264+
/// token variants. This asks a different question with the same answer: not
265+
/// "who is the user" but "is this string ours, to be kept verbatim".
266+
#[must_use]
267+
pub const fn attribution_is_gateway(&self) -> bool {
268+
matches!(self, Self::AdminToken | Self::ClusterToken)
269+
}
270+
261271
/// Returns a user ID if present in the authorization context or nil UUID
262272
pub const fn user_id(&self) -> Uuid {
263273
match self {

0 commit comments

Comments
 (0)