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