Attachment 3085 Details for Bug 2675 – automatically set lifetimes, add -C, -f and -v options

[patch] automatically set lifetimes, add -C, -f and -v options

bz2765.diff (text/plain), 11.74 KB, created by Damien Miller on 2017-11-03 16:01:19 AEDT

(hide)

Description:

Filename:

MIME Type:

Creator: Damien Miller

Created: 2017-11-03 16:01:19 AEDT

Size: 11.74 KB

patch

obsolete

>diff --git a/ssh-add.1 b/ssh-add.1
>index d5da927..46b94b9 100644
>--- a/ssh-add.1
>+++ b/ssh-add.1
>@@ -43,7 +43,7 @@
> .Nd adds private key identities to the authentication agent
> .Sh SYNOPSIS
> .Nm ssh-add
>-.Op Fl cDdkLlqXx
>+.Op Fl cCDdfkLlqvXx
> .Op Fl E Ar fingerprint_hash
> .Op Fl t Ar life
> .Op Ar
>@@ -92,6 +92,9 @@ Confirmation is performed by
> Successful confirmation is signaled by a zero exit status from
> .Xr ssh-askpass 1 ,
> rather than text entered into the requester.
>+.It Fl C
>+When loading keys into or deleting keys from the agent, process certificate
>+keys only and plain keys.
> .It Fl D
> Deletes all identities from the agent.
> .It Fl d
>@@ -118,6 +121,14 @@ The default is
> .It Fl e Ar pkcs11
> Remove keys provided by the PKCS#11 shared library
> .Ar pkcs11 .
>+.It Fl f
>+When loading certificate keys, disable expiry checking and automatic lifetime
>+limits.
>+By default,
>+.Nm
>+will refuse to load expired certificates and will request the agent to
>+automatically delete added certificates shortly after their validity
>+period expires.
> .It Fl k
> When loading keys into or deleting keys from the agent, process plain private
> keys only and skip certificates.
>@@ -136,6 +147,16 @@ Set a maximum lifetime when adding identities to an agent.
> The lifetime may be specified in seconds or in a time format
> specified in
> .Xr sshd_config 5 .
>+.It Fl v
>+Verbose mode.
>+Causes
>+.Nm
>+to print debugging messages about its progress.
>+This is helpful for debugging moduli generation.
>+Multiple
>+.Fl v
>+options increase the verbosity.
>+The maximum is 3.
> .It Fl X
> Unlock the agent.
> .It Fl x
>diff --git a/ssh-add.c b/ssh-add.c
>index f017ec7..e6a3f43 100644
>--- a/ssh-add.c
>+++ b/ssh-add.c
>@@ -75,6 +75,8 @@ static char *default_files[] = {
> 
> static int fingerprint_hash = SSH_FP_HASH_DEFAULT;
> 
>+#define CERT_EXPIRY_GRACE	60	/* grace period for expired certs */
>+
> /* Default lifetime (0 == forever) */
> static int lifetime = 0;
> 
>@@ -94,25 +96,29 @@ clear_pass(void)
> }
> 
> static int
>-delete_file(int agent_fd, const char *filename, int key_only, int qflag)
>+delete_file(int agent_fd, const char *filename, int key_only,
>+    int cert_only, int qflag)
> {
> 	struct sshkey *public, *cert = NULL;
> 	char *certpath = NULL, *comment = NULL;
> 	int r, ret = -1;
> 
>-	if ((r = sshkey_load_public(filename, &public,  &comment)) != 0) {
>-		printf("Bad key file %s: %s\n", filename, ssh_err(r));
>-		return -1;
>-	}
>-	if ((r = ssh_remove_identity(agent_fd, public)) == 0) {
>-		if (!qflag) {
>-			fprintf(stderr, "Identity removed: %s (%s)\n",
>-			    filename, comment);
>+	if (!cert_only) {
>+		if ((r = sshkey_load_public(filename, &public,
>+		    &comment)) != 0) {
>+			printf("Bad key file %s: %s\n", filename, ssh_err(r));
>+			return -1;
> 		}
>-		ret = 0;
>-	} else
>-		fprintf(stderr, "Could not remove identity \"%s\": %s\n",
>-		    filename, ssh_err(r));
>+		if ((r = ssh_remove_identity(agent_fd, public)) == 0) {
>+			if (!qflag) {
>+				fprintf(stderr, "Identity removed: %s (%s)\n",
>+				    filename, comment);
>+			}
>+			ret = 0;
>+		} else
>+			fprintf(stderr, "Could not remove identity "
>+			    "\"%s\": %s\n", filename, ssh_err(r));
>+	}
> 
> 	if (key_only)
> 		goto out;
>@@ -175,14 +181,31 @@ delete_all(int agent_fd)
> 	return ret;
> }
> 
>+/* returns non-zero if certificate has expired */
> static int
>-add_file(int agent_fd, const char *filename, int key_only, int qflag)
>+cert_expired(const char *name, time_t now, const struct sshkey_cert *cert)
> {
>-	struct sshkey *private, *cert;
>+	char msg[256];
>+
>+	sshkey_format_cert_validity(cert, msg, sizeof(msg));
>+	if ((now < 0 || cert->valid_before < (u_int64_t)now)) {
>+		error("Certificate %s has expired, was valid %s", name, msg);
>+		return 1;
>+	}
>+	return 0;
>+}
>+
>+static int
>+add_file(int agent_fd, const char *filename,
>+    int key_only, int cert_only, int force, int qflag)
>+{
>+	struct sshkey *private = NULL, *cert = NULL;
> 	char *comment = NULL;
> 	char msg[1024], *certpath = NULL;
>-	int r, fd, ret = -1;
>+	int r, fd, ret = -1, cert_fail = 0;
> 	struct sshbuf *keyblob;
>+	time_t now;
>+	u_int64_t cert_lifetime;
> 
> 	if (strcmp(filename, "-") == 0) {
> 		fd = STDIN_FILENO;
>@@ -213,6 +236,28 @@ add_file(int agent_fd, const char *filename, int key_only, int qflag)
> 	}
> 	close(fd);
> 
>+	/* Load and check the certificate before unwrapping private key */
>+	if (!key_only) {
>+		xasprintf(&certpath, "%s-cert.pub", filename);
>+		if ((r = sshkey_load_public(certpath, &cert, NULL)) != 0) {
>+			if (r != SSH_ERR_SYSTEM_ERROR || errno != ENOENT)
>+				error("Failed to load certificate \"%s\": %s",
>+				    certpath, ssh_err(r));
>+			cert_fail = 1;
>+		} else {
>+			sshkey_format_cert_validity(cert->cert,
>+			    msg, sizeof(msg));
>+			debug("Certificate %s valid %s", certpath, msg);
>+			/* Skip expired certificates */
>+			if (!force &&
>+			    cert_expired(certpath, time(NULL), cert->cert)) {
>+				cert_fail = 1;
>+			}
>+		}
>+	}
>+	if (cert_fail && cert_only)
>+		goto out;
>+
> 	/* At first, try empty passphrase */
> 	if ((r = sshkey_parse_private_fileblob(keyblob, "", &private,
> 	    &comment)) != 0 && r != SSH_ERR_KEY_WRONG_PASSPHRASE) {
>@@ -260,42 +305,61 @@ add_file(int agent_fd, const char *filename, int key_only, int qflag)
> 		comment = xstrdup(filename);
> 	sshbuf_free(keyblob);
> 
>-	if ((r = ssh_add_identity_constrained(agent_fd, private, comment,
>-	    lifetime, confirm)) == 0) {
>-		fprintf(stderr, "Identity added: %s (%s)\n", filename, comment);
>-		ret = 0;
>-		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");
>-	} else {
>-		fprintf(stderr, "Could not add identity \"%s\": %s\n",
>-		    filename, ssh_err(r));
>+	if (!cert_only) {
>+		if ((r = ssh_add_identity_constrained(agent_fd,
>+		    private, comment, lifetime, confirm)) == 0) {
>+			fprintf(stderr, "Identity added: %s (%s)\n",
>+			    filename, comment);
>+			ret = 0;
>+			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");
>+		} else {
>+			fprintf(stderr, "Could not add identity \"%s\": %s\n",
>+			    filename, ssh_err(r));
>+		}
> 	}
> 
> 	/* Skip trying to load the cert if requested */
>-	if (key_only)
>+	if (key_only || cert_fail)
> 		goto out;
> 
>-	/* Now try to add the certificate flavour too */
>-	xasprintf(&certpath, "%s-cert.pub", filename);
>-	if ((r = sshkey_load_public(certpath, &cert, NULL)) != 0) {
>-		if (r != SSH_ERR_SYSTEM_ERROR || errno != ENOENT)
>-			error("Failed to load certificate \"%s\": %s",
>-			    certpath, ssh_err(r));
>-		goto out;
>-	}
>-
> 	if (!sshkey_equal_public(cert, private)) {
> 		error("Certificate %s does not match private key %s",
> 		    certpath, filename);
> 		sshkey_free(cert);
> 		goto out;
>-	} 
>+	}
> 
>-	/* Graft with private bits */
>+	/* Redo expiry check in case user lingered at passphrase prompt */
>+	now = time(NULL);
>+	if (!force && cert_expired(certpath, now, cert->cert))
>+		goto out;
>+
>+	/* Clamp agent lifetimes to cert lifetime plus some grace */
>+	cert_lifetime = cert->cert->valid_before - (u_int64_t)now;
>+	if (cert->cert->valid_before == ~(u_int64_t)0)
>+		debug("Certificate %s has no expiry time", certpath);
>+	else
>+		debug("Certificate %s has %llu seconds remaining",
>+		    certpath, cert_lifetime);
>+	if (cert_lifetime < (u_int64_t)0 - CERT_EXPIRY_GRACE)
>+		cert_lifetime += CERT_EXPIRY_GRACE;
>+	/*
>+	 * Don't fiddle lifetimes if the cert didn't have an expiry date,
>+	 * the lifetime would be greater than that which could be expressed
>+	 * on the wire, less than the lifetimes set by the user, or the -f
>+	 * (force) flag was set.
>+	 */
>+	if (force || cert->cert->valid_before == ~(u_int64_t)0 ||
>+	    cert_lifetime > UINT_MAX ||
>+	    (lifetime != 0 && (u_int64_t)lifetime < cert_lifetime))
>+		cert_lifetime = lifetime;
>+
>+	/* Graft certificate to private bits */
> 	if ((r = sshkey_to_certified(private)) != 0) {
> 		error("%s: sshkey_to_certified: %s", __func__, ssh_err(r));
> 		sshkey_free(cert);
>@@ -309,15 +373,16 @@ add_file(int agent_fd, const char *filename, int key_only, int qflag)
> 	sshkey_free(cert);
> 
> 	if ((r = ssh_add_identity_constrained(agent_fd, private, comment,
>-	    lifetime, confirm)) != 0) {
>+	    cert_lifetime, confirm)) != 0) {
> 		error("Certificate %s (%s) add failed: %s", certpath,
> 		    private->cert->key_id, ssh_err(r));
> 		goto out;
> 	}
> 	fprintf(stderr, "Certificate added: %s (%s)\n", certpath,
> 	    private->cert->key_id);
>-	if (lifetime != 0)
>-		fprintf(stderr, "Lifetime set to %d seconds\n", lifetime);
>+	if (cert_lifetime != 0)
>+		fprintf(stderr, "Lifetime set to %llu seconds\n",
>+		    cert_lifetime);
> 	if (confirm != 0)
> 		fprintf(stderr, "The user must confirm each use of the key\n");
>  out:
>@@ -424,13 +489,16 @@ lock_agent(int agent_fd, int lock)
> }
> 
> static int
>-do_file(int agent_fd, int deleting, int key_only, char *file, int qflag)
>+do_file(int agent_fd, int deleting, int key_only, int cert_only,
>+    int force, char *file, int qflag)
> {
> 	if (deleting) {
>-		if (delete_file(agent_fd, file, key_only, qflag) == -1)
>+		if (delete_file(agent_fd, file, key_only,
>+		    cert_only, qflag) == -1)
> 			return -1;
> 	} else {
>-		if (add_file(agent_fd, file, key_only, qflag) == -1)
>+		if (add_file(agent_fd, file, key_only, cert_only,
>+		    force, qflag) == -1)
> 			return -1;
> 	}
> 	return 0;
>@@ -463,8 +531,10 @@ 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, ret = 0;
>+	int deleting = 0, key_only = 0, cert_only = 0, force = 0;
> 	int xflag = 0, lflag = 0, Dflag = 0, qflag = 0;
>+	int log_level = SYSLOG_LEVEL_INFO;
> 
> 	ssh_malloc_init();	/* must be called before any mallocs */
> 	/* Ensure that fds 0, 1 and 2 are open or directed to /dev/null */
>@@ -474,6 +544,8 @@ main(int argc, char **argv)
> 
> 	setvbuf(stdout, NULL, _IOLBF, 0);
> 
>+	log_init(argv[0], SYSLOG_LEVEL_INFO, SYSLOG_FACILITY_USER, 1);
>+
> 	/* First, get a connection to the authentication agent. */
> 	switch (r = ssh_get_authentication_socket(&agent_fd)) {
> 	case 0:
>@@ -487,13 +559,19 @@ main(int argc, char **argv)
> 		exit(2);
> 	}
> 
>-	while ((ch = getopt(argc, argv, "klLcdDxXE:e:qs:t:")) != -1) {
>+	while ((ch = getopt(argc, argv, "CcDdfkLlqvXxE:e:s:t:")) != -1) {
> 		switch (ch) {
>+		case 'C':
>+			cert_only = 1;
>+			break;
> 		case 'E':
> 			fingerprint_hash = ssh_digest_alg_by_name(optarg);
> 			if (fingerprint_hash == -1)
> 				fatal("Invalid hash algorithm \"%s\"", optarg);
> 			break;
>+		case 'f':
>+			force = 1;
>+			break;
> 		case 'k':
> 			key_only = 1;
> 			break;
>@@ -535,6 +613,15 @@ main(int argc, char **argv)
> 		case 'q':
> 			qflag = 1;
> 			break;
>+		case 'v':
>+			if (log_level == SYSLOG_LEVEL_INFO)
>+				log_level = SYSLOG_LEVEL_DEBUG1;
>+			else {
>+				if (log_level >= SYSLOG_LEVEL_DEBUG1 &&
>+				    log_level < SYSLOG_LEVEL_DEBUG3)
>+					log_level++;
>+			}
>+			break;
> 		default:
> 			usage();
> 			ret = 1;
>@@ -542,7 +629,11 @@ main(int argc, char **argv)
> 		}
> 	}
> 
>-	if ((xflag != 0) + (lflag != 0) + (Dflag != 0) > 1)
>+	/* reinit */
>+	log_init(argv[0], log_level, SYSLOG_FACILITY_USER, 1);
>+
>+	if ((xflag != 0) + (lflag != 0) + (Dflag != 0) > 1 ||
>+	    (key_only && cert_only))
> 		fatal("Invalid combination of actions");
> 	else if (xflag) {
> 		if (lock_agent(agent_fd, xflag == 'x' ? 1 : 0) == -1)
>@@ -583,8 +674,8 @@ main(int argc, char **argv)
> 			    default_files[i]);
> 			if (stat(buf, &st) < 0)
> 				continue;
>-			if (do_file(agent_fd, deleting, key_only, buf,
>-			    qflag) == -1)
>+			if (do_file(agent_fd, deleting, key_only, cert_only,
>+			    force, buf, qflag) == -1)
> 				ret = 1;
> 			else
> 				count++;
>@@ -593,8 +684,8 @@ main(int argc, char **argv)
> 			ret = 1;
> 	} else {
> 		for (i = 0; i < argc; i++) {
>-			if (do_file(agent_fd, deleting, key_only,
>-			    argv[i], qflag) == -1)
>+			if (do_file(agent_fd, deleting, key_only, cert_only,
>+			    force, argv[i], qflag) == -1)
> 				ret = 1;
> 		}
> 	}

Actions: View | Diff

Attachments on bug 2675: 2935 | 2936 | 3085