Authenticate against key files before AuthorizedKeysCommand

[Andrei Gherzan]

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