openssh unix dev - May 2019 - Authenticate against key files before AuthorizedKeysCommand

Hello,

Currently OpenSSH has a fixed order on how the key authenticates the
user: at first it tries to authenticate against TrustedUserCAKeys,
afterwards it does it against the output keys from the
AuthorizedKeysCommand and finally against the files as set in
AuthorizedKeysFile. I have an use-case where this order is not ideal.
This is because in my case the command fetches keys from the cloud which
due to connectivity issues (and whatnot) might timeout and the fallback
to the auth keys file will only happen after this timeout. In my case,
checking it first and only fallback to the cloud keys would help. This
would make the cloud keys being the fallback which even if it timeouts
it's fine because there is no other fallback afterwards (existing public
keys would have been tried).

Do you think such a feature would make sense? If yes, how would you
recommend going about it? I was thinking of having a priority
configuration variable of some sort that would decide the order I'm
mentioning above or even a simple configuration flag like
AuthorizedKeysCommandBeforeFile (default to true). I'm willing to send
patch if this is considered upstreamable.

Regards,

-- 
Andrei Gherzan
gpg: rsa4096/D4D94F67AD0E9640 | t: @agherzan

On Mon, 20 May 2019, Andrei Gherzan wrote:
> Hello,
> 
> Currently OpenSSH has a fixed order on how the key authenticates the
> user: at first it tries to authenticate against TrustedUserCAKeys,
> afterwards it does it against the output keys from the
> AuthorizedKeysCommand and finally against the files as set in
> AuthorizedKeysFile. I have an use-case where this order is not ideal.
> This is because in my case the command fetches keys from the cloud which
> due to connectivity issues (and whatnot) might timeout and the fallback
> to the auth keys file will only happen after this timeout. In my case,
> checking it first and only fallback to the cloud keys would help. This
> would make the cloud keys being the fallback which even if it timeouts
> it's fine because there is no other fallback afterwards (existing
public
> keys would have been tried).
> 
> Do you think such a feature would make sense? If yes, how would you
> recommend going about it? I was thinking of having a priority
> configuration variable of some sort that would decide the order I'm
> mentioning above or even a simple configuration flag like
> AuthorizedKeysCommandBeforeFile (default to true). I'm willing to send
> patch if this is considered upstreamable.
Maybe it makes sense to just prefer the static files to the command under
all circumstances? This is already what we do for authorized_principals
and IMO it makes the most sense.

diff --git a/auth2-pubkey.c b/auth2-pubkey.c
index ec1cdb9..cdf20da 100644
--- a/auth2-pubkey.c
+++ b/auth2-pubkey.c
@@ -1023,16 +1023,6 @@ user_key_allowed(struct ssh *ssh, struct passwd *pw,
struct sshkey *key,
 	    auth_key_is_revoked(key->cert->signature_key))
 		return 0;
 
