[PATCH] ssh-agent: Add support to load additional certificates

Thomas Jarosch thomas.jarosch at intra2net.com
Mon Jul 27 00:52:18 AEST 2015


Add support to load additional certificates
for already loaded private keys. Useful
if the private key is on a PKCS#11 hardware token.

The private keys inside ssh-agent are now using a refcount
to share the private parts between "Identities".
The reason for this change was that the PKCS#11 code
might have redirected ("wrap") the RSA functions to a hardware token.
We don't want to mess with those internals.

Tested with an OpenGPG card. Patch developed against 6.9p
and applies to original 6.9, too.

Please CC: comments.

Signed-off-by: Thomas Jarosch <thomas.jarosch at intra2net.com>

diff -u -r -p openssh-6.9p1/ssh-add.1 openssh.cert_shadow/ssh-add.1
--- openssh-6.9p1/ssh-add.1	2015-07-01 04:35:31.000000000 +0200
+++ openssh.cert_shadow/ssh-add.1	2015-07-26 16:02:14.119312097 +0200
@@ -122,6 +122,10 @@ Remove keys provided by the PKCS#11 shar
 .It Fl k
 When loading keys into or deleting keys from the agent, process plain private
 keys only and skip certificates.
+.It Fl p
+Load additional certificate for already loaded private key.
+Will refuse to load the certificate if no matching key is found.
+Useful if the private key is stored on a PKCS#11 hardware token.
 .It Fl L
 Lists public key parameters of all identities currently represented
 by the agent.
diff -u -r -p openssh-6.9p1/ssh-add.c openssh.cert_shadow/ssh-add.c
--- openssh-6.9p1/ssh-add.c	2015-07-01 04:35:31.000000000 +0200
+++ openssh.cert_shadow/ssh-add.c	2015-07-26 15:58:06.513151180 +0200
@@ -180,6 +180,49 @@ delete_all(int agent_fd)
 }
 
 static int
+add_certificate_only(int agent_fd, const char *filename)
+{
+	struct sshkey *cert = NULL;
+	char *comment = NULL;
+	int r, ret = -1;
+
+	/* Load certificate */
+	if ((r = sshkey_load_public(filename, &cert, &comment)) != 0) {
+		if (r != SSH_ERR_SYSTEM_ERROR || errno != ENOENT)
+			error("Failed to load certificate \"%s\": %s",
+			    filename, ssh_err(r));
+		goto out;
+	}
+	if (!sshkey_is_cert(cert)) {
+		error("Not a certificate: %s", filename);
+		goto out;
+	}
+
+        /* Add empty private key fields for serialization */
+	if ((r = sshkey_add_private(cert)) != 0)
+		goto out;
+
+	if ((r = ssh_add_identity_constrained(agent_fd, cert, comment,
+	    lifetime, confirm)) != 0) {
+		error("Certificate %s (%s) add failed: %s", filename,
+		    cert->cert->key_id, ssh_err(r));
+		goto out;
+	}
+	ret = 0;
+	fprintf(stderr, "Certificate added: %s (%s)\n", filename,
+	    cert->cert->key_id);
+	if (lifetime != 0)
+		fprintf(stderr, "Lifetime set to %d seconds\n", lifetime);
+	if (confirm != 0)
+		fprintf(stderr, "The user must confirm each use of the key\n");
+ out:
+	free(comment);
+	sshkey_free(cert);
+
+	return ret;
+}
+
+static int
 add_file(int agent_fd, const char *filename, int key_only)
 {
 	struct sshkey *private, *cert;
@@ -445,13 +488,16 @@ lock_agent(int agent_fd, int lock)
 }
 
 static int
