upstream: Restrict ssh-agent from signing web challenges for FIDO
keys. When signing messages in ssh-agent using a FIDO key that has an application string that does not start with "ssh:", ensure that the message being signed is one of the forms expected for the SSH protocol (currently pubkey authentication and sshsig signatures). This prevents ssh-agent forwarding on a host that has FIDO keys attached granting the ability for the remote side to sign challenges for web authentication using those keys too. Note that the converse case of web browsers signing SSH challenges is already precluded because no web RP can have the "ssh:" prefix in the application string that we require. ok markus@ OpenBSD-Commit-ID: 9ab6012574ed0352d2f097d307f4a988222d1b19
This commit is contained in:
parent
9c5f64b6cb
commit
0c111eb84e
110
ssh-agent.c
110
ssh-agent.c
|
@ -1,4 +1,4 @@
|
|||
/* $OpenBSD: ssh-agent.c,v 1.257 2020/03/06 18:28:27 markus Exp $ */
|
||||
/* $OpenBSD: ssh-agent.c,v 1.258 2020/05/26 01:26:58 djm Exp $ */
|
||||
/*
|
||||
* Author: Tatu Ylonen <ylo@cs.hut.fi>
|
||||
* Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland
|
||||
|
@ -77,6 +77,7 @@
|
|||
|
||||
#include "xmalloc.h"
|
||||
#include "ssh.h"
|
||||
#include "ssh2.h"
|
||||
#include "sshbuf.h"
|
||||
#include "sshkey.h"
|
||||
#include "authfd.h"
|
||||
|
@ -167,6 +168,9 @@ static long lifetime = 0;
|
|||
|
||||
static int fingerprint_hash = SSH_FP_HASH_DEFAULT;
|
||||
|
||||
/* Refuse signing of non-SSH messages for web-origin FIDO keys */
|
||||
static int restrict_websafe = 1;
|
||||
|
||||
static void
|
||||
close_socket(SocketEntry *e)
|
||||
{
|
||||
|
@ -282,6 +286,80 @@ agent_decode_alg(struct sshkey *key, u_int flags)
|
|||
return NULL;
|
||||
}
|
||||
|
||||
/*
|
||||
* This function inspects a message to be signed by a FIDO key that has a
|
||||
* web-like application string (i.e. one that does not begin with "ssh:".
|
||||
* It checks that the message is one of those expected for SSH operations
|
||||
* (pubkey userauth, sshsig, CA key signing) to exclude signing challenges
|
||||
* for the web.
|
||||
*/
|
||||
static int
|
||||
check_websafe_message_contents(struct sshkey *key,
|
||||
const u_char *msg, size_t len)
|
||||
{
|
||||
int matched = 0;
|
||||
struct sshbuf *b;
|
||||
u_char m, n;
|
||||
char *cp1 = NULL, *cp2 = NULL;
|
||||
int r;
|
||||
struct sshkey *mkey = NULL;
|
||||
|
||||
if ((b = sshbuf_from(msg, len)) == NULL)
|
||||
fatal("%s: sshbuf_new", __func__);
|
||||
|
||||
/* SSH userauth request */
|
||||
if ((r = sshbuf_get_string_direct(b, NULL, NULL)) == 0 && /* sess_id */
|
||||
(r = sshbuf_get_u8(b, &m)) == 0 && /* SSH2_MSG_USERAUTH_REQUEST */
|
||||
(r = sshbuf_get_cstring(b, NULL, NULL)) == 0 && /* server user */
|
||||
(r = sshbuf_get_cstring(b, &cp1, NULL)) == 0 && /* service */
|
||||
(r = sshbuf_get_cstring(b, &cp2, NULL)) == 0 && /* method */
|
||||
(r = sshbuf_get_u8(b, &n)) == 0 && /* sig-follows */
|
||||
(r = sshbuf_get_cstring(b, NULL, NULL)) == 0 && /* alg */
|
||||
(r = sshkey_froms(b, &mkey)) == 0 && /* key */
|
||||
sshbuf_len(b) == 0) {
|
||||
debug("%s: parsed userauth", __func__);
|
||||
if (m == SSH2_MSG_USERAUTH_REQUEST && n == 1 &&
|
||||
strcmp(cp1, "ssh-connection") == 0 &&
|
||||
strcmp(cp2, "publickey") == 0 &&
|
||||
sshkey_equal(key, mkey)) {
|
||||
debug("%s: well formed userauth", __func__);
|
||||
matched = 1;
|
||||
}
|
||||
}
|
||||
free(cp1);
|
||||
free(cp2);
|
||||
sshkey_free(mkey);
|
||||
sshbuf_free(b);
|
||||
if (matched)
|
||||
return 1;
|
||||
|
||||
if ((b = sshbuf_from(msg, len)) == NULL)
|
||||
fatal("%s: sshbuf_new", __func__);
|
||||
cp1 = cp2 = NULL;
|
||||
mkey = NULL;
|
||||
|
||||
/* SSHSIG */
|
||||
if ((r = sshbuf_cmp(b, 0, "SSHSIG", 6)) == 0 &&
|
||||
(r = sshbuf_consume(b, 6)) == 0 &&
|
||||
(r = sshbuf_get_cstring(b, NULL, NULL)) == 0 && /* namespace */
|
||||
(r = sshbuf_get_string_direct(b, NULL, NULL)) == 0 && /* reserved */
|
||||
(r = sshbuf_get_cstring(b, NULL, NULL)) == 0 && /* hashalg */
|
||||
(r = sshbuf_get_string_direct(b, NULL, NULL)) == 0 && /* H(msg) */
|
||||
sshbuf_len(b) == 0) {
|
||||
debug("%s: parsed sshsig", __func__);
|
||||
matched = 1;
|
||||
}
|
||||
|
||||
sshbuf_free(b);
|
||||
if (matched)
|
||||
return 1;
|
||||
|
||||
/* XXX CA signature operation */
|
||||
|
||||
error("web-origin key attempting to sign non-SSH message");
|
||||
return 0;
|
||||
}
|
||||
|
||||
/* ssh2 only */
|
||||
static void
|
||||
process_sign_request2(SocketEntry *e)
|
||||
|
@ -314,14 +392,20 @@ process_sign_request2(SocketEntry *e)
|
|||
verbose("%s: user refused key", __func__);
|
||||
goto send;
|
||||
}
|
||||
if (sshkey_is_sk(id->key) &&
|
||||
(id->key->sk_flags & SSH_SK_USER_PRESENCE_REQD)) {
|
||||
if ((fp = sshkey_fingerprint(key, SSH_FP_HASH_DEFAULT,
|
||||
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);
|
||||
if (sshkey_is_sk(id->key)) {
|
||||
if (strncmp(id->key->sk_application, "ssh:", 4) != 0 &&
|
||||
!check_websafe_message_contents(key, data, dlen)) {
|
||||
/* error already logged */
|
||||
goto send;
|
||||
}
|
||||
if ((id->key->sk_flags & SSH_SK_USER_PRESENCE_REQD)) {
|
||||
if ((fp = sshkey_fingerprint(key, SSH_FP_HASH_DEFAULT,
|
||||
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);
|
||||
}
|
||||
}
|
||||
if ((r = sshkey_sign(id->key, &signature, &slen,
|
||||
data, dlen, agent_decode_alg(key, flags),
|
||||
|
@ -1212,7 +1296,7 @@ main(int ac, char **av)
|
|||
__progname = ssh_get_progname(av[0]);
|
||||
seed_rng();
|
||||
|
||||
while ((ch = getopt(ac, av, "cDdksE:a:P:t:")) != -1) {
|
||||
while ((ch = getopt(ac, av, "cDdksE:a:O:P:t:")) != -1) {
|
||||
switch (ch) {
|
||||
case 'E':
|
||||
fingerprint_hash = ssh_digest_alg_by_name(optarg);
|
||||
|
@ -1227,6 +1311,12 @@ main(int ac, char **av)
|
|||
case 'k':
|
||||
k_flag++;
|
||||
break;
|
||||
case 'O':
|
||||
if (strcmp(optarg, "no-restrict-websafe") == 0)
|
||||
restrict_websafe = 0;
|
||||
else
|
||||
fatal("Unknown -O option");
|
||||
break;
|
||||
case 'P':
|
||||
if (provider_whitelist != NULL)
|
||||
fatal("-P option already specified");
|
||||
|
|
Loading…
Reference in New Issue