rcolebaugh / rpms / openssh

Forked from rpms/openssh 2 years ago
Clone
f94360
Index: b/session.c
f94360
===================================================================
f94360
--- b.orig/session.c
d1c05f
+++ b/session.c
d1c05f
@@ -98,6 +98,7 @@
d1c05f
 #include "atomicio.h"
d1c05f
 #include "slog.h"
d1c05f
 
d1c05f
+#define SSH_MAX_PUBKEY_BYTES 16384
d1c05f
 
d1c05f
 #if defined(KRB5) && defined(USE_AFS)
d1c05f
 #include <kafs.h>
f94360
@@ -1054,11 +1055,18 @@ copy_environment(char **source, char ***
d1c05f
 static char **
d1c05f
 do_setup_env(struct ssh *ssh, Session *s, const char *shell)
d1c05f
 {
d1c05f
-	char buf[256];
d1c05f
+	char buf[SSH_MAX_PUBKEY_BYTES];
d1c05f
+	char *pbuf = &buf[0];
d1c05f
 	size_t n;
d1c05f
 	u_int i, envsize;
d1c05f
 	char *ocp, *cp, *value, **env, *laddr;
d1c05f
 	struct passwd *pw = s->pw;
d1c05f
+	Authctxt *authctxt = s->authctxt;
d1c05f
+	struct sshkey *key;
d1c05f
+	size_t len = 0;
d1c05f
+	ssize_t total = 0;
d1c05f
+	struct sshkey_cert *cert;
d1c05f
+
d1c05f
 #if !defined (HAVE_LOGIN_CAP) && !defined (HAVE_CYGWIN)
d1c05f
 	char *path = NULL;
d1c05f
 #endif
f94360
@@ -1255,9 +1263,57 @@ do_setup_env(struct ssh *ssh, Session *s
d1c05f
 		child_set_env(&env, &envsize, "SSH_USER_AUTH", auth_info_file);
d1c05f
 	if (s->ttyfd != -1)
d1c05f
 		child_set_env(&env, &envsize, "SSH_TTY", s->tty);
d1c05f
-	if (original_command)
d1c05f
+	if (original_command) {
d1c05f
 		child_set_env(&env, &envsize, "SSH_ORIGINAL_COMMAND",
d1c05f
 		    original_command);
d1c05f
+		/*
d1c05f
+		* Set SSH_CERT_PRINCIPALS to be the principals on the ssh certificate.
d1c05f
+		* Only do so when a force command is present to prevent the client
d1c05f
+		* from changing the value of SSH_CERT_PRINCIPALS. For example, when a
d1c05f
+		* client is given shell access, the client can easily change the
d1c05f
+		* value of an environment variable by running, e.g.,
d1c05f
+		* ssh user@host.address 'SSH_CERT_PRINCIPALS=attacker env'
d1c05f
+		*/
d1c05f
+
d1c05f
+		if (authctxt->nprev_keys > 0) {
d1c05f
+			key = authctxt->prev_keys[authctxt->nprev_keys-1];
d1c05f
+			/* If a user was authorized by a certificate, set SSH_CERT_PRINCIPALS */
d1c05f
+			if (sshkey_is_cert(key)) {
d1c05f
+				cert = key->cert;
d1c05f
+
d1c05f
+				for (i = 0; i < cert->nprincipals - 1; ++i) {
d1c05f
+					/*
d1c05f
+					* total: bytes written to buf so far
d1c05f
+					* 2: one for comma and one for '\0' to be added by snprintf
d1c05f
+					* We stop at the first principal overflowing buf.
d1c05f
+					*/
d1c05f
+					if (total + strlen(cert->principals[i]) + 2 > SSH_MAX_PUBKEY_BYTES)
d1c05f
+						break;
d1c05f
+
d1c05f
+					len = snprintf(pbuf, SSH_MAX_PUBKEY_BYTES-total, "%s,",
d1c05f
+					    cert->principals[i]);
d1c05f
+					/* pbuf advances by len, the '\0' at the end will be overwritten */
d1c05f
+					pbuf += len;
d1c05f
+					total += len;
d1c05f
+				}
d1c05f
+
d1c05f
+				if (total + strlen(cert->principals[i]) + 1 <= SSH_MAX_PUBKEY_BYTES) {
d1c05f
+					len = snprintf(pbuf, SSH_MAX_PUBKEY_BYTES-total, "%s",
d1c05f
+					    cert->principals[i]);
d1c05f
+					total += len;
d1c05f
+				} else if (total > 0)
d1c05f
+					/*
d1c05f
+					* If we hit the overflow condition, remove the trailing comma.
d1c05f
+					* We only do so if the overflowing principal is not the first one on the
d1c05f
+					* certificate so that there is at least one principal in buf
d1c05f
+					*/
d1c05f
+					buf[total-1] = '\0';
d1c05f
+
d1c05f
+				if (total > 0)
d1c05f
+					child_set_env(&env, &envsize, "SSH_CERT_PRINCIPALS", buf);
d1c05f
+			}
d1c05f
+		}
d1c05f
+	}
d1c05f
 
d1c05f
 	if (debug_flag) {
d1c05f
 		/* dump the environment */
f94360
Index: b/regress/cert-princ-env.sh
f94360
===================================================================
d1c05f
--- /dev/null
d1c05f
+++ b/regress/cert-princ-env.sh
d1c05f
@@ -0,0 +1,129 @@
d1c05f
+tid="cert principal env"
d1c05f
+
d1c05f
+# change to ecdsa
d1c05f
+CERT_ID="$USER"
d1c05f
+AUTH_PRINC_FILE="$OBJ/auth_principals"
d1c05f
+CA_FILE="$OBJ/ca-rsa"
d1c05f
+IDENTITY_FILE="$OBJ/$USER-rsa"
d1c05f
+SSH_MAX_PUBKEY_BYTES=16384
d1c05f
+
d1c05f
+cat << EOF >> $OBJ/sshd_config
d1c05f
+TrustedUserCAKeys $CA_FILE.pub
d1c05f
+Protocol 2
d1c05f
+PubkeyAuthentication yes
d1c05f
+AuthenticationMethods publickey
d1c05f
+AuthorizedPrincipalsFile $AUTH_PRINC_FILE
d1c05f
+ForceCommand=/bin/env
d1c05f
+EOF
d1c05f
+
d1c05f
+cleanup() {
d1c05f
+	rm -f $CA_FILE{.pub,}
d1c05f
+	rm -f $IDENTITY_FILE{-cert.pub,.pub,}
d1c05f
+	rm -f $AUTH_PRINC_FILE
d1c05f
+}
d1c05f
+
d1c05f
+make_keys_and_certs() {
d1c05f
+	rm -f $CA_FILE{.pub,}
d1c05f
+	rm -f $IDENTITY_FILE{-cert.pub,.pub,}
d1c05f
+
d1c05f
+  local princs=$1
d1c05f
+
d1c05f
+	${SSHKEYGEN} -q -t rsa -C '' -N '' -f $CA_FILE ||
d1c05f
+	    fatal 'Could not create CA key'
d1c05f
+
d1c05f
+	${SSHKEYGEN} -q -t rsa -C '' -N '' -f $IDENTITY_FILE ||
d1c05f
+	    fatal 'Could not create keypair'
d1c05f
+
d1c05f
+	${SSHKEYGEN} -q -s $CA_FILE -I $CERT_ID -n "$princs" -z "42" "$IDENTITY_FILE.pub" ||
d1c05f
+	    fatal "Could not create SSH cert"
d1c05f
+}
d1c05f
+
d1c05f
+test_with_expected_principals() {
d1c05f
+	local princs=$1
d1c05f
+
d1c05f
+	out=$(${SSH} -E thlog -F $OBJ/ssh_config -i "$IDENTITY_FILE" somehost false) ||
d1c05f
+	    fatal "SSH failed"
d1c05f
+
d1c05f
+	echo "$out" | grep -q "SSH_CERT_PRINCIPALS=$princs$" ||
d1c05f
+	    fatal "SSH_CERT_PRINCIPALS has incorrect value"
d1c05f
+}
d1c05f
+
d1c05f
+test_with_no_expected_principals() {
d1c05f
+	local princs=$1
d1c05f
+
d1c05f
+	out=$(${SSH} -E thlog -F $OBJ/ssh_config -i "$IDENTITY_FILE" somehost false) ||
d1c05f
+	    fatal "SSH failed"
d1c05f
+
d1c05f
+	echo "$out" | grep -vq "SSH_CERT_PRINCIPALS" ||
d1c05f
+	    fatal "SSH_CERT_PRINCIPALS env should not be set"
d1c05f
+
d1c05f
+	echo "$out" | grep -vq "SSH_CERT_PRINCIPALS=$princs" ||
d1c05f
+	    fatal "SSH_CERT_PRINCIPALS has incorrect value"
d1c05f
+}
d1c05f
+
d1c05f
+
d1c05f
+echo 'a' > $AUTH_PRINC_FILE
d1c05f
+start_sshd
d1c05f
+
d1c05f
+principals="a,b,c,d"
d1c05f
+make_keys_and_certs "$principals"
d1c05f
+test_with_expected_principals "$principals"
d1c05f
+
d1c05f
+big_princ=$(cat /dev/urandom | tr -dc 'a-zA-Z0-9' | fold -w 16381 | head -n 1)
d1c05f
+make_keys_and_certs "a,$big_princ"
d1c05f
+test_with_expected_principals "a,$big_princ"
d1c05f
+
d1c05f
+# No room for two principals
d1c05f
+big_princ=$(cat /dev/urandom | tr -dc 'a-zA-Z0-9' | fold -w 16382 | head -n 1)
d1c05f
+make_keys_and_certs "a,$big_princ"
d1c05f
+test_with_expected_principals "a"
d1c05f
+
d1c05f
+make_keys_and_certs "$big_princ,a"
d1c05f
+test_with_expected_principals "$big_princ"
d1c05f
+
d1c05f
+big_princ=$(cat /dev/urandom | tr -dc 'a-zA-Z0-9' | fold -w 16384 | head -n 1)
d1c05f
+make_keys_and_certs "a,$big_princ"
d1c05f
+test_with_expected_principals "a"
d1c05f
+
d1c05f
+# principal too big for buffer
d1c05f
+big_princ=$(cat /dev/urandom | tr -dc 'a-zA-Z0-9' | fold -w $SSH_MAX_PUBKEY_BYTES | head -n 1)
d1c05f
+make_keys_and_certs "$big_princ"
d1c05f
+test_with_no_expected_principals "$big_princ"
d1c05f
+
d1c05f
+# no matching principals in certificate and auth princ file
d1c05f
+principals="b,c,d"
d1c05f
+make_keys_and_certs "$principals"
d1c05f
+test_with_no_expected_principals "$principals"
d1c05f
+
d1c05f
+stop_sshd
d1c05f
+
d1c05f
+cat << EOF >> $OBJ/sshd_config
d1c05f
+TrustedUserCAKeys $CA_FILE.pub
d1c05f
+Protocol 2
d1c05f
+PubkeyAuthentication yes
d1c05f
+AuthenticationMethods publickey
d1c05f
+AuthorizedPrincipalsFile $AUTH_PRINC_FILE
d1c05f
+EOF
d1c05f
+
d1c05f
+start_sshd
d1c05f
+
d1c05f
+# no force command, no princpals
d1c05f
+principals="a,b,c,d"
d1c05f
+make_keys_and_certs "$principals"
d1c05f
+test_with_no_expected_principals "$principals"
d1c05f
+
d1c05f
+stop_sshd
d1c05f
+
d1c05f
+cat << EOF >> $OBJ/sshd_config
d1c05f
+Protocol 2
d1c05f
+PubkeyAuthentication yes
d1c05f
+AuthenticationMethods publickey
d1c05f
+AuthorizedPrincipalsFile $AUTH_PRINC_FILE
d1c05f
+EOF
d1c05f
+
d1c05f
+start_sshd
d1c05f
+
d1c05f
+# No TrustedUserCAKeys causes pubkey auth, no principals
d1c05f
+principals="a,b,c,d"
d1c05f
+make_keys_and_certs "$principals"
d1c05f
+test_with_no_expected_principals "$principals"