Multi-factor authentication order

[NickstaDB]

When connecting to a server that requires multi-factor authentication such as both key and password, the order that those credentials are provided to the server matters. Take the following code, for example:

$ssh->login($username, $key, $password);

This will authenticate successfully if the server's sshd_config contains the following:

AuthenticationMethods publickey,password

However, the same code will fail if the config contains:

AuthenticationMethods password,publickey

This isn't a problem if you know the configuration in advance, but if you don't then this will lead to authentication failures and may result in bans etc where programs like fail2ban are used.

According to RFC4252 section 4, regarding the use of the "none" authentication method: "The main purpose of sending this request is to get the list of supported methods from the server". Section 5.1 states that when the server responds with SSH_MSG_USERAUTH_FAILURE, the response contains a comma-separated list of authentication methods that can continue. With my first sshd_config example shown above, this will initially contain publickey only, then once the key has been accepted by the server it will contain password only, which, once submitted and accepted by the server, will lead to a SSH_MSG_USERAUTH_SUCCESS message.

It looks like phpseclib currently discards the list of authentication methods that can continue, and doesn't take it into account when multiple credentials are passed to the SSH2::login() method.

Furthermore, the documentation for multi-factor SSH authentication states that the following code snippets should be equivalent:

$ssh->login($username, $key, $password);

$ssh->login($username, $key);
$ssh->login($username, $password);

In my tests, the second method was not successful at authenticating where both password and publickey were listed in the AuthenticationMethods setting of sshd_config.

[NickstaDB]

I'm going to do some debugging later and see if I can get to the bottom of why one call to login() with both creds worked, but two calls with one cred each didn't. If I get to the bottom of that then I'll look at implementing a getAcceptedAuthMethods() method that uses the "none" authentication method to get and return accepted authentication methods, that way I can use the response from that to determine what to send first.

[terrafrost]

However, the same code will fail if the config contains:

AuthenticationMethods password,publickey

That's probably because, when password auth fails, phpeclib tries to login with keyboard-interactive auth.

I'll look into making it so that it only conditionally does this.

Furthermore, the documentation for multi-factor SSH authentication states that the following code snippets should be equivalent:

$ssh->login($username, $key, $password);

$ssh->login($username, $key);
$ssh->login($username, $password);

In my tests, the second method was not successful at authenticating where both password and publickey were listed in the AuthenticationMethods setting of sshd_config.

I don't really see how that's possible. From Net/SSH2.php:

    public function login($username, ...$args)
    {
...
        return $this->sublogin($username, ...$args);
    }

    protected function sublogin($username, ...$args)
    {
...
        foreach ($args as $arg) {
            if ($this->login_helper($username, $arg)) {
                return true;
            }
        }
        return false;
    }

So per that if you pass in multiple arguments it literally does exactly what you're saying it isn't doing.

How are you conducting your tests?

According to RFC4252 section 4, regarding the use of the "none" authentication method: "The main purpose of sending this request is to get the list of supported methods from the server". Section 5.1 states that when the server responds with SSH_MSG_USERAUTH_FAILURE, the response contains a comma-separated list of authentication methods that can continue. With my first sshd_config example shown above, this will initially contain publickey only, then once the key has been accepted by the server it will contain password only, which, once submitted and accepted by the server, will lead to a SSH_MSG_USERAUTH_SUCCESS message.

What should phpseclib do if the server says it only supports publickey authentication and yet you send a password? I suppose phpseclib could error out without even trying but I think it's better to try. Could that trigger fail2ban? Sure. But you also prob ought to test your code before you run it. If one botched attempt to connect to an SSH server is going to result in your being banned then you could just as easily get banned because you have multiple private keys in your ~/.ssh/id_rsa file.

