Attachment 2909 Details for Bug 2617 – consolidated and tweaked patches

[patch] consolidated and tweaked patches

bz2643.diff (text/plain), 8.46 KB, created by Damien Miller on 2016-12-09 13:49:59 AEDT

(hide)

Description:

Filename:

MIME Type:

Creator: Damien Miller

Created: 2016-12-09 13:49:59 AEDT

Size: 8.46 KB

patch

obsolete

>Index: usr.bin/ssh/sshconnect2.c
>===================================================================
>RCS file: /cvs/src/usr.bin/ssh/sshconnect2.c,v
>retrieving revision 1.251
>diff -u -p -r1.251 sshconnect2.c
>--- usr.bin/ssh/sshconnect2.c	4 Dec 2016 23:54:02 -0000	1.251
>+++ usr.bin/ssh/sshconnect2.c	9 Dec 2016 02:45:43 -0000
>@@ -988,11 +988,11 @@ input_userauth_passwd_changereq(int type
> }
> 
> static const char *
>-identity_sign_encode(struct identity *id)
>+key_sign_encode(const struct sshkey *key)
> {
> 	struct ssh *ssh = active_state;
> 
>-	if (id->key->type == KEY_RSA) {
>+	if (key->type == KEY_RSA) {
> 		switch (ssh->kex->rsa_sha2) {
> 		case 256:
> 			return "rsa-sha2-256";
>@@ -1000,7 +1000,7 @@ identity_sign_encode(struct identity *id
> 			return "rsa-sha2-512";
> 		}
> 	}
>-	return key_ssh_name(id->key);
>+	return key_ssh_name(key);
> }
> 
> static int
>@@ -1009,31 +1009,50 @@ identity_sign(struct identity *id, u_cha
> {
> 	Key *prv;
> 	int ret;
>-	const char *alg;
>-
>-	alg = identity_sign_encode(id);
> 
> 	/* the agent supports this key */
>-	if (id->agent_fd != -1)
>+	if (id->key != NULL && id->agent_fd != -1)
> 		return ssh_agent_sign(id->agent_fd, id->key, sigp, lenp,
>-		    data, datalen, alg, compat);
>+		    data, datalen, key_sign_encode(id->key), compat);
> 
> 	/*
> 	 * we have already loaded the private key or
> 	 * the private key is stored in external hardware
> 	 */
>-	if (id->isprivate || (id->key->flags & SSHKEY_FLAG_EXT))
>-		return (sshkey_sign(id->key, sigp, lenp, data, datalen, alg,
>-		    compat));
>+	if (id->key != NULL &&
>+	    (id->isprivate || (id->key->flags & SSHKEY_FLAG_EXT)))
>+		return (sshkey_sign(id->key, sigp, lenp, data, datalen,
>+		    key_sign_encode(id->key), compat));
>+
> 	/* load the private key from the file */
> 	if ((prv = load_identity_file(id)) == NULL)
> 		return SSH_ERR_KEY_NOT_FOUND;
>-	ret = sshkey_sign(prv, sigp, lenp, data, datalen, alg, compat);
>+	ret = sshkey_sign(prv, sigp, lenp, data, datalen,
>+	    key_sign_encode(prv), compat);
> 	sshkey_free(prv);
> 	return (ret);
> }
> 
> static int
>+id_filename_matches(Identity *id, Identity *private_id)
>+{
>+	const char *suffixes[] = { ".pub", "-cert.pub", NULL };
>+	size_t len = strlen(id->filename), plen = strlen(private_id->filename);
>+	size_t i, slen;
>+
>+	if (strcmp(id->filename, private_id->filename) == 0)
>+		return 1;
>+	for (i = 0; suffixes[i]; i++) {
>+		slen = strlen(suffixes[i]);
>+		if (len > slen && plen == len - slen &&
>+		    strcmp(id->filename + (len - slen), suffixes[i]) == 0 &&
>+		    memcmp(id->filename, private_id->filename, plen) == 0)
>+			return 1;
>+	}
>+	return 0;
>+}
>+
>+static int
> sign_and_send_pubkey(Authctxt *authctxt, Identity *id)
> {
> 	Buffer b;
>@@ -1075,7 +1094,7 @@ sign_and_send_pubkey(Authctxt *authctxt,
> 	} else {
> 		buffer_put_cstring(&b, authctxt->method->name);
> 		buffer_put_char(&b, have_sig);
>-		buffer_put_cstring(&b, identity_sign_encode(id));
>+		buffer_put_cstring(&b, key_sign_encode(id->key));
> 	}
> 	buffer_put_string(&b, blob, bloblen);
> 
>@@ -1095,6 +1114,24 @@ sign_and_send_pubkey(Authctxt *authctxt,
> 				break;
> 			}
> 		}
>+		/*
>+		 * Exact key matches are preferred, but also allow
>+		 * filename matches for non-PKCS#11/agent keys that
>+		 * didn't load public keys. This supports the case
>+		 * of keeping just a private key file and public
>+		 * certificate on disk.
>+		 */
>+		if (!matched && !id->isprivate && id->agent_fd == -1 &&
>+		    (id->key->flags & SSHKEY_FLAG_EXT) == 0) {
>+			TAILQ_FOREACH(private_id, &authctxt->keys, next) {
>+				if (private_id->key == NULL &&
>+				    id_filename_matches(id, private_id)) {
>+					id = private_id;
>+					matched = 1;
>+					break;
>+				}
>+			}
>+		}
> 		if (matched) {
> 			debug2("%s: using private key \"%s\"%s for "
> 			    "certificate", __func__, id->filename,
>@@ -1173,7 +1210,7 @@ send_pubkey_test(Authctxt *authctxt, Ide
> 	packet_put_cstring(authctxt->method->name);
> 	packet_put_char(have_sig);
> 	if (!(datafellows & SSH_BUG_PKAUTH))
>-		packet_put_cstring(identity_sign_encode(id));
>+		packet_put_cstring(key_sign_encode(id->key));
> 	packet_put_string(blob, bloblen);
> 	free(blob);
> 	packet_send();
>Index: regress/usr.bin/ssh/cert-file.sh
>===================================================================
>RCS file: /cvs/src/regress/usr.bin/ssh/cert-file.sh,v
>retrieving revision 1.2
>diff -u -p -r1.2 cert-file.sh
>--- regress/usr.bin/ssh/cert-file.sh	24 Sep 2015 07:15:39 -0000	1.2
>+++ regress/usr.bin/ssh/cert-file.sh	9 Dec 2016 02:45:43 -0000
>@@ -17,24 +17,59 @@ ${SSHKEYGEN} -q -N '' -t ed25519 -f $OBJ
> 	fatal "ssh-keygen failed"
> ${SSHKEYGEN} -q -N '' -t ed25519 -f $OBJ/user_key2 || \
> 	fatal "ssh-keygen failed"
>+${SSHKEYGEN} -q -N '' -t ed25519 -f $OBJ/user_key3 || \
>+	fatal "ssh-keygen failed"
>+${SSHKEYGEN} -q -N '' -t ed25519 -f $OBJ/user_key4 || \
>+	fatal "ssh-keygen failed"
>+${SSHKEYGEN} -q -N '' -t ed25519 -f $OBJ/user_key5 || \
>+	fatal "ssh-keygen failed"
>+
> # Move the certificate to a different address to better control
> # when it is offered.
> ${SSHKEYGEN} -q -s $OBJ/user_ca_key1 -I "regress user key for $USER" \
> 	-z $$ -n ${USER} $OBJ/user_key1 ||
>-		fail "couldn't sign user_key1 with user_ca_key1"
>+		fatal "couldn't sign user_key1 with user_ca_key1"
> mv $OBJ/user_key1-cert.pub $OBJ/cert_user_key1_1.pub
> ${SSHKEYGEN} -q -s $OBJ/user_ca_key2 -I "regress user key for $USER" \
> 	-z $$ -n ${USER} $OBJ/user_key1 ||
>-		fail "couldn't sign user_key1 with user_ca_key2"
>+		fatal "couldn't sign user_key1 with user_ca_key2"
> mv $OBJ/user_key1-cert.pub $OBJ/cert_user_key1_2.pub
>+${SSHKEYGEN} -q -s $OBJ/user_ca_key1 -I "regress user key for $USER" \
>+	-z $$ -n ${USER} $OBJ/user_key3 ||
>+		fatal "couldn't sign user_key3 with user_ca_key1"
>+rm $OBJ/user_key3.pub # to test use of private key w/o public half.
>+${SSHKEYGEN} -q -s $OBJ/user_ca_key1 -I "regress user key for $USER" \
>+	-z $$ -n ${USER} $OBJ/user_key4 ||
>+		fatal "couldn't sign user_key4 with user_ca_key1"
>+rm $OBJ/user_key4 $OBJ/user_key4.pub # to test no matching pub/private key case.
> 
> trace 'try with identity files'
> opts="-F $OBJ/ssh_proxy -oIdentitiesOnly=yes"
> opts2="$opts -i $OBJ/user_key1 -i $OBJ/user_key2"
> echo "cert-authority $(cat $OBJ/user_ca_key1.pub)" > $OBJ/authorized_keys_$USER
> 
>+# Make a clean config that doesn't have any pre-added identities.
>+cat $OBJ/ssh_proxy | grep -v IdentityFile > $OBJ/no_identity_config
>+
>+# XXX: verify that certificate used was what we expect. Needs exposure of
>+# keys via enviornment variable or similar.
>+
> for p in ${SSH_PROTOCOLS}; do
>+	# Key with no .pub should work - finding the equivalent *-cert.pub.
>+	verbose "protocol $p: identity cert with no plain public file"
>+	${SSH} -F $OBJ/no_identity_config -oIdentitiesOnly=yes \
>+	    -i $OBJ/user_key3 somehost exit 5$p
>+	[ $? -ne 5$p ] && fail "ssh failed"
>+
>+	# CertificateFile matching private key with no .pub file should work.
>+	verbose "protocol $p: CertificateFile with no plain public file"
>+	${SSH} -F $OBJ/no_identity_config -oIdentitiesOnly=yes \
>+	    -oCertificateFile=$OBJ/user_key3-cert.pub \
>+	    -i $OBJ/user_key3 somehost exit 5$p
>+	[ $? -ne 5$p ] && fail "ssh failed"
>+
> 	# Just keys should fail
>+	verbose "protocol $p: plain keys"
> 	${SSH} $opts2 somehost exit 5$p
> 	r=$?
> 	if [ $r -eq 5$p ]; then
>@@ -42,6 +77,7 @@ for p in ${SSH_PROTOCOLS}; do
> 	fi
> 
> 	# Keys with untrusted cert should fail.
>+	verbose "protocol $p: untrusted cert"
> 	opts3="$opts2 -oCertificateFile=$OBJ/cert_user_key1_2.pub"
> 	${SSH} $opts3 somehost exit 5$p
> 	r=$?
>@@ -50,6 +86,7 @@ for p in ${SSH_PROTOCOLS}; do
> 	fi
> 
> 	# Good cert with bad key should fail.
>+	verbose "protocol $p: good cert, bad key"
> 	opts3="$opts -i $OBJ/user_key2"
> 	opts3="$opts3 -oCertificateFile=$OBJ/cert_user_key1_1.pub"
> 	${SSH} $opts3 somehost exit 5$p
>@@ -59,6 +96,7 @@ for p in ${SSH_PROTOCOLS}; do
> 	fi
> 
> 	# Keys with one trusted cert, should succeed.
>+	verbose "protocol $p: single trusted"
> 	opts3="$opts2 -oCertificateFile=$OBJ/cert_user_key1_1.pub"
> 	${SSH} $opts3 somehost exit 5$p
> 	r=$?
>@@ -67,20 +105,13 @@ for p in ${SSH_PROTOCOLS}; do
> 	fi
> 
> 	# Multiple certs and keys, with one trusted cert, should succeed.
>+	verbose "protocol $p: multiple trusted"
> 	opts3="$opts2 -oCertificateFile=$OBJ/cert_user_key1_2.pub"
> 	opts3="$opts3 -oCertificateFile=$OBJ/cert_user_key1_1.pub"
> 	${SSH} $opts3 somehost exit 5$p
> 	r=$?
> 	if [ $r -ne 5$p ]; then
> 		fail "ssh failed with multiple certs in protocol $p"
>-	fi
>-
>-	#Keys with trusted certificate specified in config options, should succeed.
>-	opts3="$opts2 -oCertificateFile=$OBJ/cert_user_key1_1.pub"
>-	${SSH} $opts3 somehost exit 5$p
>-	r=$?
>-	if [ $r -ne 5$p ]; then
>-		fail "ssh failed with trusted cert in config in protocol $p"
> 	fi
> done
>

Flags:

dtucker: ok+

Actions: View | Diff

Attachments on bug 2617: 2884 | 2886 | 2899 | 2901 | 2902 | 2903 | 2904 | 2905 | 2906 | 2909