Add SSH certificate authentication for targets, issued by Vault - #2397
Add SSH certificate authentication for targets, issued by Vault#2397janisdombr wants to merge 81 commits into
Conversation
|
Will be happy to see this merged since it's the only deployment blocker for us due to security concerns. |
050912e to
1578fc6
Compare
|
Went through this again against the current head ( Real progress since the last round: the AWS static-credential gap is now a genuine, well-designed fix, Three things from the last round are still open, each with a concrete fix. Vault issuer errors reaching the SSH client are still truncated to 256 characters rather than sanitized by content, so a policy or role name can survive that length. The fix is a
Also, The bigger thing: stepping back from individual lines, there's a structural question worth resolving before this merges. Under the current design, Vault can't distinguish one target/session from another, One more thing worth knowing before this merges: #2185 also adds Vault integration (a different problem, relocating static secrets into KV rather than issuing certs, but it collides mechanically with this PR in several places, workspace crate registration, This doesn't mean the direction is wrong. Ephemeral, non-stored credentials is the right fix for a real, long-standing gap, and the mechanics here are solid. |
|
Opened #2400 with a concrete design for the authorization question from the review above, rather than posting the whole thing inline here. Short version: the core piece there, identity-templated Vault roles plus per-session scoped child tokens, so Vault verifies the principal instead of trusting what Warpgate asserts, belongs in this PR before merge, not a fast-follow. Without it, this design can plausibly have a worse worst-case blast radius than what it replaces (fleet-wide, non-revocable access versus today's bounded-to-stored-credentials), so it's not a good candidate for shipping as a documented limitation. The remaining hardening in the issue (full IdP-verified non-repudiation, host-binding, revocation) is genuinely separable follow-up work once that baseline is in. |
c00a253 to
bc0fb04
Compare
|
@theredspoon Thank you for the follow-up review!
|
|
Went through the current head (
let http = reqwest::Client::builder().timeout(config.timeout).build()?;That leaves reqwest's default policy in place, which follows redirects and only strips Unbounded buffering + panic in error-body truncation
let body = response.text().await.unwrap_or_default();
let max_len = 256;
let body = if body.len() > max_len {
format!("{}... (truncated)", &body[..max_len])
} else {
body
};
Smaller items
|
|
@theredspoon both fixed, thanks. Redirects are refused outright now, which covers the metadata calls too. The error body is read chunk-wise with a 256-byte cap and truncated lossily, so a split character can't panic it. Chasing that one, I found the success path had Zeroization is end-to-end now: the login body goes through typed structs instead of a The stub validators actually validate now — decoded AWS payload, full Azure coordinates, JWT shape, GCP audience — and have tests of their own. You were right that they were asserting nothing. A pass over the rest turned up a few more: One I'd like your view on: a role with 425fb05. |
4d8e294 to
dd06172
Compare
|
Confirmed everything in On The "hostile Vault already has target access anyway" framing undersells this. The realistic case day to day is more mundane than either: a legitimate, uncompromised Vault, an operator who copies or templates a role with Suggest: default-reject any critical option. Per-target opt-in as a named allow-list of expected option keys, not a bare boolean, and for Two more, from this round:
expires_at: (auth.lease_duration > 0).then(|| {
Instant::now() + Duration::from_secs(auth.lease_duration).saturating_sub(TOKEN_EXPIRY_MARGIN)
}),
IPv6 loopback is misclassified as insecure. One more, lower priority: the AWS path is the one exception to end-to-end zeroization. |
|
Ran a wider architectural sweep across the codebase, not just this PR's diff, then went back and verified every proposed fix against the real code and this PR's own existing patterns. Certificate minting via the host-key-check admin endpoint
The fix needs to be a deterministic signal, not a race. Separately: Vault config doesn't hot-reload
Cloud metadata tokens can transit an ambient proxy
Checked against Vault's actual server source ( Three real things remain from that investigation:
Response-wrapped AppRole secret IDs need the unwrapped value cached
Response wrapping protects one-time delivery of the secret ID, it doesn't force single-use of the secret ID itself. Please resolve by caching the unwrapped secret ID, keyed on the raw file content, reusing it while the file is unchanged and only re-unwrapping when the content actually changes (an operator writing a fresh wrapping token). Keep a distinct error for the real failure case, an unwrap attempt (first use, or after a detected change) that fails because the token is stale or already consumed: Lower priority
|
409e2e5 to
4b825c1
Compare
|
Both rounds are in commit 409e2e5. @theredspoon On critical options you changed my mind. "A hostile Vault already has target access" conflated two different capabilities: force-command isn't extra access, it's laundered attribution, and the target's own log is the thing this feature exists to make trustworthy. The role-write-without-sign path settles it. So: default-reject, per-target allow-list of names with optional pinned values, and the refusal reaches the connecting user rather than a log nobody watches. Everything else landed as you described it checked_add on the lease, url::Host for IPv6, Zeroizing on the AWS path, the allow_user_key_ids message, valid_principals checked with Two places I'm weaker than I'd like, said plainly: The host-key check I took the explicit-intent route, a dedicated RCCommand::CheckHostKey that returns before authenticate_session, final hop only. What I can demonstrate is the leak: revert it and my test fails on connections still open after the request returned. What I could not reproduce is the certificate actually being minted the leaked task stalls before signing in my setup, over a 5s window. That assertion is a guard, not evidence; your 310.6s measurement is the real data point. If you can share how you drove it to sign I'll make it deterministic. The JoinHandle I didn't thread one through. CheckHostKey ends the task, and the admin caller sends an explicit abort afterwards, scoped so ServerSession's graceful disconnect stays untouched. Two mechanisms rather than the third you Tests are 15 Rust unit and 57 integration, up from 12 and 48. Each new one was verified by breaking the code it defends including one that didn't fail on the first attempt, the valid_principals case, which rejects that certificate too. Rewritten to assert who did the refusing. |
Warpgate authenticates to an SSH target with a short-lived OpenSSH user certificate signed on demand by HashiCorp Vault, instead of a private key it stores. The ephemeral keypair is generated per connection and never persisted, so a compromise of the Warpgate host yields nothing a target would accept. Targets trust the CA through TrustedUserCAKeys and need no authorized_keys. The certificate's key ID carries the Warpgate username and session UUID, so the target's own sshd log attributes a proxied session to a person rather than to the gateway. VaultAuth offers workload identity only — kubernetes, AppRole, AWS, Azure and GCP. Each reads its credential from a file or a metadata service, never from the config: a static Vault password would merely relocate the long-lived secret this feature exists to remove. Full compatibility with OpenBao is supported. Verified end to end against real infrastructure — AWS STS, a GCE instance, an Azure VM and a k3d cluster. tests/test_ssh_target_cert_auth.py runs against a stub issuer and needs neither Vault nor a cluster. Discussion: warp-tech#26 Special thanks to @theredspoon for the detailed test, OpenBao evaluation, and security recommendations.
- The admin host-key check ran on into authenticating to the target. On a certificate target that minted a real certificate and opened a real session nobody was attached to, held until the inactivity timeout, with a key ID naming no user. Now a dedicated RCCommand::CheckHostKey stops before authentication, on the final hop only so jump hosts still authenticate. - A certificate could arrive carrying critical options nobody asked for. A force-command there replaces what the user typed while keeping their own principal and key ID on the session, so the target's log attributes it to them. Write access to a Vault role is a lower bar than the right to sign with it, so this is the only place it can be caught. Refused by default; a target may name the options it expects and pin their values. - Nothing checked that the certificate named the account being reached. valid_principals is now verified against the target's username. - A response-wrapped AppRole secret ID was re-unwrapped on every login. A wrapping token is single-use, so every login after the first failed, as a generic denial. The unwrapped secret ID is now cached against the file content, and a genuine unwrap failure names the file and the fix. - lease_duration from Vault fed an unchecked Instant addition, so an oversized lease crashed the process on the login path. Now rejected as a bad response. - Cloud metadata tokens went through the same client as Vault, which honours HTTP_PROXY by default; GCE's hostname defeats a typical IP-based NO_PROXY. Metadata now uses a client built with no_proxy(). - The AWS login path was the one place credentials were not zeroized. - An IPv6 loopback Vault address was classified as a remote plaintext endpoint, because host_str renders it with brackets. - Editing the vault: section had no effect until a restart, alone among config sections. A VaultCell on a watch channel is rebuilt from run.rs; a configuration that fails to build keeps the working client. - A certificate Warpgate itself refused reported "SSH target rejected Warpgate's authentication request", naming the wrong party. It has its own error now, and the reason reaches the connecting user. - A role that forbids key IDs now produces a message naming allow_user_key_ids. Tests: 15 Rust unit and 57 integration, up from 12 and 48; each new one verified by breaking the code it defends. The stub models single-use wrapping tokens, without which the AppRole defect was invisible. Found by @theredspoon's review, which is worth more than the code it corrects.
c5dea27 to
58f831a
Compare
The stub in tests/ is fast and can be made to misbehave, but it only knows what we told it — and two of the defects found in review were invisible for exactly as long as it was the only witness. tests/vault_server.py runs the suite against a real HashiCorp Vault and a real OpenBao, reading requests back out of the server's own audit device, so the payload under assertion is the one the server received. Every behaviour the stub models is now pinned against both. Three defects came out of it: - Every login left a copy of the credential in freed memory. login_payload used serde_json::to_string, whose String grows as it is written and frees each smaller buffer without wiping it; Zeroizing only ever wipes the buffer that survives to the end. Size decides whether it shows: measured with a 4 KiB credential, which is what a Kubernetes service account token or a signed AWS header set actually is. Now serialized into a buffer reserved up front. - The certificate's key ID was never checked against the one requested. A certificate carrying a 64 KiB key ID authenticated normally. The target's sshd logs that field verbatim, and "the target's own log names the person" is the claim this path exists to deliver, so an issuer returning a different one breaks attribution silently. - The reason an authentication failed never reached the person connecting. ConnectionError::Authentication carried no detail; the reason went to the server log and the user got a fixed string. For a certificate refused because it is outside its validity window — the documented clock-skew hazard — that sends whoever is debugging it to check credentials that are fine. The variant now carries its reason and the certificate arm names the window. Also documented: OpenBao refuses to enable an audit device over the API, and its config stanza needs type, path and an options block — a top-level file_path is accepted with a warning and then ignored, which looks exactly like a working audit device that writes nothing. Tests: 16 contract tests across Vault and OpenBao (five versions under WARPGATE_VAULT_MATRIX=full), 8 for certificates a real issuer would never emit, 6 property tests over the validators, and 3 that watch the allocator to check the zeroization claim rather than trusting it.
58f831a to
d818090
Compare
|
Pushed d818090, rebased onto current main. This round came from building the test infrastructure rather than from reading the diff again. tests/vault_server.py runs the suite against a real Vault and a real OpenBao, reading requests back out of the server's own audit device, so
Also OpenBao refuses to enable an audit device over the API, and its config stanza needs Two CI gates are red and neither is from this branch:
I left both alone rather than touch unrelated files in a security PR. |
Three defects, found by reading other projects' advisories and by pointing two tools at this code that had not been used on it before. - A certificate naming more than the target account was accepted. The check asked whether the requested principal was among those returned; Vault returns the requested set verbatim or refuses, so anything extra means the answer did not come from this request. Each extra name is another account the target will accept the certificate for, chosen by whoever answered rather than by the operator, and under AuthorizedPrincipalsFile it need not resemble a username. Now required to be exactly the account asked for. This came from CVE-2024-7594, where an empty valid_principals yielded a certificate good for any user on the host, and CVE-2026-35414, where a comma inside a principal splits one name into two for one of sshd's checks and not the other. The second is also why the rule is "exactly one name" rather than "contains": it notes the attack works when the CA does not reject commas in what it is asked to sign, which is the check Warpgate already makes on the request side. - A certificate could write escape sequences to the connecting user's terminal. The refusal message quotes the critical option's name straight out of the certificate and is printed to the PTY, so a name containing \x1b[2J cleared their screen rather than appearing in the text. Certificate-derived strings are now quoted with {:?}. - The outbound SSH handshake had no bound of its own. A target that completes the TCP connection, sends a valid identification string and then goes silent held the gateway's task, socket and session slot until the *inbound* session's inactivity timeout fired — measured at 55s with that timeout set to 45s. That setting governs how long an idle interactive session may live and is legitimately raised to hours, every one of which extended this hold to match. Bounded now by a dedicated 30s deadline, with an error naming the stage so an operator is not sent to look at credentials. tests/hostile_ssh_server.py is new: six ways of being a bad SSH server, none of which needs Docker. The rest of the suite treats the target as honest, which is the one trust boundary nothing here had pushed on — and russh, which Warpgate is the client half of, has published pre-authentication panics reachable from the peer. Five of the six modes were survived without change. cargo mutants found the fourth problem, in the tests rather than the code: it replaced the error-body reader with one returning an empty string and everything still passed, because the assertions were all upper bounds. Ten mutants survived in that one function. The truncation marker is now pinned from both sides.
|
Pushed 6bd00e1. Three more defects, found by reading other projects' advisories and by pointing two tools at this code that had not been used on it before. A certificate naming more than the target account was accepted. The check asked whether the requested principal was among those returned. Vault returns the requested set verbatim or refuses, so anything extra means the answer did not come from this request and each extra name is another account the target will accept the certificate for, chosen by whoever answered rather than by the operator. Under This came out of two advisories rather than out of the diff: CVE-2024-7594, where an empty A certificate could write escape sequences to the connecting user's terminal. The refusal message quotes the critical option's name straight out of the certificate and is printed to the PTY, so a name containing The outbound SSH handshake had no bound of its own. A target that completes the TCP connection, sends a valid identification string and then goes silent held the gateway's task, socket and session slot until the inbound session's inactivity timeout fired measured at 55s with that timeout set to 45s. That setting governs how long an idle interactive session may live and is legitimately raised to hours, every one of which extended this hold to match. Bounded now by a dedicated 30s deadline, with an error that names the stage.
Checked and clean, for the record: russh 0.62.6 is current against all fourteen of its advisories, and CI is still red on |
|
Ran a final-gate pass with three independent reviewers plus direct verification against real sshd servers, since this round changed enough surface (the real-Vault/real-OpenBao test harness, the critical_options allow-list logic, the CheckHostKey command) to be worth a genuinely fresh look rather than re-confirming what's already fixed. Everything from the last round not mentioned below has been confirmed separately. Two real, previously-unflagged issues, plus a cluster of smaller ones. Host-key check returns the wrong key for any target behind a jump host Already independently reported and being fixed: issue #2412 and its open fix, PR #2413 ( What #2413 doesn't cover, since it's written against Related to that: no certificate gets minted for the jump host today, but that's not a construction guarantee the way it is for the final hop, it's the admin endpoint's abort winning a race against the SSH handshake, the same category of fragility Pinned critical options are only checked when the certificate actually carries them
Smaller items, roughly by severity
One more, separate from the above: the terminal-escape-sequence fix in Given how many of the above are tests passing without exercising what they claim to, worth doing your own adversarial pass over the test suite specifically, not just the production code, and writing down whatever gaps that turns up so they don't quietly regress later. |
… signal Three Vault client tests were latency assertions without meaning to be. The shared test config allowed five seconds per request — never a property under test, since the tests that care about the timeout set their own — but it decided how slow a machine had to be before a login over TLS counted as the behaviour under test failing. After the laptop woke from hibernation five tests failed every run, all on the login, and passed with a longer allowance. It is thirty seconds now. `test_an_endless_error_body_is_not_buffered` separates a reader that stops at the cap from one that waits out the whole stream, so the distance between them is what matters and not either number: they were ten seconds and two apart, and the early stop alone measured 3.8 seconds under load and 7.6 on the woken machine. Sixty against twenty keeps the shape and the room. Installing the crypto provider moves into the helper every TLS test reaches. Three tests installed it and the rest relied on one of those three running first — true in file order, not under cargo's parallelism. The mutation matrix passes the suites the timeout CI passes them. Left at the default, a discriminator could time out before its mutation was applied and the guard be reported `already failing`, which accuses the guard of what was only the harness being impatient. The gateway log tail is now chosen by naming the signal rather than excluding each dependency's tracing in turn. That list lost three times — to the tokio runtime, to the config watcher reporting the data directory every ten seconds, and to rustls printing every handshake — and each time a real failure arrived carrying no information.
Forty-six comment blocks recorded development history rather than reasons: what an earlier version did, what a review argued, what `cargo mutants` found, which round noticed what, what the first draft of a test got wrong. A reader needs the rule and the reason for it; a maintainer meeting this branch for the first time needs its provenance least of all. Each block keeps its invariant and loses the narration. Nothing is removed wholesale, so the saving is modest — 195 comment lines out, 131 back in — and the density is unchanged. That was not the goal: the long blocks that carry the density were read and left alone, because they argue why the code is shaped as it is, and cutting them would take away the reasoning a reviewer needs. No code changed, in any file. One pre-existing defect fixed on the way: the doc comment above `a_far_future_expiry_is_described_rather_than_panicked_on` had two unrelated paragraphs merged into it, so a sentence about the authentication budget ran mid-line into one about certificate expiry.
Hi @Eugeny, how are you? Do you think this PR is still worth merging? I know it grew far beyond what I expected and it does look like an AI battlefield, but every step was checked by us personally. I tried to cut the comments down, but almost all of them turned out to carry reasoning that makes review easier rather than harder, so most of them stayed. |
|
Sorry to keep you waiting! I will review and merge this, but unfortunately my bandwidth is full right now - I'm working hard to get the admin approvals out of the door, and the OpenBao PR is next after that. It might take a bit longer unfortunately but there are no other roadblocks against this PR at the moment |
|
@Eugeny thank you very much for your answer. I'll keep this PR updated as long as you need. |
Brings in the non-admin target page fix (warp-tech#2523), the RDP dithering fix (warp-tech#2524) and the RDP interactive logon option (warp-tech#2526). The only conflict was the git-describe version string in the admin schema, which CI regenerates before comparing.
this is working pretty well with OpenBao |
|
sometimes it returns API error: Vault denied the certificate signing request 
|
|
@sibelius thank you for testing this against OpenBao it's really useful. Sorry about that error message, it's too vague. Warpgate prints it for any 4xx from Vault, so a missing role, a bad principal, an expired token and a rate limit all look the same. And on the "Check host key" path we weren't logging the real reason at all so it wasn't even in the server log. I'm fixing that part now. If you have time could you please check a few things?
Two small things from your screenshot: the target user is |
The message shown to the caller is deliberately sanitised, and every 4xx from Vault collapses into one of them, so a refusal that names its status and body in the error's own Display left no trace anywhere. The session path already logs the full error before sanitising it; this one did not. Reported by @sibelius against OpenBao, where an intermittent "Vault denied the certificate signing request" could not be told apart from a missing role, an expired token or a rate limit.
|
|
@sibelius thank you for your valuable impact. I renamed error messages and now it will show the reason more clearly. |
Every 4xx from the sign endpoint rendered as one sentence, so a role constraint and a rejected token read alike. @sibelius lost a round trip to that against OpenBao, where the targets still on `root` were refused by a role whose allowed_users listed only `bastion`; the message sent him to look at the credential. A 400 now names the signing role, 401 and 403 the credential, and 404 a role or mount that is not there. The same collapse was in five other places, each sending the reader somewhere the fault was not: a CA bundle that cannot be read reported the role and mount as invalid, an unparseable metadata address reported a bad response from a Vault that had never answered, a login timeout blamed Vault for a credential source stalling locally, and AWS reported no credentials for credentials we had deliberately refused.
Ran CodeRabbit over a copy of this branch in the fork, at #2 — same base and same head commit, so it read the diff this pull request carries. It reported fourteen findings and thirteen held up against the source. Four tests could pass without exercising what they were named for, which is the class this branch already built a mutation matrix to hunt. A certificate target was constructed on `localhost` while every other one uses `TARGET_HOST`; on a dual-stack host that connection is refused before authentication, so both assertions held for a reason unrelated to the oversized key ID under test. `_audit` returned an empty list when the docker read failed, which turned `len(server.issued) == issued_before` into a comparison of zero with zero — the same shape the audit device warning next to it describes. `reset()` left `valid_token` behind, so the test asserting the token never reaches the log was reading a token an earlier test's login had produced; it now invalidates, reconnects, and checks that a login actually happened. The lease-zero contract test could not observe its zero: a role without `token_ttl` answers with the system maximum, and only a token with no lease at all is reported as zero. Six messages named something other than what had failed. Three places promised an HTTPS exception for loopback that `validate_address` refuses deliberately, with a comment saying why. `InvalidCertificateTtl` is returned for a value below one second and for one above the ceiling, but its text named only the lower bound, and its client message sent the operator to the role and mount, which were correct. A metadata body that is not UTF-8 borrowed "response is too large", so the log named a size problem that had not occurred. The `VaultAuth` doc comment — and with it the schema hint an operator reads in an editor — said "both methods read their credential from a file" when three of the five read a cloud identity instead. The authentication arm in the SSH session now logs before it writes to the terminal, as the arm below it does; a rejected certificate was the one connection failure leaving no server-side record, and clock skew is exactly the diagnosis that needs the log. The rest is hygiene: a jump-host stall waiting on a throwaway event that `stop()` could not release, a comment describing an inactivity timeout the fixture never set, an f-string with no placeholders, and an unused import.
`--changed` picked guards by the file whose line the mutation edits. A guard's named discriminator usually lives in another file, so weakening that test moved no anchor and the guard went unmeasured — the selection reported it as a no-op, which is exactly the case worth measuring. It now resolves every name in DISCRIMINATES to the file defining it and selects on either signal. Resolution is a static scan rather than `cargo test --list`: the selection runs before anything is built, and making it depend on a compile would put a build in front of the decision about what to build. An unresolvable name is an error, not a silent miss.
Three gaps in what the two sides of a failed session see.
`RCEvent::ConnectionError` reached the browser with no error-level log
anywhere upstream: the connect path logs at `debug!`, which the default
`warpgate=info` filter drops. A certificate failure therefore left the
server with no record at all, only the sanitised text the user saw.
The admin connection test fell back to `format!("{err:#}")`, rendering
the whole anyhow chain into an API response while every other path in
this feature returns `client_error_message`. Now it uses the same one.
The README claimed extensions were constrained when nothing checked
them, did not say `default_extensions` needs `allowed_extensions` to
match, left `ca_public_key` undocumented, and told the reader to grep
for a line that only exists at debug level.
A parity review against how the rest of Warpgate does the same jobs. `warpgate check` said in a comment that it validated the credential and did not read it: construction checks the address, mount and role names, but the Kubernetes token and the AppRole secret ID are read per login. A typo in either path passed the check and then failed every session. It now reads the file — a read rather than a login, because a response-wrapped secret ID is redeemed once and spending that here would break the first real login. The admin API validated the Vault role and nothing else. Two shapes of `allowed_critical_options` describe a target no certificate can open: an empty name, which no CA will ever put in a certificate, and two pins disagreeing about one option, since the connect path enforces every pin rather than the first one. Neither row looks wrong on its own, so the form is the last place either is cheap to see. Two guards added; both discriminate, as does the pre-existing role guard on the same file. A certificate target could not be given `allowed_extensions` through the admin UI at all, so configuring one as a jump host meant editing the database. The form now edits the list. The Vault client is rebuilt on any config change rather than only when the `vault:` section differs. This does not detect a rotated `ca_bundle` — the watcher sees `config.yaml` and nothing else — but it turns "restart the process" into "touch the file". The listener supervisor watches the directory holding its TLS material and filters events; doing the same here is the fuller fix and is not attempted in this commit. `warpgate-vault` was missing from the justfile `projects` list, so `just test` had never run its tests.
`npm run lint` runs the `biome` script, which is `"biome": "biome"` with no arguments — that prints help and exits 0, so it has never checked anything. `biome ci`, which upstream's CI runs, disagreed with the new extensions block.
Not ours, and byte-identical to origin/main until this commit: 3f36f13 left it disagreeing with `biome ci`. Upstream's biome workflow last ran against main in August, so nothing caught the drift, and every PR that merges main inherits the red check. Separate commit so it can be dropped if the maintainer would rather fix it on main himself. Produced by `biome format --write`; nothing else.
The previous commit reformatted a file that needed nothing. `biome ci` in CI runs a pinned 2.5.9, `package-lock.json` resolves 2.5.9, and the installed `node_modules` holds 2.5.6 — which disagrees with 2.5.9 about this construct and reported a file upstream had formatted correctly. Under 2.5.9 the content is byte-identical to origin/main again. The trap is worth naming: `npm run lint` is `"biome": "biome"`, and biome with no arguments prints help and exits 0, so nothing local checks this at all. `npx @biomejs/biome@2.5.9 ci`, from warpgate-web, is what the workflow actually runs.
|
I ran CodeRabbit over an identical copy of the branch on my fork, so it read the same diff this PR carries. First pass gave 14 findings and 13 of them held up. Most were worth having. Four tests could pass without testing what their names claimed — one certificate target was built on Second pass, after the recent work, gave three. One was mine: the extensions editor let you reach "nothing permitted" by removing rows, with no way back to the default. Fixed. The other two are in main rather than in this branch, so I have kept them out of this diff. One of them is real - One thing is still open and it is mine. A test this PR adds, the Nothing structural is queued and the branch is otherwise green, so start whenever suits you. The flake will not change the shape of the diff. If you would rather have the answer first, it should be settled within a day. |
`warpgate-protocol-rdp/src/client/logon.rs` logs the extended logon PDU
through its derived `Debug`:
```rust
InfoData::LogonExtended(extended) => {
debug!(?extended, "Target sent extended logon info");
return;
}
```
In `ironrdp-pdu` 0.9.0, `LogonInfoExtended`
(`src/rdp/session_info/logon_extended.rs:20`) has a plain
`#[derive(Debug)]` and holds `auto_reconnect:
Option<ServerAutoReconnect>`; `ServerAutoReconnect` holds `random_bits:
[u8; 16]`.
Those sixteen bytes are the auto-reconnect random from [MS-RDPBCGR]
2.2.10.1.1.1 — the value a client turns into the HMAC verifier that
re-attaches to the session without presenting credentials again. So the
line writes a reconnection credential into the log.
It is `debug!`, below the default `warpgate=info`, which limits the
blast radius but does not remove it: debug logging is what an operator
turns on to diagnose RDP, which is exactly when this PDU arrives.
This keeps what the line was for — the present-fields flags and the
error information — and replaces the packet itself with a boolean for
whether one was present, which is the diagnostic value it actually
carried.
Noticed while working on #2397.
Three states exist and the form could only reach two. An absent `allowed_extensions` makes the server apply `permit-pty`; an empty list means no extension is permitted at all. `.filter` always yields an array, so removing the last row landed on the empty list — and nothing in the form could return to the absent key. An operator who added an extension and then thought better of it was left with a target that refuses every certificate carrying any extension, and opens no interactive session for one carrying none, with no way to undo it short of the API. The empty list now says that on screen instead of looking like an empty form, and "Use the default" restores the key. Found by CodeRabbit on the fork review PR.
# Conflicts: # warpgate-core/src/services.rs # warpgate-web/src/admin/lib/openapi-schema.json # warpgate-web/src/gateway/lib/openapi-schema.json
6 participants
Warpgate authenticates to an SSH target with a short-lived OpenSSH user certificate signed on demand by HashiCorp Vault, instead of a private key it stores. The ephemeral keypair is generated per connection and never persisted, so a compromise of the Warpgate host yields nothing a target would accept.
Targets trust the CA through TrustedUserCAKeys and need no authorized_keys. The certificate's key ID carries the Warpgate username and session UUID, so the target's own sshd log attributes a proxied session to a person rather than to the gateway.
VaultAuth offers workload identity only — kubernetes, AppRole, AWS, Azure and GCP. Each reads its credential from a file or a metadata service, never from the config: a static Vault password would merely relocate the long-lived secret this feature exists to remove. Full compatibility with OpenBao is supported.
Verified end to end against real infrastructure — AWS STS, a GCE instance, an Azure VM and a k3d cluster.
tests/test_ssh_target_cert_auth.py runs against a stub issuer and needs neither Vault nor a cluster.
Discussion: #26
Special thanks to @theredspoon for the detailed test, OpenBao evaluation, and security recommendations.
Description
...
AI Usage
Choose the level of AI involvement for this PR.
This is not to block AI contributions but rather to speed up PR review (saves time on trying to deduce the logic behind AI hallucinations).