If phpseclib adopted an "intelligent" login method wherein it possibly re-ordered the login parameters based on the response it back from the server (which isn't the same thing as conditionally trying keyboard-interactive auth) then people's ability to manually tune the login process as demand dictated would be impacted. And as for why you might want such manual control: maybe some SSH servers need it. SSH servers aren't always implemented exactly to spec. #1631 (comment) is the most recent example of this.

I'll look at implementing a getAcceptedAuthMethods() method that uses the "none" authentication method to get and return accepted authentication methods

That's not a bad idea, altho, as you noted, this could change on multiple successive runs / logins.

[terrafrost]

That's probably because, when password auth fails, phpeclib tries to login with keyboard-interactive auth.

Well looks like I misspoke on that. It already does do it conditionally it looks like:

if (!$partial_success && in_array('keyboard-interactive', $auth_methods)) {

I just tried it out myself with AuthenticationMethods password,publickey and it seems to work.

Can you post your SSH logs when using AuthenticationMethods password,publickey? You can get them by doing define('NET_SSH2_LOGGING', 2) at the top and echo $ssh->getLog() after the login attempt fails.

Thanks!

[NickstaDB]

I've managed to do some more debugging on this one now and get a better handle on what's going on. I might have a workable solution which I'll post in a second comment, but first I want to address a few things:

Furthermore, the documentation for multi-factor SSH authentication states that the following code snippets should be equivalent:

$ssh->login($username, $key, $password);

$ssh->login($username, $key);
$ssh->login($username, $password);

The difference between these two is an extra call to $this->sublogin($username) (i.e. "none" authentication method). Investigating further, the server in question has MaxAuthTries 1 in sshd_config, and this second attempt at authenticating with the "none" method is seen as a failed authentication by OpenSSH which causes the connection to be terminated and may lead to bans etc.

What should phpseclib do if the server says it only supports publickey authentication and yet you send a password? I suppose phpseclib could error out without even trying but I think it's better to try. Could that trigger fail2ban? Sure. But you also prob ought to test your code before you run it. If one botched attempt to connect to an SSH server is going to result in your being banned then you could just as easily get banned because you have multiple private keys in your ~/.ssh/id_rsa file.

Unfortunately I don't control the servers that this application connects to, or the security controls around them. The multiple key pairs issue you mention is irrelevant here. Yes it's possible to manually work around these issues but it's far from ideal, particularly when the protocol provides the features to avoid having to have those manual workarounds and to avoid authentication failures.

If phpseclib adopted an "intelligent" login method wherein it possibly re-ordered the login parameters based on the response it back from the server (which isn't the same thing as conditionally trying keyboard-interactive auth) then people's ability to manually tune the login process as demand dictated would be impacted. And as for why you might want such manual control: maybe some SSH servers need it. SSH servers aren't always implemented exactly to spec. #1631 (comment) is the most recent example of this.

Yeah I appreciate that it may be desirable to allow manual control over the process, the library just doesn't currently support a feature of the protocol to enable that in this instance.

I just tried it out myself with AuthenticationMethods password,publickey and it seems to work.

Two things here. With AuthenticationMethods password,publickey and $ssh->login($username, $key, $password), if the server is configured with MaxAuthTries 1 then this fails with a phpseclib3\Exception\ConnectionClosedException. If MaxAuthTries is greater than 1 then it doesn't, but $ssh->execute('id') doesn't return anything after that login, whereas if I call $ssh->login($username, $password, $key) then I can execute commands over the connection.

[NickstaDB]

Now for a potential solution.

(1) Firstly, I propose adding a property to the SSH2 class which contains the authentication methods that can continue, as returned by the server in NET_SSH2_MSG_USERAUTH_FAILURE responses.

(2) Next, update SSH2::login() so that a login attempt with the "none" method is only automatically attempted if the above property has not been initialised. This prevents an authentication failure from occurring with multiple calls to SSH2::login(), but still allows library users to manually perform authentication with the "none" method if they wish to.

(3) Update the remaining login handling code to handle the NET_SSH2_MSG_USERAUTH_FAILURE response by parsing the returned authentication methods and overwriting the above property with those methods.

(4) Implement a method as I previously suggested, that only attempts authentication with the "none" method, with the purpose of setting the above mentioned property.

With this, I can use (4) to get the first supported authentication method, then call SSH2::login() with the appropriate credentials based on the value of (1). If more credentials are required then (3) means that call to SSH2::login() will read a NET_SSH2_MSG_USERAUTH_FAILURE from the server and update the property (1) with the next accepted authentication method. I can then use the property (1) to determine which credential to send next, then call SSH2::login() with the appropriate credentials where (2) will ensure that another "none" authentication attempt doesn't occur and therefore no failed authentication attempt.

It shouldn't be too much effort to implement this so I'm going to go ahead and do so but it would be good to get your thoughts. Happy to do a PR if all goes well and this sounds like a workable solution.

[NickstaDB]

Implemented, tested, and PR submitted. With this I'm able to use the following code to login using key + password, without authentication failures, regardless of the ordering of AuthenticationMethods and the value of MaxAuthTries in sshd_config.

function testLogin($ip, $username, $password, $key) {
  try {
    $ssh = new SSH2($ip, 22, 15);
    
    //Get auth methods that can continue
    $ssh->login($username);
    
    //Send first credential
    if(in_array('publickey', $ssh->getAuthMethodsToContinue()) === true) { $ssh->login($username, $key); }
    elseif(in_array('password', $ssh->getAuthMethodsToContinue()) === true) { $ssh->login($username, $password); }
    
    //Send second credential
    if(in_array('publickey', $ssh->getAuthMethodsToContinue()) === true) { $ssh->login($username, $key); }
    elseif(in_array('password', $ssh->getAuthMethodsToContinue()) === true) { $ssh->login($username, $password); }
    
    //Verify and disconnect
    echo 'Connected, id: ' . $ssh->exec('id');
    $ssh->disconnect();
  } catch(Exception $ex) { echo 'Login failed.'; }
}

[terrafrost]

The difference between these two is an extra call to $this->sublogin($username) (i.e. "none" authentication method). Investigating further, the server in question has MaxAuthTries 1 in sshd_config, and this second attempt at authenticating with the "none" method is seen as a failed authentication by OpenSSH which causes the connection to be terminated and may lead to bans etc.

That makes sense! Good find!

Anyway, I like your changes. Would you be up for applying them to the 1.0 branch too? unpackSSH2 doesn't exist in either 1.0 or 2.0 so you'll need to adapt it to work with that.

I've also thought about it some more and I'm warming up to the idea of an "intelligent" login system. If it causes issues I can always make new release with those changes reverted!

Thanks!

[NickstaDB]

Anyway, I like your changes. Would you be up for applying them to the 1.0 branch too? unpackSSH2 doesn't exist in either 1.0 or 2.0 so you'll need to adapt it to work with that.

I make no promises but I'll take a look later on. If it's more or less a case of making up for no unpackSSH2 method then it should be pretty straightforward to implement.

I've also thought about it some more and I'm warming up to the idea of an "intelligent" login system. If it causes issues I can always make new release with those changes reverted!

Sounds good! From what I've seen of the protocol this does seem to make more sense when multiple credentials are passed to login(). Anyone who wants manual control can still call login() multiple times as needed.

[NickstaDB]

Implemented in 1.0 and 2.0 branches. Same test script works in both cases, again regardless of the AuthorizationMethods and MaxAuthTries settings.

[terrafrost]

I implemented the smart login feature in 29c8591 . This feature is enabled by default and can be disabled by calling $ssh->disableSmartMFA().