Attachment 2516 Details for Bug 2323 – Require multiple publickey entries in AuthenticationMethods use different keys

[patch] Require multiple publickey entries in AuthenticationMethods use different keys

multipubkey.diff (text/plain), 8.37 KB, created by Damien Miller on 2014-12-11 14:50:55 AEDT

(hide)

Description:

Filename:

MIME Type:

Creator: Damien Miller

Created: 2014-12-11 14:50:55 AEDT

Size: 8.37 KB

patch

obsolete

>Index: usr.bin/ssh/auth.h
>===================================================================
>RCS file: /cvs/src/usr.bin/ssh/auth.h,v
>retrieving revision 1.78
>diff -u -p -r1.78 auth.h
>--- usr.bin/ssh/auth.h	3 Jul 2014 11:16:55 -0000	1.78
>+++ usr.bin/ssh/auth.h	11 Dec 2014 03:29:13 -0000
>@@ -37,6 +37,8 @@
> #include <krb5.h>
> #endif
> 
>+struct sshkey;
>+
> typedef struct Authctxt Authctxt;
> typedef struct Authmethod Authmethod;
> typedef struct KbdintDevice KbdintDevice;
>@@ -66,6 +68,9 @@ struct Authctxt {
> 	char		*krb5_ticket_file;
> #endif
> 	void		*methoddata;
>+
>+	struct sshkey	**prev_userkeys;
>+	u_int		 nprev_userkeys;
> };
> /*
>  * Every authentication method has to handle authentication requests for
>@@ -114,6 +119,8 @@ int	 hostbased_key_allowed(struct passwd
> int	 user_key_allowed(struct passwd *, Key *);
> void	 pubkey_auth_info(Authctxt *, const Key *, const char *, ...)
> 	    __attribute__((__format__ (printf, 3, 4)));
>+void	 auth2_record_userkey(Authctxt *, struct sshkey *);
>+int	 auth2_userkey_already_used(Authctxt *, struct sshkey *);
> 
> struct stat;
> int	 auth_secure_path(const char *, struct stat *, const char *, uid_t,
>Index: usr.bin/ssh/auth2-pubkey.c
>===================================================================
>RCS file: /cvs/src/usr.bin/ssh/auth2-pubkey.c,v
>retrieving revision 1.42
>diff -u -p -r1.42 auth2-pubkey.c
>--- usr.bin/ssh/auth2-pubkey.c	4 Dec 2014 02:24:32 -0000	1.42
>+++ usr.bin/ssh/auth2-pubkey.c	11 Dec 2014 03:29:13 -0000
>@@ -38,6 +38,7 @@
> #include <string.h>
> #include <time.h>
> #include <unistd.h>
>+#include <limits.h>
> 
> #include "xmalloc.h"
> #include "ssh.h"
>@@ -119,6 +120,10 @@ userauth_pubkey(Authctxt *authctxt)
> 		    "signature scheme");
> 		goto done;
> 	}
>+	if (auth2_userkey_already_used(authctxt, key)) {
>+		logit("refusing previously-used %s key", key_type(key));
>+		goto done;
>+	}
> 	if (have_sig) {
> 		sig = packet_get_string(&slen);
> 		packet_check_eom();
>@@ -156,8 +161,12 @@ userauth_pubkey(Authctxt *authctxt)
> 		authenticated = 0;
> 		if (PRIVSEP(user_key_allowed(authctxt->pw, key)) &&
> 		    PRIVSEP(key_verify(key, sig, slen, buffer_ptr(&b),
>-		    buffer_len(&b))) == 1)
>+		    buffer_len(&b))) == 1) {
> 			authenticated = 1;
>+			/* Record the successful key to prevent reuse */
>+			auth2_record_userkey(authctxt, key);
>+			key = NULL; /* Don't free below */
>+		}
> 		buffer_free(&b);
> 		free(sig);
> 	} else {
>@@ -675,6 +684,35 @@ user_key_allowed(struct passwd *pw, Key 
> 	}
> 
> 	return success;
>+}
>+
>+/* Records a public key in the list of previously-successful keys */
>+void
>+auth2_record_userkey(Authctxt *authctxt, struct sshkey *key)
>+{
>+	struct sshkey **tmp;
>+
>+	if (authctxt->nprev_userkeys >= INT_MAX ||
>+	    (tmp = reallocarray(authctxt->prev_userkeys,
>+	    authctxt->nprev_userkeys + 1, sizeof(*tmp))) == NULL)
>+		fatal("%s: reallocarray failed", __func__);
>+	authctxt->prev_userkeys = tmp;
>+	authctxt->prev_userkeys[authctxt->nprev_userkeys] = key;
>+	authctxt->nprev_userkeys++;
>+}
>+
>+/* Checks whether a key has already been used successfully for authentication */
>+int
>+auth2_userkey_already_used(Authctxt *authctxt, struct sshkey *key)
>+{
>+	u_int i;
>+
>+	for (i = 0; i < authctxt->nprev_userkeys; i++) {
>+		if (sshkey_equal_public(key, authctxt->prev_userkeys[i])) {
>+			return 1;
>+		}
>+	}
>+	return 0;
> }
> 
> Authmethod method_pubkey = {
>Index: usr.bin/ssh/monitor.c
>===================================================================
>RCS file: /cvs/src/usr.bin/ssh/monitor.c,v
>retrieving revision 1.135
>diff -u -p -r1.135 monitor.c
>--- usr.bin/ssh/monitor.c	15 Jul 2014 15:54:14 -0000	1.135
>+++ usr.bin/ssh/monitor.c	11 Dec 2014 03:29:14 -0000
>@@ -884,6 +884,7 @@ mm_answer_keyallowed(int sock, Buffer *m
> 		switch (type) {
> 		case MM_USERKEY:
> 			allowed = options.pubkey_authentication &&
>+			    !auth2_userkey_already_used(authctxt, key) &&
> 			    user_key_allowed(authctxt->pw, key);
> 			pubkey_auth_info(authctxt, key, NULL);
> 			auth_method = "publickey";
>@@ -1111,7 +1112,12 @@ mm_answer_keyverify(int sock, Buffer *m)
> 	debug3("%s: key %p signature %s",
> 	    __func__, key, (verified == 1) ? "verified" : "unverified");
> 
>-	key_free(key);
>+	/* If auth was successful then record key to ensure it isn't reused */
>+	if (verified == 1)
>+		auth2_record_userkey(authctxt, key);
>+	else
>+		key_free(key);
>+
> 	free(blob);
> 	free(signature);
> 	free(data);
>Index: usr.bin/ssh/sshd_config.5
>===================================================================
>RCS file: /cvs/src/usr.bin/ssh/sshd_config.5,v
>retrieving revision 1.180
>diff -u -p -r1.180 sshd_config.5
>--- usr.bin/ssh/sshd_config.5	22 Nov 2014 19:21:03 -0000	1.180
>+++ usr.bin/ssh/sshd_config.5	11 Dec 2014 03:29:14 -0000
>@@ -210,6 +210,18 @@ would restrict keyboard interactive auth
> .Dq bsdauth
> device.
> .Pp
>+If the
>+.Dq publickey
>+method is listed more than one,
>+.Xr sshd 8
>+verifies that keys that have been successfully are not reused for subsequent
>+authentications.
>+For example, an
>+.Cm AuthenticationMethods
>+of
>+.Dq publickey,publickey
>+will require successful authentication using two different public keys.
>+.Pp
> This option is only available for SSH protocol 2 and will yield a fatal
> error if enabled if protocol 1 is also enabled.
> Note that each authentication method listed should also be explicitly enabled
>Index: regress/usr.bin/ssh/Makefile
>===================================================================
>RCS file: /cvs/src/regress/usr.bin/ssh/Makefile,v
>retrieving revision 1.70
>diff -u -p -r1.70 Makefile
>--- regress/usr.bin/ssh/Makefile	24 Jun 2014 01:14:17 -0000	1.70
>+++ regress/usr.bin/ssh/Makefile	11 Dec 2014 03:29:14 -0000
>@@ -58,7 +58,8 @@ LTESTS= 	connect \
> 		keys-command \
> 		forward-control \
> 		integrity \
>-		krl
>+		krl \
>+		multipubkey
> 
> INTEROP_TESTS=	putty-transfer putty-ciphers putty-kex conch-ciphers
> #INTEROP_TESTS+=ssh-com ssh-com-client ssh-com-keygen ssh-com-sftp
>Index: regress/usr.bin/ssh/multipubkey.sh
>===================================================================
>RCS file: regress/usr.bin/ssh/multipubkey.sh
>diff -N regress/usr.bin/ssh/multipubkey.sh
>--- /dev/null	1 Jan 1970 00:00:00 -0000
>+++ regress/usr.bin/ssh/multipubkey.sh	11 Dec 2014 03:29:14 -0000
>@@ -0,0 +1,66 @@
>+#	$OpenBSD$
>+#	Placed in the Public Domain.
>+
>+tid="multiple pubkey"
>+
>+rm -f $OBJ/authorized_keys_$USER $OBJ/user_ca_key* $OBJ/user_key*
>+rm -f $OBJ/authorized_principals_$USER $OBJ/cert_user_key*
>+
>+mv $OBJ/sshd_proxy $OBJ/sshd_proxy.orig
>+mv $OBJ/ssh_proxy $OBJ/ssh_proxy.orig
>+
>+# Create a CA key
>+${SSHKEYGEN} -q -N '' -t ed25519  -f $OBJ/user_ca_key ||\
>+	fatal "ssh-keygen failed"
>+
>+# Make some keys and a certificate.
>+${SSHKEYGEN} -q -N '' -t ed25519 -f $OBJ/user_key1 || \
>+	fatal "ssh-keygen failed"
>+${SSHKEYGEN} -q -N '' -t ed25519 -f $OBJ/user_key2 || \
>+	fatal "ssh-keygen failed"
>+${SSHKEYGEN} -q -s $OBJ/user_ca_key -I "regress user key for $USER" \
>+	-z $$ -n ${USER},mekmitasdigoat $OBJ/user_key1 ||
>+		fail "couldn't sign user_key1"
>+# Copy the private key alongside the cert to allow better control of when
>+# it is offered.
>+mv $OBJ/user_key1-cert.pub $OBJ/cert_user_key1.pub
>+cp -p $OBJ/user_key1 $OBJ/cert_user_key1
>+
>+grep -v IdentityFile $OBJ/ssh_proxy.orig > $OBJ/ssh_proxy
>+
>+opts="-oProtocol=2 -F $OBJ/ssh_proxy -oIdentitiesOnly=yes"
>+opts="$opts -i $OBJ/cert_user_key1 -i $OBJ/user_key1 -i $OBJ/user_key2"
>+
>+for privsep in no yes; do
>+	(
>+		grep -v "Protocol"  $OBJ/sshd_proxy.orig
>+		echo "Protocol 2"
>+		echo "UsePrivilegeSeparation $privsep"
>+		echo "AuthenticationMethods publickey,publickey"
>+		echo "TrustedUserCAKeys $OBJ/user_ca_key.pub"
>+		echo "AuthorizedPrincipalsFile $OBJ/authorized_principals_%u"
>+ 	) > $OBJ/sshd_proxy
>+
>+	# Single key should fail.
>+	rm -f $OBJ/authorized_principals_$USER
>+	cat $OBJ/user_key1.pub > $OBJ/authorized_keys_$USER
>+	${SSH} $opts proxy true && fail "ssh succeeded with key"
>+
>+	# Single key with same-public cert should fail.
>+	echo mekmitasdigoat > $OBJ/authorized_principals_$USER
>+	cat $OBJ/user_key1.pub > $OBJ/authorized_keys_$USER
>+	${SSH} $opts proxy true && fail "ssh succeeded with key+cert"
>+
>+	# Multiple plain keys should succeed.
>+	rm -f $OBJ/authorized_principals_$USER
>+	cat $OBJ/user_key1.pub $OBJ/user_key2.pub > \
>+	    $OBJ/authorized_keys_$USER
>+	${SSH} $opts proxy true || fail "ssh failed with multiple keys"
>+	# Cert and different key should succeed
>+
>+	# Key and different-public cert should succeed.
>+	echo mekmitasdigoat > $OBJ/authorized_principals_$USER
>+	cat $OBJ/user_key2.pub > $OBJ/authorized_keys_$USER
>+	${SSH} $opts proxy true || fail "ssh failed with key/cert"
>+done
>+

Actions: View | Diff

Attachments on bug 2323: 2516