From 2b3b369c8cf71f9ef5942a5e074e6f86e7ca1e0c Mon Sep 17 00:00:00 2001 From: "djm@openbsd.org" Date: Sun, 19 Dec 2021 22:09:23 +0000 Subject: [PATCH 11/12] upstream: ssh-agent side of binding record session ID/hostkey/forwarding status for each active socket. Attempt to parse data-to-be-signed at signature request time and extract session ID from the blob if it is a pubkey userauth request. ok markus@ OpenBSD-Commit-ID: a80fd41e292b18b67508362129e9fed549abd318 Upstream-Status: Backport [https://github.com/openssh/openssh-portable/commit/4c1e3ce85e183a9d0c955c88589fed18e4d6a058] CVE: CVE-2023-38408 Signed-off-by: Shubham Kulkarni --- authfd.h | 3 + ssh-agent.c | 175 +++++++++++++++++++++++++++++++++++++++++++++++++--- 2 files changed, 170 insertions(+), 8 deletions(-) diff --git a/authfd.h b/authfd.h index c3bf625..9cc9807 100644 --- a/authfd.h +++ b/authfd.h @@ -76,6 +76,9 @@ int ssh_agent_sign(int sock, const struct sshkey *key, #define SSH2_AGENTC_ADD_ID_CONSTRAINED 25 #define SSH_AGENTC_ADD_SMARTCARD_KEY_CONSTRAINED 26 +/* generic extension mechanism */ +#define SSH_AGENTC_EXTENSION 27 + #define SSH_AGENT_CONSTRAIN_LIFETIME 1 #define SSH_AGENT_CONSTRAIN_CONFIRM 2 #define SSH_AGENT_CONSTRAIN_MAXSIGN 3 diff --git a/ssh-agent.c b/ssh-agent.c index 7f1e14b..01c7f2b 100644 --- a/ssh-agent.c +++ b/ssh-agent.c @@ -1,4 +1,4 @@ -/* $OpenBSD: ssh-agent.c,v 1.275 2021/01/29 06:29:46 djm Exp $ */ +/* $OpenBSD: ssh-agent.c,v 1.280 2021/12/19 22:09:23 djm Exp $ */ /* * Author: Tatu Ylonen * Copyright (c) 1995 Tatu Ylonen , Espoo, Finland @@ -98,9 +98,15 @@ #endif /* Maximum accepted message length */ -#define AGENT_MAX_LEN (256*1024) +#define AGENT_MAX_LEN (256*1024) /* Maximum bytes to read from client socket */ -#define AGENT_RBUF_LEN (4096) +#define AGENT_RBUF_LEN (4096) +/* Maximum number of recorded session IDs/hostkeys per connection */ +#define AGENT_MAX_SESSION_IDS 16 +/* Maximum size of session ID */ +#define AGENT_MAX_SID_LEN 128 + +/* XXX store hostkey_sid in a refcounted tree */ typedef enum { AUTH_UNUSED = 0, @@ -108,12 +114,20 @@ typedef enum { AUTH_CONNECTION = 2, } sock_type; +struct hostkey_sid { + struct sshkey *key; + struct sshbuf *sid; + int forwarded; +}; + typedef struct socket_entry { int fd; sock_type type; struct sshbuf *input; struct sshbuf *output; struct sshbuf *request; + size_t nsession_ids; + struct hostkey_sid *session_ids; } SocketEntry; u_int sockets_alloc = 0; @@ -174,10 +188,17 @@ static int restrict_websafe = 1; static void close_socket(SocketEntry *e) { + size_t i; + close(e->fd); sshbuf_free(e->input); sshbuf_free(e->output); sshbuf_free(e->request); + for (i = 0; i < e->nsession_ids; i++) { + sshkey_free(e->session_ids[i].key); + sshbuf_free(e->session_ids[i].sid); + } + free(e->session_ids); memset(e, '\0', sizeof(*e)); e->fd = -1; e->type = AUTH_UNUSED; @@ -423,6 +444,18 @@ check_websafe_message_contents(struct sshkey *key, struct sshbuf *data) return 0; } +static int +buf_equal(const struct sshbuf *a, const struct sshbuf *b) +{ + if (sshbuf_ptr(a) == NULL || sshbuf_ptr(b) == NULL) + return SSH_ERR_INVALID_ARGUMENT; + if (sshbuf_len(a) != sshbuf_len(b)) + return SSH_ERR_INVALID_FORMAT; + if (timingsafe_bcmp(sshbuf_ptr(a), sshbuf_ptr(b), sshbuf_len(a)) != 0) + return SSH_ERR_INVALID_FORMAT; + return 0; +} + /* ssh2 only */ static void process_sign_request2(SocketEntry *e) @@ -431,8 +464,8 @@ process_sign_request2(SocketEntry *e) size_t i, slen = 0; u_int compat = 0, flags; int r, ok = -1; - char *fp = NULL; - struct sshbuf *msg = NULL, *data = NULL; + char *fp = NULL, *user = NULL, *sig_dest = NULL; + struct sshbuf *msg = NULL, *data = NULL, *sid = NULL; struct sshkey *key = NULL; struct identity *id; struct notifier_ctx *notifier = NULL; @@ -452,7 +485,33 @@ process_sign_request2(SocketEntry *e) verbose("%s: %s key not found", __func__, sshkey_type(key)); goto send; } - if (id->confirm && confirm_key(id, NULL) != 0) { + /* + * If session IDs were recorded for this socket, then use them to + * annotate the confirmation messages with the host keys. + */ + if (e->nsession_ids > 0 && + parse_userauth_request(data, key, &user, &sid) == 0) { + /* + * session ID from userauth request should match the final + * ID in the list recorded in the socket, unless the ssh + * client at that point lacks the binding extension (or if + * an attacker is trying to steal use of the agent). + */ + i = e->nsession_ids - 1; + if (buf_equal(sid, e->session_ids[i].sid) == 0) { + if ((fp = sshkey_fingerprint(e->session_ids[i].key, + SSH_FP_HASH_DEFAULT, SSH_FP_DEFAULT)) == NULL) + fatal("%s: fingerprint failed", __func__); + debug3("%s: destination %s %s (slot %zu)", __func__, + sshkey_type(e->session_ids[i].key), fp, i); + xasprintf(&sig_dest, "public key request for " + "target user \"%s\" to %s %s", user, + sshkey_type(e->session_ids[i].key), fp); + free(fp); + fp = NULL; + } + }// + if (id->confirm && confirm_key(id, sig_dest) != 0) { verbose("%s: user refused key", __func__); goto send; } @@ -467,8 +526,10 @@ process_sign_request2(SocketEntry *e) SSH_FP_DEFAULT)) == NULL) fatal("%s: fingerprint failed", __func__); notifier = notify_start(0, - "Confirm user presence for key %s %s", - sshkey_type(id->key), fp); + "Confirm user presence for key %s %s%s%s", + sshkey_type(id->key), fp, + sig_dest == NULL ? "" : "\n", + sig_dest == NULL ? "" : sig_dest); } } if ((r = sshkey_sign(id->key, &signature, &slen, @@ -492,11 +553,14 @@ process_sign_request2(SocketEntry *e) if ((r = sshbuf_put_stringb(e->output, msg)) != 0) fatal("%s: buffer error: %s", __func__, ssh_err(r)); + sshbuf_free(sid); sshbuf_free(data); sshbuf_free(msg); sshkey_free(key); free(fp); free(signature); + free(sig_dest); + free(user); } /* shared */ @@ -925,6 +989,98 @@ send: } #endif /* ENABLE_PKCS11 */ +static int +process_ext_session_bind(SocketEntry *e) +{ + int r, sid_match, key_match; + struct sshkey *key = NULL; + struct sshbuf *sid = NULL, *sig = NULL; + char *fp = NULL; + u_char fwd; + size_t i; + + debug2("%s: entering", __func__); + if ((r = sshkey_froms(e->request, &key)) != 0 || + (r = sshbuf_froms(e->request, &sid)) != 0 || + (r = sshbuf_froms(e->request, &sig)) != 0 || + (r = sshbuf_get_u8(e->request, &fwd)) != 0) { + error("%s: parse: %s", __func__, ssh_err(r)); + goto out; + } + if ((fp = sshkey_fingerprint(key, SSH_FP_HASH_DEFAULT, + SSH_FP_DEFAULT)) == NULL) + fatal("%s: fingerprint failed", __func__); + /* check signature with hostkey on session ID */ + if ((r = sshkey_verify(key, sshbuf_ptr(sig), sshbuf_len(sig), + sshbuf_ptr(sid), sshbuf_len(sid), NULL, 0, NULL)) != 0) { + error("%s: sshkey_verify for %s %s: %s", __func__, sshkey_type(key), fp, ssh_err(r)); + goto out; + } + /* check whether sid/key already recorded */ + for (i = 0; i < e->nsession_ids; i++) { + sid_match = buf_equal(sid, e->session_ids[i].sid) == 0; + key_match = sshkey_equal(key, e->session_ids[i].key); + if (sid_match && key_match) { + debug("%s: session ID already recorded for %s %s", __func__, + sshkey_type(key), fp); + r = 0; + goto out; + } else if (sid_match) { + error("%s: session ID recorded against different key " + "for %s %s", __func__, sshkey_type(key), fp); + r = -1; + goto out; + } + /* + * new sid with previously-seen key can happen, e.g. multiple + * connections to the same host. + */ + } + /* record new key/sid */ + if (e->nsession_ids >= AGENT_MAX_SESSION_IDS) { + error("%s: too many session IDs recorded", __func__); + goto out; + } + e->session_ids = xrecallocarray(e->session_ids, e->nsession_ids, + e->nsession_ids + 1, sizeof(*e->session_ids)); + i = e->nsession_ids++; + debug("%s: recorded %s %s (slot %zu of %d)", __func__, sshkey_type(key), fp, i, + AGENT_MAX_SESSION_IDS); + e->session_ids[i].key = key; + e->session_ids[i].forwarded = fwd != 0; + key = NULL; /* transferred */ + /* can't transfer sid; it's refcounted and scoped to request's life */ + if ((e->session_ids[i].sid = sshbuf_new()) == NULL) + fatal("%s: sshbuf_new", __func__); + if ((r = sshbuf_putb(e->session_ids[i].sid, sid)) != 0) + fatal("%s: sshbuf_putb session ID: %s", __func__, ssh_err(r)); + /* success */ + r = 0; + out: + sshkey_free(key); + sshbuf_free(sid); + sshbuf_free(sig); + return r == 0 ? 1 : 0; +} + +static void +process_extension(SocketEntry *e) +{ + int r, success = 0; + char *name; + + debug2("%s: entering", __func__); + if ((r = sshbuf_get_cstring(e->request, &name, NULL)) != 0) { + error("%s: parse: %s", __func__, ssh_err(r)); + goto send; + } + if (strcmp(name, "session-bind@openssh.com") == 0) + success = process_ext_session_bind(e); + else + debug("%s: unsupported extension \"%s\"", __func__, name); +send: + send_status(e, success); +} /* * dispatch incoming message. * returns 1 on success, 0 for incomplete messages or -1 on error. @@ -1019,6 +1175,9 @@ process_message(u_int socknum) process_remove_smartcard_key(e); break; #endif /* ENABLE_PKCS11 */ + case SSH_AGENTC_EXTENSION: + process_extension(e); + break; default: /* Unknown message. Respond with failure. */ error("Unknown message %d", type); -- 2.41.0