1 files changed, 100 insertions, 10 deletions

diff --git a/ssh-agent.c b/ssh-agent.c
index e081413b8..effffffed 100644
--- a/ssh-agent.c
+++ b/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");