-	if ((success = user_cert_trusted_ca(ssh, pw, key, &opts)) != 0)
-		goto out;
-	sshauthopt_free(opts);
-	opts = NULL;
-
-	if ((success = user_key_command_allowed2(ssh, pw, key, &opts)) != 0)
-		goto out;
-	sshauthopt_free(opts);
-	opts = NULL;
-
 	for (i = 0; !success && i < options.num_authkeys_files; i++) {
 		if (strcasecmp(options.authorized_keys_files[i], "none") == 0)
 			continue;
@@ -1042,6 +1032,16 @@ user_key_allowed(struct ssh *ssh, struct passwd *pw,
struct sshkey *key,
 		free(file);
 	}
 
+	if ((success = user_cert_trusted_ca(ssh, pw, key, &opts)) != 0)
+		goto out;
+	sshauthopt_free(opts);
+	opts = NULL;
+
+	if ((success = user_key_command_allowed2(ssh, pw, key, &opts)) != 0)
+		goto out;
+	sshauthopt_free(opts);
+	opts = NULL;
+
  out:
 	if (success && authoptsp != NULL) {
 		*authoptsp = opts;

Hi,

On 21/05/2019 02.43, Damien Miller wrote:
> On Mon, 20 May 2019, Andrei Gherzan wrote:
>
>> Hello,
>>
>> Currently OpenSSH has a fixed order on how the key authenticates the
>> user: at first it tries to authenticate against TrustedUserCAKeys,
>> afterwards it does it against the output keys from the
>> AuthorizedKeysCommand and finally against the files as set in
>> AuthorizedKeysFile. I have an use-case where this order is not ideal.
>> This is because in my case the command fetches keys from the cloud
which
>> due to connectivity issues (and whatnot) might timeout and the fallback
>> to the auth keys file will only happen after this timeout. In my case,
>> checking it first and only fallback to the cloud keys would help. This
>> would make the cloud keys being the fallback which even if it timeouts
>> it's fine because there is no other fallback afterwards (existing
public
>> keys would have been tried).
>>
>> Do you think such a feature would make sense? If yes, how would you
>> recommend going about it? I was thinking of having a priority
>> configuration variable of some sort that would decide the order I'm
>> mentioning above or even a simple configuration flag like
>> AuthorizedKeysCommandBeforeFile (default to true). I'm willing to
send
>> patch if this is considered upstreamable.
> Maybe it makes sense to just prefer the static files to the command under
> all circumstances? This is already what we do for authorized_principals
> and IMO it makes the most sense.
This was my initial thought but I was reluctant in proposing it because
at least it changes the expected behavior which might, in turn, break
people's other use-cases.. Also, I think the code assumes "fall
through"
for freeing opts so if we move the key files block we will need to do it
too. But in anyway, the question is, would this be a reasonable change?
>
> diff --git a/auth2-pubkey.c b/auth2-pubkey.c
> index ec1cdb9..cdf20da 100644
> --- a/auth2-pubkey.c
> +++ b/auth2-pubkey.c
> @@ -1023,16 +1023,6 @@ user_key_allowed(struct ssh *ssh, struct passwd *pw,
struct sshkey *key,
>  	    auth_key_is_revoked(key->cert->signature_key))
>  		return 0;
>  
> -	if ((success = user_cert_trusted_ca(ssh, pw, key, &opts)) != 0)
> -		goto out;
> -	sshauthopt_free(opts);
> -	opts = NULL;
> -
> -	if ((success = user_key_command_allowed2(ssh, pw, key, &opts)) != 0)
> -		goto out;
> -	sshauthopt_free(opts);
> -	opts = NULL;
> -
>  	for (i = 0; !success && i < options.num_authkeys_files; i++) {
>  		if (strcasecmp(options.authorized_keys_files[i], "none") == 0)
>  			continue;
> @@ -1042,6 +1032,16 @@ user_key_allowed(struct ssh *ssh, struct passwd *pw,
struct sshkey *key,
>  		free(file);
>  	}
>  
> +	if ((success = user_cert_trusted_ca(ssh, pw, key, &opts)) != 0)
> +		goto out;
> +	sshauthopt_free(opts);
> +	opts = NULL;
> +
> +	if ((success = user_key_command_allowed2(ssh, pw, key, &opts)) != 0)
> +		goto out;
> +	sshauthopt_free(opts);
> +	opts = NULL;
> +
>   out:
>  	if (success && authoptsp != NULL) {
>  		*authoptsp = opts;
-- 
Andrei Gherzan
gpg: rsa4096/D4D94F67AD0E9640 | t: @agherzan

On 21/05/2019 02.43, Damien Miller wrote:
> On Mon, 20 May 2019, Andrei Gherzan wrote:
>
>> Hello,
>>
>> Currently OpenSSH has a fixed order on how the key authenticates the
>> user: at first it tries to authenticate against TrustedUserCAKeys,
>> afterwards it does it against the output keys from the
>> AuthorizedKeysCommand and finally against the files as set in
>> AuthorizedKeysFile. I have an use-case where this order is not ideal.
>> This is because in my case the command fetches keys from the cloud
which
>> due to connectivity issues (and whatnot) might timeout and the fallback
>> to the auth keys file will only happen after this timeout. In my case,
>> checking it first and only fallback to the cloud keys would help. This
>> would make the cloud keys being the fallback which even if it timeouts
>> it's fine because there is no other fallback afterwards (existing
public
>> keys would have been tried).
>>
>> Do you think such a feature would make sense? If yes, how would you
>> recommend going about it? I was thinking of having a priority
>> configuration variable of some sort that would decide the order I'm
>> mentioning above or even a simple configuration flag like
>> AuthorizedKeysCommandBeforeFile (default to true). I'm willing to
send
>> patch if this is considered upstreamable.
> Maybe it makes sense to just prefer the static files to the command under
> all circumstances? This is already what we do for authorized_principals
> and IMO it makes the most sense.
>
> diff --git a/auth2-pubkey.c b/auth2-pubkey.c
> index ec1cdb9..cdf20da 100644
> --- a/auth2-pubkey.c
> +++ b/auth2-pubkey.c
> @@ -1023,16 +1023,6 @@ user_key_allowed(struct ssh *ssh, struct passwd *pw,
struct sshkey *key,
>  	    auth_key_is_revoked(key->cert->signature_key))
>  		return 0;
>  
> -	if ((success = user_cert_trusted_ca(ssh, pw, key, &opts)) != 0)
> -		goto out;
> -	sshauthopt_free(opts);
> -	opts = NULL;
> -
> -	if ((success = user_key_command_allowed2(ssh, pw, key, &opts)) != 0)
> -		goto out;
> -	sshauthopt_free(opts);
> -	opts = NULL;
> -
>  	for (i = 0; !success && i < options.num_authkeys_files; i++) {
>  		if (strcasecmp(options.authorized_keys_files[i], "none") == 0)
>  			continue;
> @@ -1042,6 +1032,16 @@ user_key_allowed(struct ssh *ssh, struct passwd *pw,
struct sshkey *key,
>  		free(file);
>  	}
>  
> +	if ((success = user_cert_trusted_ca(ssh, pw, key, &opts)) != 0)
> +		goto out;
> +	sshauthopt_free(opts);
> +	opts = NULL;
> +
> +	if ((success = user_key_command_allowed2(ssh, pw, key, &opts)) != 0)
> +		goto out;
> +	sshauthopt_free(opts);
> +	opts = NULL;
> +
>   out:
>  	if (success && authoptsp != NULL) {
>  		*authoptsp = opts;
So would a patch like this be accepted? Does it sound sane to switch
this default order?

-- 
Andrei Gherzan
gpg: rsa4096/D4D94F67AD0E9640 | t: @agherzan

On Tue, 21 May 2019, Damien Miller wrote:
> On Mon, 20 May 2019, Andrei Gherzan wrote:
> 
> > Do you think such a feature would make sense? If yes, how would you
> > recommend going about it? I was thinking of having a priority
> > configuration variable of some sort that would decide the order
I'm
> > mentioning above or even a simple configuration flag like
> > AuthorizedKeysCommandBeforeFile (default to true). I'm willing to
send
> > patch if this is considered upstreamable.
> 
> Maybe it makes sense to just prefer the static files to the command under
> all circumstances? This is already what we do for authorized_principals
> and IMO it makes the most sense.
This has just been comitted and will be in OpenSSH 8.1 - thanks

-d