ssh-agent: record failed session-bind attempts

Record failed attempts to session-bind a connection and refuse signing
operations on that connection henceforth.

Prevents a future situation where we add a new hostkey type that is not
recognised by an older ssh-agent, that consequently causes session-bind
to fail (this situation is only likely to arise when people mix ssh(1)
and ssh-agent(1) of different versions on the same host). Previously,
after such a failure the agent socket would be considered unbound and
not subject to restriction.

Spotted by Jann Horn

diff --git a/usr.bin/ssh/ssh-agent.c b/usr.bin/ssh/ssh-agent.c
index 8012ec2..cd77127 100644 (file)
--- a/usr.bin/ssh/ssh-agent.c
+++ b/usr.bin/ssh/ssh-agent.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssh-agent.c,v 1.302 2023/12/18 14:46:56 djm Exp $ */
+/* $OpenBSD: ssh-agent.c,v 1.303 2023/12/18 14:48:08 djm Exp $ */
 /*
  * Author: Tatu Ylonen <ylo@cs.hut.fi>
  * Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland
@@ -117,6 +117,7 @@ typedef struct socket_entry {
        struct sshbuf *request;
        size_t nsession_ids;
        struct hostkey_sid *session_ids;
+       int session_bind_attempted;
 } SocketEntry;
 
 u_int sockets_alloc = 0;
@@ -464,6 +465,10 @@ identity_permitted(Identity *id, SocketEntry *e, char *user,
            e->nsession_ids, id->ndest_constraints);
        if (id->ndest_constraints == 0)
                return 0; /* unconstrained */
+       if (e->session_bind_attempted && e->nsession_ids == 0) {
+               error_f("previous session bind failed on socket");
+               return -1;
+       }
        if (e->nsession_ids == 0)
                return 0; /* local use */
        /*
@@ -543,6 +548,12 @@ identity_permitted(Identity *id, SocketEntry *e, char *user,
        return 0;
 }
 
+static int
+socket_is_remote(SocketEntry *e)
+{
+       return e->session_bind_attempted || (e->nsession_ids != 0);
+}
+
 /* return matching private key for given public key */
 static Identity *
 lookup_identity(struct sshkey *key)
@@ -1354,7 +1365,7 @@ process_add_identity(SocketEntry *e)
                if (strcasecmp(sk_provider, "internal") == 0) {
                        debug_f("internal provider");
                } else {
-                       if (e->nsession_ids != 0 && !remote_add_provider) {
+                       if (socket_is_remote(e) && !remote_add_provider) {
                                verbose("failed add of SK provider \"%.100s\": "
                                    "remote addition of providers is disabled",
                                    sk_provider);
@@ -1552,7 +1563,7 @@ process_add_smartcard_key(SocketEntry *e)
                goto send;
        }
        dump_dest_constraints(__func__, dest_constraints, ndest_constraints);
-       if (e->nsession_ids != 0 && !remote_add_provider) {
+       if (socket_is_remote(e) && !remote_add_provider) {
                verbose("failed PKCS#11 add of \"%.100s\": remote addition of "
                    "providers is disabled", provider);
                goto send;
@@ -1667,6 +1678,7 @@ process_ext_session_bind(SocketEntry *e)
        u_char fwd = 0;
 
        debug2_f("entering");
+       e->session_bind_attempted = 1;
        if ((r = sshkey_froms(e->request, &key)) != 0 ||
            (r = sshbuf_froms(e->request, &sid)) != 0 ||
            (r = sshbuf_froms(e->request, &sig)) != 0 ||