tdeioslave/sftp: save/restore seqNr for multi-factor auth

In case the server is set up for multi-factor authentication we could
be have to query several things from the user like password, a key
passphrase, their mother's maiden name etc. It doesn't make a big
difference during an initial connection, but it butchers the
reconnection process: it can retrieve the answer of the user to the
first question (e.g. their password), but it fails to retrieve the
second one (e.g. the key passphrase). So the user would be forced to
reenter the answer for the second question upon each reconnection.

The reason for this is the passwdserver's desig (see DESIGN [1]):
Each query for AuthInfo with the openPassDlg() has an secNr number
associated with it. If it's smaller than the one of the one stored for
the privious request, than the one from the cache will be returned
automagically, if it's bigger the dialog will be prompted to the user.
Each call to openPassDlg() advances s_seqNr to the last value reported
by the passwdserver. So the first call will return the cached value and
subsequent calls will actually display the dialog to the user (assuming
authentication with the cached data failed).

But in case of multi-factor auth we have to query user for several
independent values. And we want to try to retrieve each one of those
from the cache. So we have to get a bit hacky and manually manipulate
the SlaveBase::s_seqNr value.

[1]: https://mirror.git.trinitydesktop.org/gitea/TDE/tdelibs/src/branch/master/tdeio/kpasswdserver/DESIGN

Signed-off-by: Alexander Golubev <[email protected]>

2 files changed, 28 insertions, 6 deletions

diff --git a/tdeioslave/sftp/tdeio_sftp.cpp b/tdeioslave/sftp/tdeio_sftp.cpp
index 8009f12f4..4896bf586 100644
--- a/tdeioslave/sftp/tdeio_sftp.cpp
+++ b/tdeioslave/sftp/tdeio_sftp.cpp
@@ -250,7 +250,9 @@ int sftpProtocol::auth_callback(const char *prompt, char *buf, size_t len,
 
   bool firstTry = !mPubKeyAuthData.attemptedKeys.contains(keyFile);
 
-  if (!firstTry) {
+  if (firstTry) {
+    SlaveBase::s_seqNr = mPubKeyAuthData.current_seqNr;
+  } else {
     errMsg = i18n("Incorrect or invalid passphrase.").append('\n');
   }
 
@@ -298,12 +300,14 @@ void sftpProtocol::log_callback(ssh_session session, int priority,
 }
 
 int sftpProtocol::authenticatePublicKey(){
+  kdDebug(TDEIO_SFTP_DB) << "Trying to authenticate with public key" << endl;
+
   // First let's do some cleanup
+  mPubKeyAuthData.attemptedKeys.clear();
+  mPubKeyAuthData.current_seqNr = SlaveBase::s_seqNr;
   mPubKeyAuthData.wasCalled = 0;
   mPubKeyAuthData.wasCanceled = 0;
-  mPubKeyAuthData.attemptedKeys.clear();
 
-  kdDebug(TDEIO_SFTP_DB) << "Trying to authenticate with public key" << endl;
   int rc;
 
   while (1) {
@@ -323,6 +327,7 @@ int sftpProtocol::authenticatePublicKey(){
         break;
       } else {
         kdDebug(TDEIO_SFTP_DB) << "User entered wrong passphrase for the key" << endl;
+        mPubKeyAuthData.current_seqNr = SlaveBase::s_seqNr;
         // Try it again
       }
     } else {
@@ -335,15 +340,18 @@ int sftpProtocol::authenticatePublicKey(){
 }
 
 int sftpProtocol::authenticateKeyboardInteractive(bool noPaswordQuery) {
-  int rc = SSH_AUTH_ERROR;
-
   kdDebug(TDEIO_SFTP_DB) << "Entering keyboard interactive function" << endl;
 
+  int rc = SSH_AUTH_ERROR;
+
   bool retryDenied = false; // a flag to avoid infinite looping
 
   TQString cachablePassword;
   PasswordPurger cachePurger(cachablePassword);
 
+  // Different prompts during a single pass should be queried with the same s_seqNr value
+  long current_seqNr = SlaveBase::s_seqNr;
+
   while (1) {
     int n = 0;
     int i = 0;
@@ -353,6 +361,8 @@ int sftpProtocol::authenticateKeyboardInteractive(bool noPaswordQuery) {
     if (rc == SSH_AUTH_DENIED) { // do nothing
       kdDebug(TDEIO_SFTP_DB) << "kb-interactive auth was denied; retrying again" << endl;
       if (retryDenied) {
+        // If we were denied update the s_seqNr
+        current_seqNr = SlaveBase::s_seqNr;
         continue;
       } else {
         break;
@@ -384,6 +394,9 @@ int sftpProtocol::authenticateKeyboardInteractive(bool noPaswordQuery) {
       TQString answer;
       TQString errMsg;
 
+      // restore the s_seqNr so it would be the same for all the prompts
+      SlaveBase::s_seqNr = current_seqNr;
+
       prompt = TQString::fromUtf8(ssh_userauth_kbdint_getprompt(mSession, i, &echo));
       kdDebug(TDEIO_SFTP_DB) << "prompt=" << prompt << " echo=" << TQString::number(echo) << endl;
 
@@ -1096,7 +1109,6 @@ connection_restart:
     return;
   }
 
-
   // Preinit the list of supported auth methods
   static const auto authMethodsNormal= [](){
     std::vector<std::unique_ptr<SSHAuthMethod>> rv;
@@ -1113,6 +1125,12 @@ connection_restart:
 
   int attemptedMethods = 0;
 
+  // Backup of the value of the SlaveBase::s_seqNr. This is used to query different data values
+  // with openPassDlg() with the same seqNr. Otherwise it will result in the prompting of the pass
+  // dialog to the user in cases the values should be recovered from the cache.
+  // This is a bit hacky but necessary
+  long current_seqNr = SlaveBase::s_seqNr;
+
   while (rc != SSH_AUTH_SUCCESS) {
     // Note this loop can rerun in case of multistage ssh authentication e.g. "password,publickey"
     // which will require user to provide a valid password at first and then a valid public key.
@@ -1120,6 +1138,8 @@ connection_restart:
     bool wasCanceled = false;
     int availableMethodes = ssh_auth_list(mSession);
 
+    SlaveBase::s_seqNr = current_seqNr;
+
     if (!availableMethodes) {
       // Technically libssh docs suggest that the server merely MAY send auth methods, but it's
       // highly unclear what we should do in such case and it looks like openssh doesn't have an
diff --git a/tdeioslave/sftp/tdeio_sftp.h b/tdeioslave/sftp/tdeio_sftp.h
index 6a33dc8a0..84e3e47c3 100644
--- a/tdeioslave/sftp/tdeio_sftp.h
+++ b/tdeioslave/sftp/tdeio_sftp.h
@@ -160,6 +160,8 @@ private: // Private variables
      *        so no fancy containers here
      */
     TQStringList attemptedKeys;
+    /** A backup for SlaveBase::s_seqNr to pass the same value to prompts for different keys */
+    long current_seqNr;
     /** true if callback was called */
     bool wasCalled;
     /** true if user canceled password entry dialog */