-do_file(int agent_fd, int deleting, int key_only, char *file)
+do_file(int agent_fd, int deleting, int key_only, int cert_only, char *file)
 {
 	if (deleting) {
 		if (delete_file(agent_fd, file, key_only) == -1)
 			return -1;
 	} else {
-		if (add_file(agent_fd, file, key_only) == -1)
+		if (cert_only) {
+			if (add_certificate_only(agent_fd, file) == -1)
+				return -1;
+		} else if (add_file(agent_fd, file, key_only) == -1)
 			return -1;
 	}
 	return 0;
@@ -466,6 +512,7 @@ usage(void)
 	fprintf(stderr, "  -E hash     Specify hash algorithm used for fingerprints.\n");
 	fprintf(stderr, "  -L          List public key parameters of all identities.\n");
 	fprintf(stderr, "  -k          Load only keys and not certificates.\n");
+	fprintf(stderr, "  -p          Load additional certificate. Private key must be loaded.\n");
 	fprintf(stderr, "  -c          Require confirmation to sign using identities\n");
 	fprintf(stderr, "  -t life     Set lifetime (in seconds) when adding identities.\n");
 	fprintf(stderr, "  -d          Delete identity.\n");
@@ -483,7 +530,7 @@ main(int argc, char **argv)
 	extern int optind;
 	int agent_fd;
 	char *pkcs11provider = NULL;
-	int r, i, ch, deleting = 0, ret = 0, key_only = 0;
+	int r, i, ch, deleting = 0, ret = 0, key_only = 0, cert_only = 0;
 	int xflag = 0, lflag = 0, Dflag = 0;
 
 	/* Ensure that fds 0, 1 and 2 are open or directed to /dev/null */
@@ -511,7 +558,7 @@ main(int argc, char **argv)
 		exit(2);
 	}
 
