feat(pam): heartbeats - #379
Open
bernie-g wants to merge 10 commits into
Open
Conversation
A Windows-auth SQL Server login has no SQL-managed password, so the database/sql path cannot carry it and the connection test fell back to a plain TCP check. Route ntlm and kerberos through the session proxy's own handshake instead, which already speaks both. The live NTLM test is env-gated and skips unless a server is provided.
The test-connection and WinRM handlers returned 502 with a message for every failure, so a dial timeout and a refused password were indistinguishable upstream. The control plane needs them apart: a refused credential stops the schedule, an unreachable target keeps retrying. Classify at the source, where the driver error still exists, using typed errors where the drivers provide them and message matching only where they don't. WinRM already separated ErrAuth from ErrConnect internally and was flattening both; that now survives the response too.
|
💬 Discussion in Slack: #pr-review-cli-379-feat-gateway-classify-probe-failures-as-auth-or-transport Posted by Review Police — reviews, comments, new commits, and CI failures will stream into this channel. |
11 tasks
![veria-ai[bot]](https://avatars.githubusercontent.com/in/1723776?s=60&v=4){kind=small}
Contributor
PR overviewAll previously flagged issues have been addressed. No open security concerns remain on this pull request. Security reviewNo open security issues remain on this pull request. Fixed/addressed: 2 · PR risk: 0/10 |
bernie-g
changed the title
Contributor
|
| Filename | Overview |
|---|---|
| packages/gateway-v2/test_connection_handler.go | Adds MSSQL Windows-auth routing, but TLS-enabled requests always fail and timed-out proxy handshakes continue running. |
| packages/gateway-v2/test_connection_failure_kind.go | Adds failure classification, but categorizes MSSQL database-access and LDAP authorization failures as refused credentials. |
| packages/gateway-v2/winrm_handler.go | Adds an optional structured failure kind while preserving JSON error handling. |
| packages/pam/handlers/mssql/proxy.go | Exposes the existing authentication handshake for probes without adding context cancellation or deadlines. |
| packages/gateway-v2/test_connection_mssql_auth_test.go | Verifies auth-method routing but does not cover TLS-enabled or stalled-target behavior. |
Reviews (1): Last reviewed commit: "feat(gateway): report whether a probe fa..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
A target that accepted the connection and withheld its PRELOGIN response left the verifier blocked on a read after the caller had already timed out, and since transport failures are retried those goroutines and sockets accumulated.
The branch set EnableTLS without a TLSConfig, so every SSL-enabled NTLM or Kerberos check failed against a working server. Comments across the change are cut back to the few that explain something the code cannot.
ssh: handshake failed wraps everything that goes wrong after TCP connect, including a key-exchange mismatch or a peer hangup, so a host answering port 22 with a broken handshake was reported as a rejected credential and took the account off the check schedule. A genuine rejection still matches the authentication strings nested inside it.
The classifier read Postgres SQLSTATEs, MySQL/MSSQL error numbers, LDAP result codes, and a list of driver error substrings to decide whether a credential was refused or the target was never reached. That list had to grow with every account type and was wrong twice in review, once for cloud outages and once for SSH handshakes. The probes already know the answer: they dial, then they authenticate. Each one now tags its failure with the phase it happened in, so the vendor codes and the substring lists are gone and a new account type needs no classification code at all. This is the same shape gateway-retry.ts already uses upstream, where an established channel decides retryability.
ssh.ServerAuthError is returned by NewServerConn, not by the client, so the type check added with the phase refactor could never match and every SSH failure classified as transport. The client wraps a refused password and a broken handshake in the same ssh: handshake failed text, so the two are only separable by whether the client got as far as offering a credential, which the auth callbacks now record. Adds live_probe_test.go (build tag liveprobe): 36 cases across Postgres, MySQL, MSSQL, MSSQL over NTLM, Redis, MongoDB, LDAP, SSH, and TCP, run against real servers.
enroll.go and systemd.go were already unformatted on main and a blanket gofmt -w reformatted them into this branch.
Both needed a hand-built environment (local containers on fixed ports, a SQL Server on EC2) that nobody else can reproduce, so they documented a verification run rather than testing anything on their own. The phase behaviour they proved is covered by the self-contained tests.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description 📣
The test-connection and WinRM handlers returned the same 502 for a refused credential and an unreachable target, so the control plane could not tell them apart. They now classify the failure at the source and return it as
auth,transport, orunknown.Also routes MSSQL Windows-auth logins through the session proxy's handshake, which already speaks NTLM and Kerberos.
Type ✨
Tests 🛠️
Unit tests for the MSSQL auth routing, plus a live NTLM check (env-gated, skipped in CI) verified against a real SQL Server 2022 instance: correct password authenticates, wrong password is rejected.
go test ./packages/gateway-v2/ ./packages/pam/handlers/mssql/