upstream: Extends the SK API to accept a set of key/value options

for all operations. These are intended to future-proof the API a little by
making it easier to specify additional fields for without having to change
the API version for each.

At present, only two options are defined: one to explicitly specify
the device for an operation (rather than accepting the middleware's
autoselection) and another to specify the FIDO2 username that may
be used when generating a resident key. These new options may be
invoked at key generation time via ssh-keygen -O

This also implements a suggestion from Markus to avoid "int" in favour
of uint32_t for the algorithm argument in the API, to make implementation
of ssh-sk-client/helper a little easier.

feedback, fixes and ok markus@

OpenBSD-Commit-ID: 973ce11704609022ab36abbdeb6bc23c8001eabc

1 files changed, 27 insertions, 18 deletions

diff --git a/ssh-sk-helper.c b/ssh-sk-helper.c
index 590ff850..85a461d5 100644
--- a/ssh-sk-helper.c
+++ b/ssh-sk-helper.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssh-sk-helper.c,v 1.6 2019/12/30 09:23:28 djm Exp $ */
+/* $OpenBSD: ssh-sk-helper.c,v 1.7 2020/01/06 02:00:46 djm Exp $ */
 /*
  * Copyright (c) 2019 Google LLC
  *
@@ -77,6 +77,17 @@ reply_error(int r, char *fmt, ...)
 	return resp;
 }
 
+/* If the specified string is zero length, then free it and replace with NULL */
+static void
+null_empty(char **s)
+{
+	if (s == NULL || *s == NULL || **s != '\0')
+		return;
+
+	free(*s);
+	*s = NULL;
+}
+
 static struct sshbuf *
 process_sign(struct sshbuf *req)
 {
@@ -108,10 +119,7 @@ process_sign(struct sshbuf *req)
 	    "msg len %zu, compat 0x%lx", __progname, sshkey_type(key),
 	    provider, msglen, (u_long)compat);
 
-	if (*pin == 0) {
-		free(pin);
-		pin = NULL;
-	}
+	null_empty(&pin);
 
 	if ((r = sshsk_sign(provider, key, &sig, &siglen,
 	    message, msglen, compat, pin)) != 0) {
@@ -138,7 +146,7 @@ process_enroll(struct sshbuf *req)
 {
 	int r;
 	u_int type;
-	char *provider, *application, *pin;
+	char *provider, *application, *pin, *device, *userid;
 	uint8_t flags;
 	struct sshbuf *challenge, *attest, *kbuf, *resp;
 	struct sshkey *key;
@@ -149,7 +157,9 @@ process_enroll(struct sshbuf *req)
 
 	if ((r = sshbuf_get_u32(req, &type)) != 0 ||
 	    (r = sshbuf_get_cstring(req, &provider, NULL)) != 0 ||
+	    (r = sshbuf_get_cstring(req, &device, NULL)) != 0 ||
 	    (r = sshbuf_get_cstring(req, &application, NULL)) != 0 ||
+	    (r = sshbuf_get_cstring(req, &userid, NULL)) != 0 ||
 	    (r = sshbuf_get_u8(req, &flags)) != 0 ||
 	    (r = sshbuf_get_cstring(req, &pin, NULL)) != 0 ||
 	    (r = sshbuf_froms(req, &challenge)) != 0)
@@ -163,13 +173,12 @@ process_enroll(struct sshbuf *req)
 		sshbuf_free(challenge);
 		challenge = NULL;
 	}
-	if (*pin == 0) {
-		free(pin);
-		pin = NULL;
-	}
+	null_empty(&device);
+	null_empty(&userid);
+	null_empty(&pin);
 
-	if ((r = sshsk_enroll((int)type, provider, application, flags, pin,
-	    challenge, &key, attest)) != 0) {
+	if ((r = sshsk_enroll((int)type, provider, device, application, userid,
+	    flags, pin, challenge, &key, attest)) != 0) {
 		resp = reply_error(r, "Enrollment failed: %s", ssh_err(r));
 		goto out;
 	}
@@ -200,7 +209,7 @@ static struct sshbuf *
 process_load_resident(struct sshbuf *req)
 {
 	int r;
-	char *provider, *pin;
+	char *provider, *pin, *device;
 	struct sshbuf *kbuf, *resp;
 	struct sshkey **keys = NULL;
 	size_t nkeys = 0, i;
@@ -209,17 +218,17 @@ process_load_resident(struct sshbuf *req)
 		fatal("%s: sshbuf_new failed", __progname);
 
 	if ((r = sshbuf_get_cstring(req, &provider, NULL)) != 0 ||
+	    (r = sshbuf_get_cstring(req, &device, NULL)) != 0 ||
 	    (r = sshbuf_get_cstring(req, &pin, NULL)) != 0)
 		fatal("%s: buffer error: %s", __progname, ssh_err(r));
 	if (sshbuf_len(req) != 0)
 		fatal("%s: trailing data in request", __progname);
 
-	if (*pin == 0) {
-		free(pin);
-		pin = NULL;
-	}
+	null_empty(&device);
+	null_empty(&pin);
 
-	if ((r = sshsk_load_resident(provider, pin, &keys, &nkeys)) != 0) {
+	if ((r = sshsk_load_resident(provider, device, pin,
+	    &keys, &nkeys)) != 0) {
 		resp = reply_error(r, " sshsk_load_resident failed: %s",
 		    ssh_err(r));
 		goto out;