-	while ((ch = getopt(argc, argv, "klLcdDxXE:e:s:t:")) != -1) {
+	while ((ch = getopt(argc, argv, "kplLcdDxXE:e:s:t:")) != -1) {
 		switch (ch) {
 		case 'E':
 			fingerprint_hash = ssh_digest_alg_by_name(optarg);
@@ -519,8 +566,15 @@ main(int argc, char **argv)
 				fatal("Invalid hash algorithm \"%s\"", optarg);
 			break;
 		case 'k':
+			if (cert_only)
+				fatal("-k and -p are incompatible");
 			key_only = 1;
 			break;
+		case 'p':
+			if (key_only)
+				fatal("-k and -p are incompatible");
+			cert_only = 1;
+			break;
 		case 'l':
 		case 'L':
 			if (lflag != 0)
@@ -604,7 +658,7 @@ main(int argc, char **argv)
 			    default_files[i]);
 			if (stat(buf, &st) < 0)
 				continue;
-			if (do_file(agent_fd, deleting, key_only, buf) == -1)
+			if (do_file(agent_fd, deleting, key_only, cert_only, buf) == -1)
 				ret = 1;
 			else
 				count++;
@@ -613,7 +667,7 @@ main(int argc, char **argv)
 			ret = 1;
 	} else {
 		for (i = 0; i < argc; i++) {
-			if (do_file(agent_fd, deleting, key_only,
+			if (do_file(agent_fd, deleting, key_only, cert_only,
 			    argv[i]) == -1)
 				ret = 1;
 		}
diff -u -r -p openssh-6.9p1/ssh-agent.c openssh.cert_shadow/ssh-agent.c
--- openssh-6.9p1/ssh-agent.c	2015-07-01 04:35:31.000000000 +0200
+++ openssh.cert_shadow/ssh-agent.c	2015-07-26 14:59:53.733842195 +0200
@@ -112,9 +112,15 @@ typedef struct {
 u_int sockets_alloc = 0;
 SocketEntry *sockets = NULL;
 
+typedef struct refcountkey {
+	struct sshkey *key;
+	int count;
+} RefcountKey;
+
 typedef struct identity {
 	TAILQ_ENTRY(identity) next;
-	struct sshkey *key;
+	RefcountKey *idkey;
+	RefcountKey *shadowed_key;
 	char *comment;
 	char *provider;
 	time_t death;
@@ -188,16 +194,43 @@ idtab_lookup(int version)
 	return &idtable[version];
 }
 
+static RefcountKey *
+refkey_new(struct sshkey *key)
+{
+	RefcountKey *ref = xcalloc(1, sizeof(RefcountKey));
+
+	ref->key = key;
+	ref->count = 1;
+	
+	return ref;
+}
+
+static RefcountKey *
+refkey_addref(RefcountKey *refkey)
+{
+	++refkey->count;
+	return refkey;
+}
+
+static void refkey_unref(RefcountKey *refkey)
+{
+	if (refkey && --refkey->count <= 0) {
+		sshkey_free(refkey->key);
+		free(refkey);
+	}
+}
+
 static void
 free_identity(Identity *id)
 {
-	sshkey_free(id->key);
+	refkey_unref(id->idkey);
+	refkey_unref(id->shadowed_key);
 	free(id->provider);
 	free(id->comment);
 	free(id);
 }
 
-/* return matching private key for given public key */
+/* return matching Identity for given public key */
 static Identity *
 lookup_identity(struct sshkey *key, int version)
 {
@@ -205,7 +238,22 @@ lookup_identity(struct sshkey *key, int
 
 	Idtab *tab = idtab_lookup(version);
 	TAILQ_FOREACH(id, &tab->idlist, next) {
-		if (sshkey_equal(key, id->key))
+		if (sshkey_equal(key, id->idkey->key))
+			return (id);
+	}
+	return (NULL);
+}
+
+/* return matching private key for given public key */
+static Identity *
+lookup_identity_unshadowed_key(struct sshkey *key, int version)
+{
+	Identity *id;
+
+	Idtab *tab = idtab_lookup(version);
+	TAILQ_FOREACH(id, &tab->idlist, next) {
+		if (sshkey_equal_public(key, id->idkey->key) &&
+		    id->shadowed_key == NULL)
 			return (id);
 	}
 	return (NULL);
@@ -218,7 +266,7 @@ confirm_key(Identity *id)
 	char *p;
 	int ret = -1;
 
-	p = sshkey_fingerprint(id->key, fingerprint_hash, SSH_FP_DEFAULT);
+	p = sshkey_fingerprint(id->idkey->key, fingerprint_hash, SSH_FP_DEFAULT);
 	if (p != NULL &&
 	    ask_permission("Allow use of key %s?\nKey fingerprint %s.",
 	    id->comment, p))
@@ -256,14 +304,14 @@ process_request_identities(SocketEntry *
 	    (r = sshbuf_put_u32(msg, tab->nentries)) != 0)
 		fatal("%s: buffer error: %s", __func__, ssh_err(r));
 	TAILQ_FOREACH(id, &tab->idlist, next) {
-		if (id->key->type == KEY_RSA1) {
+		if (id->idkey->key->type == KEY_RSA1) {
 #ifdef WITH_SSH1
 			if ((r = sshbuf_put_u32(msg,
-			    BN_num_bits(id->key->rsa->n))) != 0 ||
+			    BN_num_bits(id->idkey->key->rsa->n))) != 0 ||
 			    (r = sshbuf_put_bignum1(msg,
-			    id->key->rsa->e)) != 0 ||
+			    id->idkey->key->rsa->e)) != 0 ||
 			    (r = sshbuf_put_bignum1(msg,
-			    id->key->rsa->n)) != 0)
+			    id->idkey->key->rsa->n)) != 0)
 				fatal("%s: buffer error: %s",
 				    __func__, ssh_err(r));
 #endif
@@ -271,7 +319,7 @@ process_request_identities(SocketEntry *
 			u_char *blob;
 			size_t blen;
 
-			if ((r = sshkey_to_blob(id->key, &blob, &blen)) != 0) {
+			if ((r = sshkey_to_blob(id->idkey->key, &blob, &blen)) != 0) {
 				error("%s: sshkey_to_blob: %s", __func__,
 				    ssh_err(r));
 				continue;
@@ -327,7 +375,7 @@ process_authentication_challenge1(Socket
 
 	id = lookup_identity(key, 1);
 	if (id != NULL && (!id->confirm || confirm_key(id) == 0)) {
-		struct sshkey *private = id->key;
+		struct sshkey *private = id->idkey->key;
 		/* Decrypt the challenge using the private key. */
 		if ((r = rsa_private_decrypt(challenge, challenge,
 		    private->rsa) != 0)) {
@@ -380,7 +428,7 @@ process_sign_request2(SocketEntry *e)
 	u_int compat = 0, flags;
 	int r, ok = -1;
 	struct sshbuf *msg;
-	struct sshkey *key;
+	struct sshkey *key, *sign_key;
 	struct identity *id;
 
 	if ((msg = sshbuf_new()) == NULL)
@@ -403,7 +451,12 @@ process_sign_request2(SocketEntry *e)
 		verbose("%s: user refused key", __func__);
 		goto send;
 	}
-	if ((r = sshkey_sign(id->key, &signature, &slen,
+
+	if (id->shadowed_key)
+		sign_key = id->shadowed_key->key;
+	else
+		sign_key = id->idkey->key;
+	if ((r = sshkey_sign(sign_key, &signature, &slen,
 	    data, dlen, compat)) != 0) {
 		error("%s: sshkey_sign: %s", __func__, ssh_err(ok));
 		goto send;
@@ -643,12 +696,38 @@ process_add_identity(SocketEntry *e, int
 		}
 	}
 
-	success = 1;
 	if (lifetime && !death)
 		death = monotime() + lifetime;
+
+	/* handle additional certificates for an existing private key */
+	if (!sshkey_is_private(k)) {
+		id = lookup_identity_unshadowed_key(k, version);
+		/* ensure we have a private key and this cert is new */
+		if (id != NULL && lookup_identity(k, version) == NULL) {
+			Identity *certid = xcalloc(1, sizeof(Identity));
+			certid->idkey = refkey_new(k);
+			certid->shadowed_key = refkey_addref(id->idkey);
+			if (id->provider)
+				certid->provider = xstrdup(id->provider);
+			if (id->comment)
+				certid->comment = xstrdup(id->comment); /* XXX */
+			certid->death = death;
+			certid->confirm = confirm | id->confirm;
+
+			TAILQ_INSERT_TAIL(&tab->idlist, certid, next);
+			tab->nentries++;
+			success = 1;
+		} else
+			sshkey_free(k);
+
+		free(comment);
+		goto send;
+	}
+
+	success = 1;
 	if ((id = lookup_identity(k, version)) == NULL) {
 		id = xcalloc(1, sizeof(Identity));
-		id->key = k;
+		id->idkey = refkey_new(k);
 		TAILQ_INSERT_TAIL(&tab->idlist, id, next);
 		/* Increment the number of identities. */
 		tab->nentries++;
@@ -774,7 +853,7 @@ process_add_smartcard_key(SocketEntry *e
 		tab = idtab_lookup(version);
 		if (lookup_identity(k, version) == NULL) {
 			id = xcalloc(1, sizeof(Identity));
-			id->key = k;
+			id->idkey = refkey_new(k);
 			id->provider = xstrdup(provider);
 			id->comment = xstrdup(provider); /* XXX */
 			id->death = death;
diff -u -r -p openssh-6.9p1/sshkey.c openssh.cert_shadow/sshkey.c
--- openssh-6.9p1/sshkey.c	2015-07-01 04:35:31.000000000 +0200
+++ openssh.cert_shadow/sshkey.c	2015-07-26 13:55:40.978410299 +0200
@@ -324,6 +324,48 @@ sshkey_is_cert(const struct sshkey *k)
 	return sshkey_type_is_cert(k->type);
 }
 
+/* TODO: Please review carefully */
+int
+sshkey_is_private(const struct sshkey *k)
+{
+	switch (k->type) {
+#ifdef WITH_OPENSSL
+	case KEY_RSA1:
+	case KEY_RSA:
+	case KEY_RSA_CERT_V00:
+	case KEY_RSA_CERT:
+		if (k->rsa && k->rsa->d && k->rsa->q && k->rsa->p &&
+			k->rsa->iqmp &&
+			!BN_is_zero(k->rsa->d) &&
+			!BN_is_zero(k->rsa->q) &&
+			!BN_is_zero(k->rsa->p) &&
+			!BN_is_zero(k->rsa->iqmp))
+				return 1;
+		break;
+	case KEY_DSA:
+	case KEY_DSA_CERT_V00:
+	case KEY_DSA_CERT:
+		if (k->dsa && k->dsa->priv_key)
+			return 1;
+		break;
+	case KEY_ECDSA:
+	case KEY_ECDSA_CERT:
+		if (k->ecdsa && EC_KEY_get0_private_key(k->ecdsa))
+			return 1;
+		break;
+#endif /* WITH_OPENSSL */
+	case KEY_ED25519:
+	case KEY_ED25519_CERT:
+		if (k->ed25519_sk)
+			return 1;
+		break;
+	case KEY_UNSPEC:
+		break;
+	}
+
+	return 0;
+}
+
 /* Return the cert-less equivalent to a certified key type */
 int
 sshkey_type_plain(int type)
diff -u -r -p openssh-6.9p1/sshkey.h openssh.cert_shadow/sshkey.h
--- openssh-6.9p1/sshkey.h	2015-07-01 04:35:31.000000000 +0200
+++ openssh.cert_shadow/sshkey.h	2015-07-26 11:15:33.344024398 +0200
@@ -135,6 +135,7 @@ int		 sshkey_generate(int type, u_int bi
 int		 sshkey_from_private(const struct sshkey *, struct sshkey **);
 int	 sshkey_type_from_name(const char *);
 int	 sshkey_is_cert(const struct sshkey *);
+int	 sshkey_is_private(const struct sshkey *);
 int	 sshkey_type_is_cert(int);
 int	 sshkey_type_plain(int);
 int	 sshkey_to_certified(struct sshkey *, int);


More information about the openssh-unix-dev mailing list