From 47491fd10fdd0810bac602d8fdc7152d1c1b65cd Mon Sep 17 00:00:00 2001 From: "Matt, Matthias" Date: Fri, 9 Oct 2026 18:35:36 +0200 Subject: [PATCH 1/4] feat(auth): deliver webhook denial messages as SSH user-auth banners An authentication or authorization webhook can now supply an optional user-facing message along with a negative decision (HTTP 200 with success=false and a non-empty message field). The message is preserved through the webhook client and the auth integration layer as a terminal failure and delivered to the connecting SSH client as an SSH_MSG_USERAUTH_BANNER (RFC 4252, section 5.4) immediately before the authentication failure, via the ssh.BannerError mechanism of the pinned golang.org/x/crypto version. Safety rules: - Only webhook-supplied messages ever become banners. Internal, infrastructure, and unavailable errors never do. - A message on a successful response is not a denial and never becomes a banner. - Message-less denials behave exactly as before: no error, no banner. - Message-bearing denials are terminal: no retries, no backend session initialized, and non-200/transport failures keep their fail-closed retry/timeout behavior. Verified with real SSH client connections, including the OpenSSH client in noninteractive use, and covered by regression tests at the webhook client and auth integration seams. Part of #1, closes #3. Signed-off-by: Matt, Matthias --- auth/denial.go | 29 ++ auth/protocol.go | 9 + internal/auth/webhook_client_impl.go | 34 ++ internal/auth/webhook_denial_test.go | 204 ++++++++++ internal/authintegration/banner_test.go | 376 +++++++++++++++++++ internal/authintegration/handler.go | 8 +- internal/authintegration/integration_test.go | 5 +- internal/sshserver/serverImpl.go | 28 +- 8 files changed, 686 insertions(+), 7 deletions(-) create mode 100644 auth/denial.go create mode 100644 internal/auth/webhook_denial_test.go create mode 100644 internal/authintegration/banner_test.go diff --git a/auth/denial.go b/auth/denial.go new file mode 100644 index 00000000..7ee09ea2 --- /dev/null +++ b/auth/denial.go @@ -0,0 +1,29 @@ +package auth + +import "errors" + +// UserFacingDenial is an error carrying a user-facing message that an +// authentication or authorization webhook supplied along with a negative +// decision (HTTP 200 with success = false and a non-empty message field). +// +// The message is supplied by the webhook and is safe to be displayed to the +// connecting user during authentication, e.g. as an SSH user authentication +// banner as described in RFC 4252, section 5.4. Errors that do not carry a +// UserFacingDenial (e.g. infrastructure or internal errors) must never be +// displayed to the user. +type UserFacingDenial struct { + // Message is the user-facing denial message supplied by the webhook. + Message string +} + +// Error implements the error interface and returns the user-facing message. +func (e *UserFacingDenial) Error() string { + return e.Message +} + +// IsUserFacingDenial reports whether err carries a user-facing denial message +// supplied by an authentication or authorization webhook. +func IsUserFacingDenial(err error) bool { + var denial *UserFacingDenial + return errors.As(err, &denial) +} diff --git a/auth/protocol.go b/auth/protocol.go index f0450a7b..fb165756 100644 --- a/auth/protocol.go +++ b/auth/protocol.go @@ -46,6 +46,15 @@ type ResponseBody struct { // required: true // in: body Success bool `json:"success"` + + // Message is an optional user-facing message describing a negative + // decision (success = false). It is delivered to the connecting client + // during authentication (e.g. as an SSH user authentication banner as + // described in RFC 4252, section 5.4) and must therefore be safe to be + // displayed to an unauthenticated user. A message on a successful response + // is not a denial and is not delivered as a banner. Messages on + // infrastructure failures (non-200 responses) are never delivered. + Message string `json:"message,omitempty"` } // Response is the full HTTP authentication response. diff --git a/internal/auth/webhook_client_impl.go b/internal/auth/webhook_client_impl.go index 9a48f943..476d9294 100644 --- a/internal/auth/webhook_client_impl.go +++ b/internal/auth/webhook_client_impl.go @@ -141,6 +141,16 @@ loop: authenticatedMeta.Merge(authResponse.ConnectionAuthenticatedMetadata) client.logAuthResponse(logger, method, authResponse, lastLabels, authenticatedMeta.RemoteAddress.IP) + if !authResponse.Success && authResponse.Message != "" { + // The webhook made a negative decision and supplied a user-facing + // message. This is a terminal failure carrying that message, not a + // retryable backend error. + return &webhookClientContext{ + authenticatedMeta, + false, + client.userFacingDenialError(authType, authResponse.Message), + } + } return &webhookClientContext{ authenticatedMeta, authResponse.Success, @@ -170,6 +180,19 @@ func (client *webhookClient) logAttempt(logger log.Logger, method string, lastLa client.backendRequestsMetric.Increment(lastLabels...) } +// userFacingDenialError creates an error for a webhook denial that supplied a +// user-facing message. The error chains to an auth.UserFacingDenial carrying the +// message so downstream layers can deliver it to the connecting user. +func (client *webhookClient) userFacingDenialError(authType string, userMessage string) error { + return message.WrapUser( + &auth.UserFacingDenial{Message: userMessage}, + message.EAuthFailed, + userMessage, + "Auth webhook denied %s and supplied a user-facing message", + authType, + ) +} + func (client *webhookClient) logAndReturnPermanentFailure( meta metadata.ConnectionAuthPendingMetadata, lastError error, @@ -310,6 +333,17 @@ loop: authenticatedMeta := meta.Authenticated("") authenticatedMeta.Merge(authResponse.ConnectionAuthenticatedMetadata) client.logAuthzResponse(authenticatedMeta, logger, authResponse, lastLabels) + + if !authResponse.Success && authResponse.Message != "" { + // The webhook made a negative decision and supplied a user-facing + // message. This is a terminal failure carrying that message, not a + // retryable backend error. + return &webhookClientContext{ + authenticatedMeta, + false, + client.userFacingDenialError("authorization", authResponse.Message), + } + } return &webhookClientContext{ authenticatedMeta, authResponse.Success, diff --git a/internal/auth/webhook_denial_test.go b/internal/auth/webhook_denial_test.go new file mode 100644 index 00000000..c356ab7d --- /dev/null +++ b/internal/auth/webhook_denial_test.go @@ -0,0 +1,204 @@ +package auth_test + +import ( + "fmt" + "net/http" + "net/http/httptest" + "testing" + "time" + + "github.com/stretchr/testify/assert" + + auth2 "go.containerssh.io/containerssh/auth" + "go.containerssh.io/containerssh/config" + "go.containerssh.io/containerssh/internal/auth" + "go.containerssh.io/containerssh/internal/geoip/dummy" + "go.containerssh.io/containerssh/internal/metrics" + "go.containerssh.io/containerssh/log" + "go.containerssh.io/containerssh/metadata" +) + +// startAuthFixture starts a webhook HTTP server that answers every authentication +// and authorization request with the provided JSON response body. +func startAuthFixture(t *testing.T, responseBody string) *httptest.Server { + t.Helper() + server := httptest.NewServer(http.HandlerFunc(func(writer http.ResponseWriter, _ *http.Request) { + writer.Header().Set("Content-Type", "application/json") + _, err := writer.Write([]byte(responseBody)) + assert.NoError(t, err) + })) + t.Cleanup(server.Close) + return server +} + +// newFixtureWebhookClient creates a webhook client for the fixture server at url. +func newFixtureWebhookClient(t *testing.T, url string) auth.WebhookClient { + t.Helper() + client, err := auth.NewWebhookClient( + auth.AuthenticationTypeAll, + config.AuthWebhookClientConfig{ + HTTPClientConfiguration: config.HTTPClientConfiguration{ + URL: url, + Timeout: 2 * time.Second, + }, + AuthTimeout: 30 * time.Second, + }, + log.NewTestLogger(t), + metrics.New(dummy.New()), + ) + assert.NoError(t, err) + return client +} + +// TestPubKeyDenialWithMessageIsTerminalAndCarriesMessage verifies that an HTTP 200 +// response with success=false and a user-facing message results in a terminal +// authentication failure carrying that message. The message must not cause +// retries and must complete promptly. +func TestPubKeyDenialWithMessageIsTerminalAndCarriesMessage(t *testing.T) { + denialMessage := "unknown pod template 'unknown' — use a configured template username" + server := startAuthFixture( + t, + fmt.Sprintf(`{"success":false,"message":%q}`, denialMessage), + ) + client := newFixtureWebhookClient(t, server.URL) + + start := time.Now() + authContext := client.PubKey( + metadata.NewTestAuthenticatingMetadata("unknown"), + auth2.PublicKey{PublicKey: "ssh-rsa asdf"}, + ) + elapsed := time.Since(start) + + assert.False(t, authContext.Success(), "a denial must not authenticate") + assert.Less( + t, + elapsed, + 5*time.Second, + "a policy denial must complete promptly and must not enter retry handling", + ) + + var denial *auth2.UserFacingDenial + if assert.ErrorAs( + t, + authContext.Error(), + &denial, + "a message-bearing denial must carry the user-facing message", + ) { + assert.Equal(t, denialMessage, denial.Message) + } +} + +// TestPubKeyDenialWithoutMessageKeepsExistingBehavior verifies that message-less +// denials behave exactly as before: a plain failure without an error. +func TestPubKeyDenialWithoutMessageKeepsExistingBehavior(t *testing.T) { + server := startAuthFixture(t, `{"success":false}`) + client := newFixtureWebhookClient(t, server.URL) + + authContext := client.PubKey( + metadata.NewTestAuthenticatingMetadata("foo"), + auth2.PublicKey{PublicKey: "ssh-rsa asdf"}, + ) + + assert.False(t, authContext.Success()) + assert.NoError( + t, + authContext.Error(), + "a message-less denial must not acquire an error", + ) + assert.False( + t, + auth2.IsUserFacingDenial(authContext.Error()), + "a message-less denial must not carry a user-facing message", + ) +} + +// TestPubKeySuccessWithMessageSucceeds verifies that a message on a successful +// response does not become a denial. +func TestPubKeySuccessWithMessageSucceeds(t *testing.T) { + server := startAuthFixture(t, `{"success":true,"message":"welcome"}`) + client := newFixtureWebhookClient(t, server.URL) + + authContext := client.PubKey( + metadata.NewTestAuthenticatingMetadata("foo"), + auth2.PublicKey{PublicKey: "ssh-rsa asdf"}, + ) + + assert.True(t, authContext.Success()) + assert.NoError(t, authContext.Error()) +} + +// TestAuthzDenialWithMessageIsTerminalAndCarriesMessage verifies that a message-bearing +// authorization denial is a terminal failure carrying the user-facing message. +func TestAuthzDenialWithMessageIsTerminalAndCarriesMessage(t *testing.T) { + denialMessage := "pod limit (3) reached — delete a box" + server := startAuthFixture( + t, + fmt.Sprintf(`{"success":false,"message":%q}`, denialMessage), + ) + client := newFixtureWebhookClient(t, server.URL) + + start := time.Now() + authzContext := client.Authorize( + metadata.NewTestAuthenticatingMetadata("foo").Authenticated("foo"), + ) + elapsed := time.Since(start) + + assert.False(t, authzContext.Success()) + assert.Less( + t, + elapsed, + 5*time.Second, + "a policy denial must complete promptly and must not enter retry handling", + ) + + var denial *auth2.UserFacingDenial + if assert.ErrorAs( + t, + authzContext.Error(), + &denial, + "a message-bearing denial must carry the user-facing message", + ) { + assert.Equal(t, denialMessage, denial.Message) + } +} + +// TestWebhookNon200DoesNotBecomeUserFacingDenial verifies that infrastructure +// failures (non-200 responses) keep their fail-closed behavior and never turn +// into user-facing denial messages. +func TestWebhookNon200DoesNotBecomeUserFacingDenial(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(writer http.ResponseWriter, _ *http.Request) { + writer.WriteHeader(500) + })) + t.Cleanup(server.Close) + + client, err := auth.NewWebhookClient( + auth.AuthenticationTypeAll, + config.AuthWebhookClientConfig{ + HTTPClientConfiguration: config.HTTPClientConfiguration{ + URL: server.URL, + Timeout: 2 * time.Second, + }, + AuthTimeout: 1 * time.Second, + }, + log.NewTestLogger(t), + metrics.New(dummy.New()), + ) + assert.NoError(t, err) + + authContext := client.PubKey( + metadata.NewTestAuthenticatingMetadata("foo"), + auth2.PublicKey{PublicKey: "ssh-rsa asdf"}, + ) + + assert.False(t, authContext.Success()) + assert.Error( + t, + authContext.Error(), + "a non-200 response must remain a fail-closed backend error", + ) + assert.False( + t, + auth2.IsUserFacingDenial(authContext.Error()), + "an infrastructure failure must not become a user-facing denial", + ) +} diff --git a/internal/authintegration/banner_test.go b/internal/authintegration/banner_test.go new file mode 100644 index 00000000..30882000 --- /dev/null +++ b/internal/authintegration/banner_test.go @@ -0,0 +1,376 @@ +package authintegration_test + +import ( + "bytes" + "context" + "crypto/ed25519" + "crypto/rand" + "fmt" + "net/http" + "net/http/httptest" + "os/exec" + "strings" + "sync" + "sync/atomic" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "golang.org/x/crypto/ssh" + + "go.containerssh.io/containerssh/config" + "go.containerssh.io/containerssh/internal/authintegration" + "go.containerssh.io/containerssh/internal/geoip/dummy" + "go.containerssh.io/containerssh/internal/metrics" + "go.containerssh.io/containerssh/internal/sshserver" + "go.containerssh.io/containerssh/internal/structutils" + "go.containerssh.io/containerssh/internal/test" + "go.containerssh.io/containerssh/log" + "go.containerssh.io/containerssh/service" +) + +// authWebhookFixture is a controlled authentication webhook fixture. It answers +// requests to /password, /pubkey, and /authz with the configured JSON response +// bodies. +type authWebhookFixture struct { + t *testing.T + mutex *sync.Mutex + responses map[string]string + failPaths map[string]int + httpServer *httptest.Server +} + +func startAuthWebhookFixture(t *testing.T) *authWebhookFixture { + fixture := &authWebhookFixture{ + t: t, + mutex: &sync.Mutex{}, + responses: map[string]string{}, + failPaths: map[string]int{}, + } + fixture.httpServer = httptest.NewServer(http.HandlerFunc(func(writer http.ResponseWriter, request *http.Request) { + parts := strings.Split(request.URL.Path, "/") + path := parts[len(parts)-1] + fixture.mutex.Lock() + defer fixture.mutex.Unlock() + status, ok := fixture.failPaths[path] + if !ok { + status = http.StatusOK + } + response, ok := fixture.responses[path] + if !ok { + response = `{"success":false}` + } + writer.Header().Set("Content-Type", "application/json") + writer.WriteHeader(status) + _, err := writer.Write([]byte(response)) + assert.NoError(fixture.t, err) + })) + t.Cleanup(fixture.httpServer.Close) + return fixture +} + +// setResponse sets the JSON response body for a request path (password, pubkey, +// or authz). +func (f *authWebhookFixture) setResponse(path string, response string) { + f.mutex.Lock() + defer f.mutex.Unlock() + f.responses[path] = response +} + +// setFailure sets a failing HTTP status code for a request path to simulate a +// webhook outage. +func (f *authWebhookFixture) setFailure(path string, status int) { + f.mutex.Lock() + defer f.mutex.Unlock() + f.failPaths[path] = status +} + +func (f *authWebhookFixture) URL() string { + return f.httpServer.URL +} + +// startFixtureSSHServer starts an SSH server wired against the fixture webhook +// with public key authentication and authorization enabled. It returns the SSH +// configuration and the backend so tests can assert on backend interactions. +func startFixtureSSHServer(t *testing.T, fixture *authWebhookFixture) (config.SSHConfig, *testBackend) { + backend := &testBackend{} + webhookConfig := config.AuthWebhookClientConfig{ + HTTPClientConfiguration: config.HTTPClientConfiguration{ + URL: fixture.URL(), + Timeout: 2 * time.Second, + }, + AuthTimeout: 2 * time.Second, + } + handler, _, err := authintegration.New( + config.AuthConfig{ + PublicKeyAuth: config.PublicKeyAuthConfig{ + Method: config.PubKeyAuthMethodWebhook, + Webhook: webhookConfig, + }, + Authz: config.AuthzConfig{ + Method: config.AuthzMethodWebhook, + Webhook: webhookConfig, + }, + }, + backend, + log.NewTestLogger(t), + metrics.New(dummy.New()), + authintegration.BehaviorNoPassthrough, + ) + assert.NoError(t, err) + + sshServerConfig := config.SSHConfig{} + structutils.Defaults(&sshServerConfig) + assert.NoError(t, sshServerConfig.GenerateHostKey()) + port := test.GetNextPort(t, "SSH") + sshServerConfig.Listen = fmt.Sprintf("127.0.0.1:%d", port) + srv, err := sshserver.New(sshServerConfig, handler, log.NewTestLogger(t)) + assert.NoError(t, err) + + lifecycle := service.NewLifecycle(srv) + ready := make(chan struct{}) + lifecycle.OnRunning( + func(s service.Service, l service.Lifecycle) { + ready <- struct{}{} + }, + ) + go func() { + assert.NoError(t, lifecycle.Run()) + }() + <-ready + t.Cleanup(func() { + lifecycle.Stop(context.Background()) + }) + return sshServerConfig, backend +} + +// sshConnectionResult captures the observable result of a real SSH connection +// attempt: the dial error and all user authentication banners received. +type sshConnectionResult struct { + dialError error + banners []string +} + +// connectWithPublicKey performs a real SSH connection attempt using public key +// authentication with the provided username and collects any user authentication +// banners the server sends. +func connectWithPublicKey(t *testing.T, cfg config.SSHConfig, username string) sshConnectionResult { + t.Helper() + _, privateKey, err := ed25519.GenerateKey(rand.Reader) + assert.NoError(t, err) + signer, err := ssh.NewSignerFromKey(privateKey) + assert.NoError(t, err) + + result := sshConnectionResult{} + clientConfig := ssh.ClientConfig{ + User: username, + Auth: []ssh.AuthMethod{ + ssh.PublicKeys(signer), + }, + HostKeyCallback: ssh.InsecureIgnoreHostKey(), //nolint:gosec + Timeout: 10 * time.Second, + BannerCallback: func(message string) error { + result.banners = append(result.banners, message) + return nil + }, + } + client, err := ssh.Dial("tcp", cfg.Listen, &clientConfig) + result.dialError = err + if client != nil { + assert.NoError(t, client.Close()) + } + return result +} + +// TestPubKeyDenialWithMessageReachesClientAsBanner is the tracer bullet for +// readable denials: a webhook that answers HTTP 200 with success=false and a +// user-facing message causes the connecting SSH client to see exactly that text +// as an SSH user authentication banner immediately before the authentication +// failure. +func TestPubKeyDenialWithMessageReachesClientAsBanner(t *testing.T) { + denialMessage := "unknown pod template 'unknown' — use a configured template username" + fixture := startAuthWebhookFixture(t) + fixture.setResponse( + "pubkey", + fmt.Sprintf(`{"success":false,"message":%q}`, denialMessage), + ) + fixture.setResponse("authz", `{"success":true}`) + + start := time.Now() + sshServerConfig, backend := startFixtureSSHServer(t, fixture) + result := connectWithPublicKey(t, sshServerConfig, "unknown") + elapsed := time.Since(start) + + assert.Error( + t, + result.dialError, + "a message-bearing denial must still fail authentication", + ) + assert.Equal( + t, + []string{denialMessage}, + result.banners, + "the client must see the exact webhook message as a pre-failure banner", + ) + assert.Less( + t, + elapsed, + 5*time.Second, + "a healthy permanent denial must complete promptly", + ) + assert.Equal( + t, + int32(0), + atomic.LoadInt32(&backend.handshakeSuccesses), + "a denial must not initialize a backend session", + ) +} + +// TestPubKeyDenialWithoutMessageHasNoBanner verifies that message-less denials +// behave exactly as before: authentication fails without a banner. +func TestPubKeyDenialWithoutMessageHasNoBanner(t *testing.T) { + fixture := startAuthWebhookFixture(t) + fixture.setResponse("pubkey", `{"success":false}`) + + sshServerConfig, backend := startFixtureSSHServer(t, fixture) + result := connectWithPublicKey(t, sshServerConfig, "foo") + + assert.Error(t, result.dialError) + assert.Empty( + t, + result.banners, + "a message-less denial must not acquire a banner", + ) + assert.Equal( + t, + int32(0), + atomic.LoadInt32(&backend.handshakeSuccesses), + "a denial must not initialize a backend session", + ) +} + +// TestPubKeySuccessWithMessageAuthenticates verifies that a message on a +// successful webhook response does not become a denial banner and authentication +// still succeeds. +func TestPubKeySuccessWithMessageAuthenticates(t *testing.T) { + fixture := startAuthWebhookFixture(t) + fixture.setResponse("pubkey", `{"success":true,"message":"welcome"}`) + fixture.setResponse("authz", `{"success":true}`) + + sshServerConfig, backend := startFixtureSSHServer(t, fixture) + result := connectWithPublicKey(t, sshServerConfig, "foo") + + assert.NoError( + t, + result.dialError, + "a message on a successful response must not deny authentication", + ) + assert.Empty( + t, + result.banners, + "a successful authentication must not produce a denial banner", + ) + assert.Equal( + t, + int32(1), + atomic.LoadInt32(&backend.handshakeSuccesses), + "a successful authentication must initialize the backend", + ) +} + +// TestWebhookOutageDoesNotProduceBanner verifies that a webhook outage keeps +// its fail-closed behavior and never surfaces an internal error as a banner. +func TestWebhookOutageDoesNotProduceBanner(t *testing.T) { + fixture := startAuthWebhookFixture(t) + fixture.setFailure("pubkey", http.StatusInternalServerError) + + sshServerConfig, backend := startFixtureSSHServer(t, fixture) + result := connectWithPublicKey(t, sshServerConfig, "foo") + + assert.Error( + t, + result.dialError, + "a webhook outage must fail closed", + ) + assert.Empty( + t, + result.banners, + "an infrastructure failure must never appear as a banner", + ) + assert.Equal( + t, + int32(0), + atomic.LoadInt32(&backend.handshakeSuccesses), + "a webhook outage must not initialize a backend session", + ) +} + +// TestOpenSSHClientSeesDenialBanner verifies, with the actual OpenSSH client in +// noninteractive use, that a message-bearing denial is printed as the banner and +// authentication still fails. The test is skipped when no OpenSSH client is +// available. +func TestOpenSSHClientSeesDenialBanner(t *testing.T) { + sshBinary, err := exec.LookPath("ssh") + if err != nil { + t.Skip("OpenSSH client not available, skipping OpenSSH banner verification") + } + sshKeyGen, err := exec.LookPath("ssh-keygen") + if err != nil { + t.Skip("ssh-keygen not available, skipping OpenSSH banner verification") + } + + tempDir := t.TempDir() + keyFile := tempDir + "/testkey" + if out, err := exec.Command(sshKeyGen, "-q", "-t", "ed25519", "-N", "", "-f", keyFile).CombinedOutput(); err != nil { + assert.FailNow(t, "failed to generate test key", string(out)) + return + } + + denialMessage := "unknown pod template 'unknown' — use a configured template username" + fixture := startAuthWebhookFixture(t) + fixture.setResponse( + "pubkey", + fmt.Sprintf(`{"success":false,"message":%q}`, denialMessage), + ) + fixture.setResponse("authz", `{"success":true}`) + + sshServerConfig, backend := startFixtureSSHServer(t, fixture) + listens := strings.Split(sshServerConfig.Listen, ":") + + ctx, cancel := context.WithTimeout(context.Background(), 15*time.Second) + defer cancel() + cmd := exec.CommandContext(ctx, sshBinary, + "-o", "BatchMode=yes", + "-o", "ConnectTimeout=10", + "-o", "StrictHostKeyChecking=no", + "-o", "UserKnownHostsFile=/dev/null", + "-o", "IdentitiesOnly=yes", + "-o", "PreferredAuthentications=publickey", + "-o", "PasswordAuthentication=no", + "-o", "KbdInteractiveAuthentication=no", + "-i", keyFile, + "-p", listens[len(listens)-1], + "unknown@127.0.0.1", "true", + ) + var stderr bytes.Buffer + cmd.Stderr = &stderr + runErr := cmd.Run() + + assert.Error( + t, + runErr, + "the OpenSSH client must still fail authentication on a denial", + ) + assert.Contains( + t, + stderr.String(), + denialMessage, + "the OpenSSH client must print the denial message as the banner", + ) + assert.Equal( + t, + int32(0), + atomic.LoadInt32(&backend.handshakeSuccesses), + "a denial must not initialize a backend session", + ) +} diff --git a/internal/authintegration/handler.go b/internal/authintegration/handler.go index b544c5c8..1f77e3b1 100644 --- a/internal/authintegration/handler.go +++ b/internal/authintegration/handler.go @@ -137,7 +137,7 @@ func (h *networkConnectionHandler) OnAuthPassword( authContext := h.passwordAuthenticator.Password(meta, password) h.authContext = authContext if !authContext.Success() { - if authContext.Error() != nil { + if authContext.Error() != nil && !auth2.IsUserFacingDenial(authContext.Error()) { if h.behavior == BehaviorPassthroughOnUnavailable { return h.backend.OnAuthPassword(meta, password) } @@ -146,7 +146,7 @@ func (h *networkConnectionHandler) OnAuthPassword( if h.behavior == BehaviorPassthroughOnFailure { return h.backend.OnAuthPassword(meta, password) } - return sshserver.AuthResponseFailure, meta.AuthFailed(), nil + return sshserver.AuthResponseFailure, meta.AuthFailed(), authContext.Error() } if h.behavior == BehaviorPassthroughOnSuccess { return h.backend.OnAuthPassword(meta, password) @@ -172,7 +172,7 @@ func (h *networkConnectionHandler) OnAuthPubKey( authContext := h.publicKeyAuthenticator.PubKey(meta, pubKey) h.authContext = authContext if !authContext.Success() { - if authContext.Error() != nil { + if authContext.Error() != nil && !auth2.IsUserFacingDenial(authContext.Error()) { if h.behavior == BehaviorPassthroughOnUnavailable { return h.backend.OnAuthPubKey(meta, pubKey) } @@ -235,7 +235,7 @@ func (h *networkConnectionHandler) OnAuthKeyboardInteractive( ) h.authContext = authContext if !authContext.Success() { - if authContext.Error() != nil { + if authContext.Error() != nil && !auth2.IsUserFacingDenial(authContext.Error()) { if h.behavior == BehaviorPassthroughOnUnavailable { return h.backend.OnAuthKeyboardInteractive(meta, challenge) } diff --git a/internal/authintegration/integration_test.go b/internal/authintegration/integration_test.go index 4308fa9e..9b5a0b2e 100644 --- a/internal/authintegration/integration_test.go +++ b/internal/authintegration/integration_test.go @@ -3,6 +3,7 @@ package authintegration_test import ( "context" "fmt" + "sync/atomic" "testing" "time" @@ -142,6 +143,7 @@ func testConnection(t *testing.T, username string, authMethod ssh.AuthMethod, ss // region Backend type testBackend struct { sshserver.AbstractNetworkConnectionHandler + handshakeSuccesses int32 } func (t *testBackend) OnUnsupportedGlobalRequest(_ uint64, _ string, _ []byte) {} @@ -206,9 +208,10 @@ func (t *testBackend) OnHandshakeFailed(_ metadata.ConnectionMetadata, _ error) func (t *testBackend) OnHandshakeSuccess(meta metadata.ConnectionAuthenticatedMetadata) ( connection sshserver.SSHConnectionHandler, - metadata metadata.ConnectionAuthenticatedMetadata, + metadataOut metadata.ConnectionAuthenticatedMetadata, failureReason error, ) { + atomic.AddInt32(&t.handshakeSuccesses, 1) return t, meta, nil } diff --git a/internal/sshserver/serverImpl.go b/internal/sshserver/serverImpl.go index 17a169d1..74caa31a 100644 --- a/internal/sshserver/serverImpl.go +++ b/internal/sshserver/serverImpl.go @@ -207,11 +207,19 @@ func (s *serverImpl) wrapAndLogAuthFailure( Label("username", conn.Username). Label("method", strings.ToLower(authMethod)) logger.Info(err) - } else { + return err + } + var denial *auth.UserFacingDenial + if errors.As(err, &denial) { + // The error carries a user-facing message supplied by the + // authentication or authorization webhook. Preserve that message instead + // of overwriting it with the generic failure text and deliver it to the + // client as a user authentication banner (RFC 4252, section 5.4) + // immediately before the authentication failure. err = messageCodes.WrapUser( err, messageCodes.ESSHAuthFailed, - "Authentication failed.", + denial.Message, "%s authentication for user %s failed.", authMethod, conn.Username, @@ -220,7 +228,23 @@ func (s *serverImpl) wrapAndLogAuthFailure( Label("method", strings.ToLower(authMethod)). Label("reason", err.Error()) logger.Info(err) + return &ssh.BannerError{ + Err: err, + Message: denial.Message, + } } + err = messageCodes.WrapUser( + err, + messageCodes.ESSHAuthFailed, + "Authentication failed.", + "%s authentication for user %s failed.", + authMethod, + conn.Username, + ). + Label("username", conn.Username). + Label("method", strings.ToLower(authMethod)). + Label("reason", err.Error()) + logger.Info(err) return err } From 74851a8b40cfc0bda431a7fcafa124e45a9e0c25 Mon Sep 17 00:00:00 2001 From: "Matt, Matthias" Date: Fri, 9 Oct 2026 18:37:33 +0200 Subject: [PATCH 2/4] test(auth): cover webhook transport errors not becoming user-facing denials Adds a connection-refused fixture test asserting that a webhook outage keeps its fail-closed retry/timeout behavior and never surfaces as a user-facing denial, complementing the non-200 coverage. Part of #1, refs #3. Signed-off-by: Matt, Matthias --- internal/auth/webhook_denial_test.go | 40 ++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/internal/auth/webhook_denial_test.go b/internal/auth/webhook_denial_test.go index c356ab7d..cdeb02d9 100644 --- a/internal/auth/webhook_denial_test.go +++ b/internal/auth/webhook_denial_test.go @@ -14,6 +14,7 @@ import ( "go.containerssh.io/containerssh/internal/auth" "go.containerssh.io/containerssh/internal/geoip/dummy" "go.containerssh.io/containerssh/internal/metrics" + "go.containerssh.io/containerssh/internal/test" "go.containerssh.io/containerssh/log" "go.containerssh.io/containerssh/metadata" ) @@ -202,3 +203,42 @@ func TestWebhookNon200DoesNotBecomeUserFacingDenial(t *testing.T) { "an infrastructure failure must not become a user-facing denial", ) } + +// TestWebhookTransportErrorDoesNotBecomeUserFacingDenial verifies that a +// webhook outage (connection refused) keeps its fail-closed retry/timeout +// behavior and never turns into a user-facing denial message. +func TestWebhookTransportErrorDoesNotBecomeUserFacingDenial(t *testing.T) { + // Point the client at a closed port so the connection is refused. + closedPort := test.GetNextPort(t, "closed auth server") + + client, err := auth.NewWebhookClient( + auth.AuthenticationTypeAll, + config.AuthWebhookClientConfig{ + HTTPClientConfiguration: config.HTTPClientConfiguration{ + URL: fmt.Sprintf("http://127.0.0.1:%d", closedPort), + Timeout: 2 * time.Second, + }, + AuthTimeout: 1 * time.Second, + }, + log.NewTestLogger(t), + metrics.New(dummy.New()), + ) + assert.NoError(t, err) + + authContext := client.PubKey( + metadata.NewTestAuthenticatingMetadata("foo"), + auth2.PublicKey{PublicKey: "ssh-rsa asdf"}, + ) + + assert.False(t, authContext.Success()) + assert.Error( + t, + authContext.Error(), + "a webhook outage must remain a fail-closed backend error", + ) + assert.False( + t, + auth2.IsUserFacingDenial(authContext.Error()), + "a webhook outage must not become a user-facing denial", + ) +} From f9a630f413986a39ee87499f113ff54b09e2a6ce Mon Sep 17 00:00:00 2001 From: "Matt, Matthias" Date: Fri, 16 Oct 2026 14:10:05 +0200 Subject: [PATCH 3/4] docs(auth): add package comment required by staticcheck --- auth/denial.go | 1 + 1 file changed, 1 insertion(+) diff --git a/auth/denial.go b/auth/denial.go index 7ee09ea2..68433059 100644 --- a/auth/denial.go +++ b/auth/denial.go @@ -1,3 +1,4 @@ +// Package auth defines authentication and authorization webhook protocols and errors. package auth import "errors" From 44e60a8fa5546769e8819b5363103b76955c707f Mon Sep 17 00:00:00 2001 From: "Matt, Matthias" Date: Fri, 16 Oct 2026 15:40:17 +0200 Subject: [PATCH 4/4] test(auth): cover authorization denials reaching the client as banners MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Extends the regression coverage of the readable-denial message transport to the authorization path, closing the motivating scenario end-to-end: successful public-key authentication followed by an authorization denial with an explicit message (e.g. "unknown pod template 'unknown' — use a configured template username") makes the SSH client see exactly that text as a user authentication banner (RFC 4252, section 5.4) immediately before the connection fails. The message transport introduced for the tracer bullet already routes the authorization webhook response through the post-authentication policy check that runs during authentication; this change locks that behavior in with controlled webhook fixtures at the established seams: - End-to-end at the auth integration seam, the authorization denial with a message is observed as the exact pre-failure banner by a real SSH client, completes within five seconds (it never enters config-webhook retry handling), and never initializes a backend session. Message-less authorization denials keep the previous behavior: plain failure, no banner. - End-to-end with the actual OpenSSH client in noninteractive use, exercising the representative unknown-template denial string, with the OpenSSH test setup factored into shared helpers. - At the webhook client seam, message-less authorization denials stay plain failures and a message on a successful authorization response is not a denial, mirroring the public-key path coverage. Part of #1, closes #4. Signed-off-by: Matt, Matthias --- internal/auth/webhook_denial_test.go | 38 ++++++ internal/authintegration/banner_test.go | 174 +++++++++++++++++++++--- 2 files changed, 192 insertions(+), 20 deletions(-) diff --git a/internal/auth/webhook_denial_test.go b/internal/auth/webhook_denial_test.go index cdeb02d9..1b39153e 100644 --- a/internal/auth/webhook_denial_test.go +++ b/internal/auth/webhook_denial_test.go @@ -163,6 +163,44 @@ func TestAuthzDenialWithMessageIsTerminalAndCarriesMessage(t *testing.T) { } } +// TestAuthzDenialWithoutMessageKeepsExistingBehavior verifies that message-less +// authorization denials behave exactly as before: a plain failure without an +// error. +func TestAuthzDenialWithoutMessageKeepsExistingBehavior(t *testing.T) { + server := startAuthFixture(t, `{"success":false}`) + client := newFixtureWebhookClient(t, server.URL) + + authzContext := client.Authorize( + metadata.NewTestAuthenticatingMetadata("foo").Authenticated("foo"), + ) + + assert.False(t, authzContext.Success()) + assert.NoError( + t, + authzContext.Error(), + "a message-less denial must not acquire an error", + ) + assert.False( + t, + auth2.IsUserFacingDenial(authzContext.Error()), + "a message-less denial must not carry a user-facing message", + ) +} + +// TestAuthzSuccessWithMessageSucceeds verifies that a message on a successful +// authorization response does not become a denial. +func TestAuthzSuccessWithMessageSucceeds(t *testing.T) { + server := startAuthFixture(t, `{"success":true,"message":"welcome"}`) + client := newFixtureWebhookClient(t, server.URL) + + authzContext := client.Authorize( + metadata.NewTestAuthenticatingMetadata("foo").Authenticated("foo"), + ) + + assert.True(t, authzContext.Success()) + assert.NoError(t, authzContext.Error()) +} + // TestWebhookNon200DoesNotBecomeUserFacingDenial verifies that infrastructure // failures (non-200 responses) keep their fail-closed behavior and never turn // into user-facing denial messages. diff --git a/internal/authintegration/banner_test.go b/internal/authintegration/banner_test.go index 30882000..6581e34e 100644 --- a/internal/authintegration/banner_test.go +++ b/internal/authintegration/banner_test.go @@ -226,6 +226,74 @@ func TestPubKeyDenialWithMessageReachesClientAsBanner(t *testing.T) { ) } +// TestAuthzDenialWithMessageReachesClientAsBanner is the motivating scenario +// for readable denials: the user authenticates successfully by public key, the +// authorization check denies with an explicit message, and the SSH client sees +// exactly that text as a banner immediately before the connection fails. +func TestAuthzDenialWithMessageReachesClientAsBanner(t *testing.T) { + denialMessage := "unknown pod template 'unknown' — use a configured template username" + fixture := startAuthWebhookFixture(t) + fixture.setResponse("pubkey", `{"success":true}`) + fixture.setResponse( + "authz", + fmt.Sprintf(`{"success":false,"message":%q}`, denialMessage), + ) + + start := time.Now() + sshServerConfig, backend := startFixtureSSHServer(t, fixture) + result := connectWithPublicKey(t, sshServerConfig, "unknown") + elapsed := time.Since(start) + + assert.Error( + t, + result.dialError, + "a message-bearing authorization denial must still fail authentication", + ) + assert.Equal( + t, + []string{denialMessage}, + result.banners, + "the client must see the exact authorization denial message as a pre-failure banner", + ) + assert.Less( + t, + elapsed, + 5*time.Second, + "a healthy permanent authorization denial must complete promptly", + ) + assert.Equal( + t, + int32(0), + atomic.LoadInt32(&backend.handshakeSuccesses), + "an authorization denial must not initialize a backend session", + ) +} + +// TestAuthzDenialWithoutMessageHasNoBanner verifies that message-less +// authorization denials behave exactly as before: authentication fails without +// a banner. +func TestAuthzDenialWithoutMessageHasNoBanner(t *testing.T) { + fixture := startAuthWebhookFixture(t) + fixture.setResponse("pubkey", `{"success":true}`) + fixture.setResponse("authz", `{"success":false}`) + + sshServerConfig, backend := startFixtureSSHServer(t, fixture) + result := connectWithPublicKey(t, sshServerConfig, "foo") + + assert.Error(t, result.dialError) + assert.Empty( + t, + result.banners, + "a message-less authorization denial must not acquire a banner", + ) + assert.Equal( + t, + int32(0), + atomic.LoadInt32(&backend.handshakeSuccesses), + "an authorization denial must not initialize a backend session", + ) +} + // TestPubKeyDenialWithoutMessageHasNoBanner verifies that message-less denials // behave exactly as before: authentication fails without a banner. func TestPubKeyDenialWithoutMessageHasNoBanner(t *testing.T) { @@ -305,13 +373,11 @@ func TestWebhookOutageDoesNotProduceBanner(t *testing.T) { ) } -// TestOpenSSHClientSeesDenialBanner verifies, with the actual OpenSSH client in -// noninteractive use, that a message-bearing denial is printed as the banner and -// authentication still fails. The test is skipped when no OpenSSH client is -// available. -func TestOpenSSHClientSeesDenialBanner(t *testing.T) { - sshBinary, err := exec.LookPath("ssh") - if err != nil { +// generateOpenSSHTestKey generates a fresh ed25519 key for the OpenSSH client +// tests and returns its path. The test is skipped when no OpenSSH client +// tooling is available. +func generateOpenSSHTestKey(t *testing.T) string { + if _, err := exec.LookPath("ssh"); err != nil { t.Skip("OpenSSH client not available, skipping OpenSSH banner verification") } sshKeyGen, err := exec.LookPath("ssh-keygen") @@ -323,23 +389,24 @@ func TestOpenSSHClientSeesDenialBanner(t *testing.T) { keyFile := tempDir + "/testkey" if out, err := exec.Command(sshKeyGen, "-q", "-t", "ed25519", "-N", "", "-f", keyFile).CombinedOutput(); err != nil { assert.FailNow(t, "failed to generate test key", string(out)) - return + return "" } + return keyFile +} - denialMessage := "unknown pod template 'unknown' — use a configured template username" - fixture := startAuthWebhookFixture(t) - fixture.setResponse( - "pubkey", - fmt.Sprintf(`{"success":false,"message":%q}`, denialMessage), - ) - fixture.setResponse("authz", `{"success":true}`) - - sshServerConfig, backend := startFixtureSSHServer(t, fixture) +// runOpenSSHClient connects with the actual OpenSSH client in noninteractive +// use and returns the captured stderr and the command error. +func runOpenSSHClient( + t *testing.T, + sshServerConfig config.SSHConfig, + username string, + keyFile string, +) (string, error) { listens := strings.Split(sshServerConfig.Listen, ":") ctx, cancel := context.WithTimeout(context.Background(), 15*time.Second) defer cancel() - cmd := exec.CommandContext(ctx, sshBinary, + cmd := exec.CommandContext(ctx, "ssh", "-o", "BatchMode=yes", "-o", "ConnectTimeout=10", "-o", "StrictHostKeyChecking=no", @@ -350,11 +417,31 @@ func TestOpenSSHClientSeesDenialBanner(t *testing.T) { "-o", "KbdInteractiveAuthentication=no", "-i", keyFile, "-p", listens[len(listens)-1], - "unknown@127.0.0.1", "true", + username+"@127.0.0.1", "true", ) var stderr bytes.Buffer cmd.Stderr = &stderr runErr := cmd.Run() + return stderr.String(), runErr +} + +// TestOpenSSHClientSeesDenialBanner verifies, with the actual OpenSSH client in +// noninteractive use, that a message-bearing denial is printed as the banner and +// authentication still fails. The test is skipped when no OpenSSH client is +// available. +func TestOpenSSHClientSeesDenialBanner(t *testing.T) { + keyFile := generateOpenSSHTestKey(t) + + denialMessage := "unknown pod template 'unknown' — use a configured template username" + fixture := startAuthWebhookFixture(t) + fixture.setResponse( + "pubkey", + fmt.Sprintf(`{"success":false,"message":%q}`, denialMessage), + ) + fixture.setResponse("authz", `{"success":true}`) + + sshServerConfig, backend := startFixtureSSHServer(t, fixture) + stderr, runErr := runOpenSSHClient(t, sshServerConfig, "unknown", keyFile) assert.Error( t, @@ -363,7 +450,7 @@ func TestOpenSSHClientSeesDenialBanner(t *testing.T) { ) assert.Contains( t, - stderr.String(), + stderr, denialMessage, "the OpenSSH client must print the denial message as the banner", ) @@ -374,3 +461,50 @@ func TestOpenSSHClientSeesDenialBanner(t *testing.T) { "a denial must not initialize a backend session", ) } + +// TestOpenSSHClientSeesAuthzDenialBanner verifies, with the actual OpenSSH +// client in noninteractive use, the motivating scenario for readable denials: +// the public-key authentication succeeds, the authorization check denies with +// an explicit message, and the OpenSSH client prints exactly that text as the +// banner while the connection fails. The test is skipped when no OpenSSH +// client is available. +func TestOpenSSHClientSeesAuthzDenialBanner(t *testing.T) { + keyFile := generateOpenSSHTestKey(t) + + denialMessage := "unknown pod template 'unknown' — use a configured template username" + fixture := startAuthWebhookFixture(t) + fixture.setResponse("pubkey", `{"success":true}`) + fixture.setResponse( + "authz", + fmt.Sprintf(`{"success":false,"message":%q}`, denialMessage), + ) + + start := time.Now() + sshServerConfig, backend := startFixtureSSHServer(t, fixture) + stderr, runErr := runOpenSSHClient(t, sshServerConfig, "unknown", keyFile) + elapsed := time.Since(start) + + assert.Error( + t, + runErr, + "the OpenSSH client must still fail the connection on an authorization denial", + ) + assert.Contains( + t, + stderr, + denialMessage, + "the OpenSSH client must print the authorization denial message as the banner", + ) + assert.Less( + t, + elapsed, + 15*time.Second, + "a healthy permanent authorization denial must complete promptly and must not enter retry handling", + ) + assert.Equal( + t, + int32(0), + atomic.LoadInt32(&backend.handshakeSuccesses), + "an authorization denial must not initialize a backend session", + ) +}