Make session key authorization atomic, unblock the encoder on teardown, drop agent privileges unconditionally

shared/sessionauth/auth.go

@@ -251,12 +251,50 @@ func (a *Authorizer) LookupSessionDisplayName(pubKey []byte) string {
 	return name
 }
 
+// AuthorizeSessionKey resolves a Noise-verified static public key and authorizes
+// the identity it names for osUsername, under a single read lock.
+//
+// The two halves must not be separated: Update swaps sessionPubKeys and
+// authorizedUsers together, so a caller that looks the key up and then
+// authorizes it can have the key revoked in between and still be admitted on
+// the hash it already holds. Revocation has to close that window.
+func (a *Authorizer) AuthorizeSessionKey(pubKey []byte, osUsername string) (sshuserhash.UserIDHash, string, error) {
+	var zero sshuserhash.UserIDHash
+	if len(pubKey) != sessionPubKeyLen {
+		return zero, "", fmt.Errorf("session pubkey wrong length: %d", len(pubKey))
+	}
+	var key [sessionPubKeyLen]byte
+	copy(key[:], pubKey)
+
+	a.mu.RLock()
+	defer a.mu.RUnlock()
+
+	hash, ok := a.sessionPubKeys[key]
+	if !ok {
+		return zero, "", ErrSessionKeyNotKnown
+	}
+	osUser, err := a.authorizeOSUserBySessionKeyLocked(hash, osUsername)
+	if err != nil {
+		return zero, "", err
+	}
+	return hash, osUser, nil
+}
+
 // AuthorizeOSUserBySessionKey resolves the OS-user mapping for a session
 // key. Mirrors Authorize but skips the JWT-hash step since the key has
 // already been verified and the user identity hash is in hand.
+//
+// Prefer AuthorizeSessionKey where the key is also being looked up: splitting
+// the two lets a revocation land between them.
 func (a *Authorizer) AuthorizeOSUserBySessionKey(userIDHash sshuserhash.UserIDHash, osUsername string) (string, error) {
 	a.mu.RLock()
 	defer a.mu.RUnlock()
+	return a.authorizeOSUserBySessionKeyLocked(userIDHash, osUsername)
+}
+
+// authorizeOSUserBySessionKeyLocked is AuthorizeOSUserBySessionKey with a.mu
+// already held for reading.
+func (a *Authorizer) authorizeOSUserBySessionKeyLocked(userIDHash sshuserhash.UserIDHash, osUsername string) (string, error) {
 	userIndex, found := a.findUserIndex(userIDHash)
 	if !found {
 		return "", fmt.Errorf("session user (hash: %s) not in authorized list for OS user %q: %w", userIDHash, osUsername, ErrUserNotAuthorized)

shared/sessionauth/auth_test.go

@@ -711,3 +711,49 @@ func bytesRepeat(b byte, n int) []byte {
 	}
 	return out
 }
+
+// AuthorizeSessionKey resolves the key and authorizes the identity it names
+// under one lock. Splitting those steps let an Update that revoked the key land
+// in between, after which the caller still held a hash that authorized fine.
+func TestAuthorizer_AuthorizeSessionKey_RevocationIsAtomic(t *testing.T) {
+	pub := bytesRepeat(0x55, sessionPubKeyLen)
+	userHash, err := sshauth.HashUserID("alice")
+	require.NoError(t, err)
+
+	granted := &Config{
+		AuthorizedUsers: []sshauth.UserIDHash{userHash},
+		MachineUsers:    map[string][]uint32{Wildcard: {0}},
+		SessionPubKeys:  []SessionPubKey{{PubKey: pub, UserIDHash: userHash}},
+	}
+	// The user stays authorized; only the session key is withdrawn. That is the
+	// shape that used to slip through, because the second step never looked at
+	// the key again.
+	revoked := &Config{
+		AuthorizedUsers: []sshauth.UserIDHash{userHash},
+		MachineUsers:    map[string][]uint32{Wildcard: {0}},
+	}
+
+	a := NewAuthorizer()
+	a.Update(granted)
+	gotHash, _, err := a.AuthorizeSessionKey(pub, "alice")
+	require.NoError(t, err)
+	assert.Equal(t, userHash, gotHash)
+
+	a.Update(revoked)
+	_, _, err = a.AuthorizeSessionKey(pub, "alice")
+	require.ErrorIs(t, err, ErrSessionKeyNotKnown, "a revoked key must not authorize, even for a still-authorized user")
+}
+
+// A key that resolves but whose user is not authorized is refused by the second
+// half of the same call.
+func TestAuthorizer_AuthorizeSessionKey_UnauthorizedUser(t *testing.T) {
+	pub := bytesRepeat(0x66, sessionPubKeyLen)
+	userHash, err := sshauth.HashUserID("alice")
+	require.NoError(t, err)
+
+	a := NewAuthorizer()
+	a.Update(&Config{SessionPubKeys: []SessionPubKey{{PubKey: pub, UserIDHash: userHash}}})
+
+	_, _, err = a.AuthorizeSessionKey(pub, "alice")
+	require.ErrorIs(t, err, ErrUserNotAuthorized)
+}