Soft Serve has Critical Authentication Bypass

@@ -2,6 +2,7 @@ package ssh
 
 import (
 	"fmt"
+	"strconv"
 	"time"
 
 	"charm.land/log/v2"
@@ -30,23 +31,43 @@ func AuthenticationMiddleware(sh ssh.Handler) ssh.Handler {
 		// validate the authentication. We need to verify that the _last_ key
 		// that was approved is the one that's being used.
 
+		ctx := s.Context()
+		be := backend.FromContext(ctx)
+
+		var pkFp string
+		perms := s.Permissions().Permissions
 		pk := s.PublicKey()
 		if pk != nil {
 			// There is no public key stored in the context, public-key auth
 			// was never requested, skip
-			perms := s.Permissions().Permissions
 			if perms == nil {
 				wish.Fatalln(s, ErrPermissionDenied)
 				return
 			}
 
-			// Check if the key is the same as the one we have in context
-			fp := perms.Extensions["pubkey-fp"]
-			if fp == "" || fp != gossh.FingerprintSHA256(pk) {
-				wish.Fatalln(s, ErrPermissionDenied)
-				return
-			}
+			pkFp = gossh.FingerprintSHA256(pk)
+		}
+
+		// Check if the key is the same as the one we have in context
+		fp := perms.Extensions["pubkey-fp"]
+		if fp != "" && fp != pkFp {
+			wish.Fatalln(s, ErrPermissionDenied)
+			return
+		}
+
+		ac := be.AllowKeyless(ctx)
+		publicKeyCounter.WithLabelValues(strconv.FormatBool(ac || pk != nil)).Inc()
+		if !ac && pk == nil {
+			wish.Fatalln(s, ErrPermissionDenied)
+			return
+		}
+
+		// Set the auth'd user, or anon, in the context
+		var user proto.User
+		if pk != nil {
+			user, _ = be.UserByPublicKey(ctx, pk)
 		}
+		ctx.SetValue(proto.ContextKeyUser, user)
 
 		sh(s)
 	}

@@ -0,0 +1,180 @@
+package ssh
+
+import (
+	"context"
+	"net"
+	"testing"
+
+	"github.com/charmbracelet/keygen"
+	"github.com/charmbracelet/soft-serve/pkg/backend"
+	"github.com/charmbracelet/soft-serve/pkg/config"
+	"github.com/charmbracelet/soft-serve/pkg/db"
+	"github.com/charmbracelet/soft-serve/pkg/db/migrate"
+	"github.com/charmbracelet/soft-serve/pkg/proto"
+	"github.com/charmbracelet/soft-serve/pkg/store"
+	"github.com/charmbracelet/soft-serve/pkg/store/database"
+	"github.com/charmbracelet/ssh"
+	"github.com/matryer/is"
+	gossh "golang.org/x/crypto/ssh"
+	_ "modernc.org/sqlite"
+)
+
+// TestAuthenticationBypass tests for CVE-TBD: Authentication Bypass Vulnerability
+//
+// VULNERABILITY:
+// A critical authentication bypass allows an attacker to impersonate any user
+// (including Admin) by "offering" the victim's public key during the SSH handshake
+// before authenticating with their own valid key. This occurs because the user
+// identity is stored in the session context during the "offer" phase in
+// PublicKeyHandler and is not properly cleared/validated in AuthenticationMiddleware.
+//
+// This test verifies that:
+// 1. User context is properly set based on the AUTHENTICATED key, not offered keys
+// 2. User context from failed authentication attempts is not preserved
+// 3. Non-admin users cannot gain admin privileges through this attack
+func TestAuthenticationBypass(t *testing.T) {
+	is := is.New(t)
+	ctx := context.Background()
+
+	// Setup temporary database
+	dp := t.TempDir()
+	cfg := config.DefaultConfig()
+	cfg.DataPath = dp
+	cfg.DB.Driver = "sqlite"
+	cfg.DB.DataSource = dp + "/test.db"
+
+	ctx = config.WithContext(ctx, cfg)
+	dbx, err := db.Open(ctx, cfg.DB.Driver, cfg.DB.DataSource)
+	is.NoErr(err)
+	defer dbx.Close()
+
+	is.NoErr(migrate.Migrate(ctx, dbx))
+	dbstore := database.New(ctx, dbx)
+	ctx = store.WithContext(ctx, dbstore)
+	be := backend.New(ctx, cfg, dbx, dbstore)
+	ctx = backend.WithContext(ctx, be)
+
+	// Generate keys for admin and attacker
+	adminKeyPath := dp + "/admin_key"
+	adminPair, err := keygen.New(adminKeyPath, keygen.WithKeyType(keygen.Ed25519), keygen.WithWrite())
+	is.NoErr(err)
+
+	attackerKeyPath := dp + "/attacker_key"
+	attackerPair, err := keygen.New(attackerKeyPath, keygen.WithKeyType(keygen.Ed25519), keygen.WithWrite())
+	is.NoErr(err)
+
+	// Parse public keys
+	adminPubKey, _, _, _, err := gossh.ParseAuthorizedKey([]byte(adminPair.AuthorizedKey()))
+	is.NoErr(err)
+
+	attackerPubKey, _, _, _, err := gossh.ParseAuthorizedKey([]byte(attackerPair.AuthorizedKey()))
+	is.NoErr(err)
+
+	// Create admin user
+	adminUser, err := be.CreateUser(ctx, "testadmin", proto.UserOptions{
+		Admin:      true,
+		PublicKeys: []gossh.PublicKey{adminPubKey},
+	})
+	is.NoErr(err)
+	is.True(adminUser != nil)
+
+	// Create attacker (non-admin) user
+	attackerUser, err := be.CreateUser(ctx, "testattacker", proto.UserOptions{
+		Admin:      false,
+		PublicKeys: []gossh.PublicKey{attackerPubKey},
+	})
+	is.NoErr(err)
+	is.True(attackerUser != nil)
+	is.True(!attackerUser.IsAdmin()) // Verify attacker is NOT admin
+
+	// Test: Verify that looking up user by key gives correct user
+	t.Run("user_lookup_by_key", func(t *testing.T) {
+		is := is.New(t)
+
+		// Looking up admin key should return admin user
+		user, err := be.UserByPublicKey(ctx, adminPubKey)
+		is.NoErr(err)
+		is.Equal(user.Username(), "testadmin")
+		is.True(user.IsAdmin())
+
+		// Looking up attacker key should return attacker user
+		user, err = be.UserByPublicKey(ctx, attackerPubKey)
+		is.NoErr(err)
+		is.Equal(user.Username(), "testattacker")
+		is.True(!user.IsAdmin())
+	})
+
+	// Test: Simulate the authentication bypass vulnerability
+	// This test documents the EXPECTED behavior to prevent regression
+	t.Run("authentication_bypass_simulation", func(t *testing.T) {
+		is := is.New(t)
+
+		// Create a mock context
+		mockCtx := &mockSSHContext{
+			Context:     ctx,
+			values:      make(map[any]any),
+			permissions: &ssh.Permissions{Permissions: &gossh.Permissions{Extensions: make(map[string]string)}},
+		}
+
+		// ATTACK SIMULATION:
+		// Step 1: SSH client offers admin's public key
+		// PublicKeyHandler is called and sets admin user in context
+		mockCtx.SetValue(proto.ContextKeyUser, adminUser)
+		mockCtx.permissions.Extensions["pubkey-fp"] = gossh.FingerprintSHA256(adminPubKey)
+
+		// Step 2: Signature verification FAILS (attacker doesn't have admin's private key)
+		// SSH protocol continues to next key...
+
+		// Step 3: SSH client offers attacker's key (which SUCCEEDS)
+		// PublicKeyHandler is called again, fingerprint is updated
+		mockCtx.permissions.Extensions["pubkey-fp"] = gossh.FingerprintSHA256(attackerPubKey)
+		// BUG: Admin user is STILL in context from step 1!
+
+		// Step 4: AuthenticationMiddleware should re-lookup user based on authenticated key
+		// The middleware MUST NOT trust the user already in context
+		authenticatedUser, err := be.UserByPublicKey(mockCtx, attackerPubKey)
+		is.NoErr(err)
+
+		// EXPECTED: User should be "attacker", NOT "admin"
+		is.Equal(authenticatedUser.Username(), "testattacker")
+		is.True(!authenticatedUser.IsAdmin())
+
+		// If the vulnerability exists, the context would still have admin user
+		contextUser := proto.UserFromContext(mockCtx)
+		if contextUser != nil && contextUser.Username() == "testadmin" {
+			t.Logf("WARNING: Context still contains admin user! This indicates the vulnerability exists.")
+			t.Logf("The authenticated key is attacker's, but context has admin user.")
+		}
+	})
+}
+
+// mockSSHContext implements ssh.Context for testing
+type mockSSHContext struct {
+	context.Context
+	values      map[any]any
+	permissions *ssh.Permissions
+}
+
+func (m *mockSSHContext) SetValue(key, value any) {
+	m.values[key] = value
+}
+
+func (m *mockSSHContext) Value(key any) any {
+	if v, ok := m.values[key]; ok {
+		return v
+	}
+	return m.Context.Value(key)
+}
+
+func (m *mockSSHContext) Permissions() *ssh.Permissions {
+	return m.permissions
+}
+
+func (m *mockSSHContext) User() string          { return "" }
+func (m *mockSSHContext) RemoteAddr() net.Addr  { return &net.TCPAddr{} }
+func (m *mockSSHContext) LocalAddr() net.Addr   { return &net.TCPAddr{} }
+func (m *mockSSHContext) ServerVersion() string { return "" }
+func (m *mockSSHContext) ClientVersion() string { return "" }
+func (m *mockSSHContext) SessionID() string     { return "" }
+func (m *mockSSHContext) Lock()                 {}
+func (m *mockSSHContext) Unlock()               {}

@@ -16,7 +16,6 @@ import (
 	"github.com/charmbracelet/soft-serve/pkg/backend"
 	"github.com/charmbracelet/soft-serve/pkg/config"
 	"github.com/charmbracelet/soft-serve/pkg/db"
-	"github.com/charmbracelet/soft-serve/pkg/proto"
 	"github.com/charmbracelet/soft-serve/pkg/store"
 	"github.com/charmbracelet/ssh"
 	"github.com/prometheus/client_golang/prometheus"
@@ -74,13 +73,14 @@ func NewSSHServer(ctx context.Context) (*SSHServer, error) {
 			CommandMiddleware,
 			// Logging middleware.
 			LoggingMiddleware,
-			// Context middleware.
-			ContextMiddleware(cfg, dbx, datastore, be, logger),
 			// Authentication middleware.
 			// gossh.PublicKeyHandler doesn't guarantee that the public key
 			// is in fact the one used for authentication, so we need to
 			// check it again here.
 			AuthenticationMiddleware,
+			// Context middleware.
+			// This must come first to set up the context.
+			ContextMiddleware(cfg, dbx, datastore, be, logger),
 		),
 	}
 
@@ -166,14 +166,6 @@ func (s *SSHServer) PublicKeyHandler(ctx ssh.Context, pk ssh.PublicKey) (allowed
 	}
 
 	allowed = true
-	defer func(allowed *bool) {
-		publicKeyCounter.WithLabelValues(strconv.FormatBool(*allowed)).Inc()
-	}(&allowed)
-
-	user, _ := s.be.UserByPublicKey(ctx, pk)
-	if user != nil {
-		ctx.SetValue(proto.ContextKeyUser, user)
-	}
 
 	// XXX: store the first "approved" public-key fingerprint in the
 	// permissions block to use for authentication later.
@@ -194,10 +186,10 @@ func (s *SSHServer) KeyboardInteractiveHandler(ctx ssh.Context, _ gossh.Keyboard
 	keyboardInteractiveCounter.WithLabelValues(strconv.FormatBool(ac)).Inc()
 
 	// If we're allowing keyless access, reset the public key fingerprint
-	if ac {
-		initializePermissions(ctx)
-		perms := ctx.Permissions()
+	initializePermissions(ctx)
+	perms := ctx.Permissions()
 
+	if ac {
 		// XXX: reset the public-key fingerprint. This is used to validate the
 		// public key being used to authenticate.
 		perms.Extensions["pubkey-fp"] = ""

@@ -82,6 +82,10 @@ func TestScript(t *testing.T) {
 	admin1Key, admin1 := mkkey("admin1")
 	_, admin2 := mkkey("admin2")
 	user1Key, user1 := mkkey("user1")
+	attackerKey, attacker := mkkey("attacker")
+	attackerSigner := &maliciousSigner{
+		publicKey: admin1.PublicKey(),
+	}
 
 	testscript.Run(t, testscript.Params{
 		Dir:                 "./testdata/",
@@ -90,8 +94,10 @@ func TestScript(t *testing.T) {
 		Cmds: map[string]func(ts *testscript.TestScript, neg bool, args []string){
 			"soft":                   cmdSoft("admin", admin1.Signer()),
 			"usoft":                  cmdSoft("user1", user1.Signer()),
+			"attacksoft":             cmdSoft("attacker", attackerSigner, attacker.Signer()),
 			"git":                    cmdGit(admin1Key),
 			"ugit":                   cmdGit(user1Key),
+			"agit":                   cmdGit(attackerKey),
 			"curl":                   cmdCurl,
 			"mkfile":                 cmdMkfile,
 			"envfile":                cmdEnvfile,
@@ -127,6 +133,7 @@ func TestScript(t *testing.T) {
 			e.Setenv("ADMIN1_AUTHORIZED_KEY", admin1.AuthorizedKey())
 			e.Setenv("ADMIN2_AUTHORIZED_KEY", admin2.AuthorizedKey())
 			e.Setenv("USER1_AUTHORIZED_KEY", user1.AuthorizedKey())
+			e.Setenv("ATTACKER_AUTHORIZED_KEY", attacker.AuthorizedKey())
 			e.Setenv("SSH_KNOWN_HOSTS_FILE", filepath.Join(t.TempDir(), "known_hosts"))
 			e.Setenv("SSH_KNOWN_CONFIG_FILE", filepath.Join(t.TempDir(), "config"))
 
@@ -189,14 +196,14 @@ func TestScript(t *testing.T) {
 	})
 }
 
-func cmdSoft(user string, key ssh.Signer) func(ts *testscript.TestScript, neg bool, args []string) {
+func cmdSoft(user string, keys ...ssh.Signer) func(ts *testscript.TestScript, neg bool, args []string) {
 	return func(ts *testscript.TestScript, neg bool, args []string) {
 		cli, err := ssh.Dial(
 			"tcp",
 			net.JoinHostPort("localhost", ts.Getenv("SSH_PORT")),
 			&ssh.ClientConfig{
 				User:            user,
-				Auth:            []ssh.AuthMethod{ssh.PublicKeys(key)},
+				Auth:            []ssh.AuthMethod{ssh.PublicKeys(keys...)},
 				HostKeyCallback: ssh.InsecureIgnoreHostKey(),
 			},
 		)
@@ -613,3 +620,20 @@ func setupPostgres(t testscript.T, cfg *config.Config) (func(), error) {
 		}
 	}, nil
 }
+
+type maliciousSigner struct {
+	publicKey ssh.PublicKey
+}
+
+var _ ssh.Signer = (*maliciousSigner)(nil)
+
+// PublicKey implements ssh.Signer.
+func (m *maliciousSigner) PublicKey() ssh.PublicKey {
+	return m.publicKey
+}
+
+// Sign implements ssh.Signer.
+func (m *maliciousSigner) Sign(rand io.Reader, data []byte) (*ssh.Signature, error) {
+	// The attacker doesn't know how to sign the data without a private key.
+	return &ssh.Signature{}, nil
+}

@@ -0,0 +1,63 @@
+# vi: set ft=conf
+# Regression test for authentication bypass vulnerability
+#
+# VULNERABILITY DESCRIPTION:
+# A critical authentication bypass allows an attacker to impersonate any user
+# (including Admin) by offering the user's public key but failing to sign with
+# it, then successfully authenticating with their own key.
+#
+# ATTACK SCENARIO:
+# 1. Attacker obtains Admin's public key (publicly available)
+# 2. Attacker configures SSH client to offer TWO keys in sequence:
+#    - First: Admin's public key (attacker has this but not the private key)
+#    - Second: Attacker's own valid key pair
+# 3. During SSH handshake:
+#    - Server sees admin's public key offered
+#    - PublicKeyHandler() is called, looks up admin user, stores in context
+#    - Server requests signature with admin's key
+#    - Attacker can't sign (doesn't have admin's private key), this key fails
+#    - Server tries next key (attacker's key)
+#    - PublicKeyHandler() called again with attacker's key
+#    - Server requests signature with attacker's key
+#    - Attacker signs successfully with their own private key
+# 4. Admin user is still in context from step 3, even though authentication
+#    succeeded with attacker's key!
+# 5. Attacker gains full Admin privileges
+#
+# THIS TEST VERIFIES:
+# - Using "attacksoft" command which offers both admin and attacker keys
+# - Attacker should NOT be able to perform admin user operations
+# - Attacker should NOT gain admin user privileges
+
+[windows] dos2unix notauthorizederr.txt
+
+# start soft serve
+exec soft serve &
+# wait for SSH server to start
+ensureserverrunning SSH_PORT
+
+# Create a private repo as admin that only admin can access
+soft repo create admin-only-repo -p
+
+# TEST 1: Simulate the attack using attacksoft command
+! attacksoft repo create attacker-created-repo
+
+# TEST 2: Verify attacker cannot access admin's private repo
+! attacksoft git-upload-pack admin-only-repo
+cmp stderr notauthorizederr.txt
+
+# TEST 3: Verify admin can still create repos (sanity check)
+soft repo create admin-created-repo
+
+# TEST 4: Verify attacker cannot delete admin's repo
+! attacksoft repo delete admin-only-repo
+
+# TEST 5: Verify attacker cannot change settings
+! attacksoft settings anon-access read-write
+
+# stop the server
+[windows] stopserver
+[windows] ! stderr .
+
+-- notauthorizederr.txt --
+Error: you are not authorized to do this