From dc1cc88a46a6ad3ff98c22861b3c93fcc9168dfd Mon Sep 17 00:00:00 2001 From: Nicola Murino Date: Sat, 4 Dec 2021 15:14:44 +0100 Subject: [PATCH] keyboard interactive hooks: allow to validate passcode --- dataprovider/dataprovider.go | 31 ++++- docs/keyboard-interactive.md | 2 +- examples/webclient-integrations/test.html | 8 +- openapi/openapi.yaml | 5 +- sftpd/sftpd_test.go | 154 +++++++++++++++++++++- templates/webclient/editfile.html | 2 +- templates/webclient/files.html | 8 +- 7 files changed, 195 insertions(+), 15 deletions(-) diff --git a/dataprovider/dataprovider.go b/dataprovider/dataprovider.go index 81e7bf3b..0f3d76a4 100644 --- a/dataprovider/dataprovider.go +++ b/dataprovider/dataprovider.go @@ -2348,11 +2348,32 @@ func getKeyboardInteractiveAnswers(client ssh.KeyboardInteractiveChallenge, resp return answers, err } if len(answers) == 1 && response.CheckPwd > 0 { - _, err = checkUserAndPass(user, answers[0], ip, protocol) - providerLog(logger.LevelInfo, "interactive auth hook requested password validation for user %#v, validation error: %v", - user.Username, err) - if err != nil { - return answers, err + if response.CheckPwd == 2 { + if !user.Filters.TOTPConfig.Enabled || !util.IsStringInSlice(protocolSSH, user.Filters.TOTPConfig.Protocols) { + providerLog(logger.LevelInfo, "keyboard interactive auth error: unable to check TOTP passcode, TOTP is not enabled for user %#v", + user.Username) + return answers, errors.New("TOTP not enabled for SSH protocol") + } + err := user.Filters.TOTPConfig.Secret.TryDecrypt() + if err != nil { + providerLog(logger.LevelWarn, "unable to decrypt TOTP secret for user %#v, protocol %v, err: %v", + user.Username, protocol, err) + return answers, fmt.Errorf("unable to decrypt TOTP secret: %w", err) + } + match, err := mfa.ValidateTOTPPasscode(user.Filters.TOTPConfig.ConfigName, answers[0], + user.Filters.TOTPConfig.Secret.GetPayload()) + if !match || err != nil { + providerLog(logger.LevelInfo, "keyboard interactive auth error: unable to validate passcode for user %#v, match? %v, err: %v", + user.Username, match, err) + return answers, errors.New("unable to validate TOTP passcode") + } + } else { + _, err = checkUserAndPass(user, answers[0], ip, protocol) + providerLog(logger.LevelInfo, "interactive auth hook requested password validation for user %#v, validation error: %v", + user.Username, err) + if err != nil { + return answers, err + } } answers[0] = "OK" } diff --git a/docs/keyboard-interactive.md b/docs/keyboard-interactive.md index 1f30d8cc..58c3344a 100644 --- a/docs/keyboard-interactive.md +++ b/docs/keyboard-interactive.md @@ -19,7 +19,7 @@ The program must write the questions on its standard output, in a single line, u - `instruction`, string. A short description to show to the user that is trying to authenticate. Can be empty or omitted - `questions`, list of questions to be asked to the user - `echos` list of boolean flags corresponding to the questions (so the lengths of both lists must be the same) and indicating whether user's reply for a particular question should be echoed on the screen while they are typing: true if it should be echoed, or false if it should be hidden. -- `check_password` optional integer. Ask exactly one question and set this field to 1 if the expected answer is the user password and you want that SFTPGo checks it for you. If the password is correct, the returned response to the program is `OK`. If the password is wrong, the program will be terminated and an authentication error will be returned to the user that is trying to authenticate. +- `check_password` optional integer. Ask exactly one question and set this field to `1` if the expected answer is the user password and you want that SFTPGo checks it for you or to `2` if the user has the SFTPGo built-in TOTP enabled and the expected answer is the user one time passcode. If the password/passcode is correct, the returned response to the program is `OK`. If the password is wrong, the program will be terminated and an authentication error will be returned to the user that is trying to authenticate. - `auth_result`, integer. Set this field to 1 to indicate successful authentication. 0 is ignored. Any other value means authentication error. If this field is found and it is different from 0 then SFTPGo will not read any other questions from the external program, and it will finalize the authentication. SFTPGo writes the user answers to the program standard input, one per line, in the same order as the questions. diff --git a/examples/webclient-integrations/test.html b/examples/webclient-integrations/test.html index cae527ef..30dc14da 100644 --- a/examples/webclient-integrations/test.html +++ b/examples/webclient-integrations/test.html @@ -24,7 +24,7 @@ // in real world usage set the origin when you call postMessage, we use `*` for testing purpose here $(document).ready(function () { if (window.opener == null || window.opener.closed) { - console.log("windows opener gone!"); + console.log("window opener gone!"); return; } // notify SFTPGo that the page is ready to receive the file @@ -33,7 +33,7 @@ window.addEventListener('message', (event) => { if (window.opener == null || window.opener.closed) { - console.log("windows opener gone!"); + console.log("window opener gone!"); return; } // you should check the origin before continuing @@ -76,6 +76,10 @@ }); function saveBlob(binary){ + if (window.opener == null || window.opener.closed) { + console.log("window opener gone!"); + return; + } // if we have modified the file we can send it back to SFTPGo as a blob for saving console.log("save blob, binary? "+binary); if (binary){ diff --git a/openapi/openapi.yaml b/openapi/openapi.yaml index cd9a2f50..f080b30d 100644 --- a/openapi/openapi.yaml +++ b/openapi/openapi.yaml @@ -4871,7 +4871,10 @@ components: - 1 - 2 description: | - * `0` - clear or explicit TLS * `1` - explicit TLS required * `2` - implicit TLS + TLS mode: + * `0` - clear or explicit TLS + * `1` - explicit TLS required + * `2` - implicit TLS force_passive_ip: type: string description: External IP address to expose for passive connections diff --git a/sftpd/sftpd_test.go b/sftpd/sftpd_test.go index 15a41ddd..a3f06e0c 100644 --- a/sftpd/sftpd_test.go +++ b/sftpd/sftpd_test.go @@ -30,6 +30,8 @@ import ( _ "github.com/go-sql-driver/mysql" _ "github.com/lib/pq" _ "github.com/mattn/go-sqlite3" + "github.com/pquerna/otp" + "github.com/pquerna/otp/totp" "github.com/pkg/sftp" "github.com/rs/zerolog" @@ -42,6 +44,7 @@ import ( "github.com/drakkan/sftpgo/v2/httpdtest" "github.com/drakkan/sftpgo/v2/kms" "github.com/drakkan/sftpgo/v2/logger" + "github.com/drakkan/sftpgo/v2/mfa" "github.com/drakkan/sftpgo/v2/sdk" "github.com/drakkan/sftpgo/v2/sftpd" "github.com/drakkan/sftpgo/v2/util" @@ -193,6 +196,12 @@ func TestMain(m *testing.M) { logger.ErrorToConsole("error initializing kms: %v", err) os.Exit(1) } + mfaConfig := config.GetMFAConfig() + err = mfaConfig.Initialize() + if err != nil { + logger.ErrorToConsole("error initializing MFA: %v", err) + os.Exit(1) + } sftpdConf := config.GetSFTPDConfig() httpdConf := config.GetHTTPDConfig() @@ -312,7 +321,7 @@ func TestMain(m *testing.M) { os.Remove(postConnectPath) os.Remove(preDownloadPath) os.Remove(preUploadPath) - os.Remove(keyIntAuthPath) + //os.Remove(keyIntAuthPath) os.Remove(checkPwdPath) os.Exit(exitCode) } @@ -2246,6 +2255,127 @@ func TestLoginKeyboardInteractiveAuth(t *testing.T) { assert.NoError(t, err) } +func TestInteractiveLoginWithPasscode(t *testing.T) { + if runtime.GOOS == osWindows { + t.Skip("this test is not available on Windows") + } + user, _, err := httpdtest.AddUser(getTestUser(false), http.StatusCreated) + assert.NoError(t, err) + // test password check + err = os.WriteFile(keyIntAuthPath, getKeyboardInteractiveScriptForBuiltinChecks(false, 1), os.ModePerm) + assert.NoError(t, err) + conn, client, err := getKeyboardInteractiveSftpClient(user, []string{defaultPassword}) + if assert.NoError(t, err) { + defer conn.Close() + defer client.Close() + assert.NoError(t, checkBasicSFTP(client)) + } + // wrong password + _, _, err = getKeyboardInteractiveSftpClient(user, []string{"wrong_password"}) + assert.Error(t, err) + // correct password but the script returns an error + err = os.WriteFile(keyIntAuthPath, getKeyboardInteractiveScriptForBuiltinChecks(false, 0), os.ModePerm) + assert.NoError(t, err) + _, _, err = getKeyboardInteractiveSftpClient(user, []string{"wrong_password"}) + assert.Error(t, err) + // add multi-factor authentication + configName, _, secret, _, err := mfa.GenerateTOTPSecret(mfa.GetAvailableTOTPConfigNames()[0], user.Username) + assert.NoError(t, err) + user.Password = defaultPassword + user.Filters.TOTPConfig = sdk.TOTPConfig{ + Enabled: true, + ConfigName: configName, + Secret: kms.NewPlainSecret(secret), + Protocols: []string{common.ProtocolSSH}, + } + err = dataprovider.UpdateUser(&user, "", "") + assert.NoError(t, err) + + passcode, err := totp.GenerateCodeCustom(secret, time.Now(), totp.ValidateOpts{ + Period: 30, + Skew: 1, + Digits: otp.DigitsSix, + Algorithm: otp.AlgorithmSHA1, + }) + assert.NoError(t, err) + err = os.WriteFile(keyIntAuthPath, getKeyboardInteractiveScriptForBuiltinChecks(true, 1), os.ModePerm) + assert.NoError(t, err) + + passwordAsked := false + passcodeAsked := false + authMethods := []ssh.AuthMethod{ + ssh.KeyboardInteractive(func(user, instruction string, questions []string, echos []bool) ([]string, error) { + var answers []string + if strings.HasPrefix(questions[0], "Password") { + answers = append(answers, defaultPassword) + passwordAsked = true + } else { + answers = append(answers, passcode) + passcodeAsked = true + } + return answers, nil + }), + } + conn, client, err = getCustomAuthSftpClient(user, authMethods, "") + if assert.NoError(t, err) { + defer conn.Close() + defer client.Close() + assert.NoError(t, checkBasicSFTP(client)) + } + assert.True(t, passwordAsked) + assert.True(t, passcodeAsked) + // the same passcode cannot be reused + _, _, err = getCustomAuthSftpClient(user, authMethods, "") + assert.Error(t, err) + // correct passcode but the script returns an error + configName, _, secret, _, err = mfa.GenerateTOTPSecret(mfa.GetAvailableTOTPConfigNames()[0], user.Username) + assert.NoError(t, err) + user.Password = defaultPassword + user.Filters.TOTPConfig = sdk.TOTPConfig{ + Enabled: true, + ConfigName: configName, + Secret: kms.NewPlainSecret(secret), + Protocols: []string{common.ProtocolSSH}, + } + err = dataprovider.UpdateUser(&user, "", "") + assert.NoError(t, err) + passcode, err = totp.GenerateCodeCustom(secret, time.Now(), totp.ValidateOpts{ + Period: 30, + Skew: 1, + Digits: otp.DigitsSix, + Algorithm: otp.AlgorithmSHA1, + }) + assert.NoError(t, err) + err = os.WriteFile(keyIntAuthPath, getKeyboardInteractiveScriptForBuiltinChecks(true, 0), os.ModePerm) + assert.NoError(t, err) + passwordAsked = false + passcodeAsked = false + _, _, err = getCustomAuthSftpClient(user, authMethods, "") + assert.Error(t, err) + authMethods = []ssh.AuthMethod{ + ssh.KeyboardInteractive(func(user, instruction string, questions []string, echos []bool) ([]string, error) { + var answers []string + if strings.HasPrefix(questions[0], "Password") { + answers = append(answers, defaultPassword) + passwordAsked = true + } else { + answers = append(answers, passcode) + passcodeAsked = true + } + return answers, nil + }), + } + _, _, err = getCustomAuthSftpClient(user, authMethods, "") + assert.Error(t, err) + assert.True(t, passwordAsked) + assert.True(t, passcodeAsked) + + _, err = httpdtest.RemoveUser(user, http.StatusOK) + assert.NoError(t, err) + err = os.RemoveAll(user.GetHomeDir()) + assert.NoError(t, err) +} + func TestPreLoginScript(t *testing.T) { if runtime.GOOS == osWindows { t.Skip("this test is not available on Windows") @@ -9909,6 +10039,28 @@ func addFileToGitRepo(repoPath string, fileSize int64) ([]byte, error) { return cmd.CombinedOutput() } +func getKeyboardInteractiveScriptForBuiltinChecks(addPasscode bool, result int) []byte { + content := []byte("#!/bin/sh\n\n") + echos := []bool{false} + q, _ := json.Marshal([]string{"Password: "}) + e, _ := json.Marshal(echos) + content = append(content, []byte(fmt.Sprintf("echo '{\"questions\":%v,\"echos\":%v,\"check_password\":1}'\n", string(q), string(e)))...) + content = append(content, []byte("read ANSWER\n\n")...) + content = append(content, []byte("if test \"$ANSWER\" != \"OK\"; then\n")...) + content = append(content, []byte("exit 1\n")...) + content = append(content, []byte("fi\n\n")...) + if addPasscode { + q, _ := json.Marshal([]string{"Passcode: "}) + content = append(content, []byte(fmt.Sprintf("echo '{\"questions\":%v,\"echos\":%v,\"check_password\":2}'\n", string(q), string(e)))...) + content = append(content, []byte("read ANSWER\n\n")...) + content = append(content, []byte("if test \"$ANSWER\" != \"OK\"; then\n")...) + content = append(content, []byte("exit 1\n")...) + content = append(content, []byte("fi\n\n")...) + } + content = append(content, []byte(fmt.Sprintf("echo '{\"auth_result\":%v}'\n", result))...) + return content +} + func getKeyboardInteractiveScriptContent(questions []string, sleepTime int, nonJSONResponse bool, result int) []byte { content := []byte("#!/bin/sh\n\n") q, _ := json.Marshal(questions) diff --git a/templates/webclient/editfile.html b/templates/webclient/editfile.html index 2eef6f74..215a4e29 100644 --- a/templates/webclient/editfile.html +++ b/templates/webclient/editfile.html @@ -121,7 +121,7 @@ cm.setOption("mode", mode.mode); } cm.setValue("{{.Data}}"); - setInterval(keepAlive, 180000); + setInterval(keepAlive, 300000); }); function keepAlive() { diff --git a/templates/webclient/files.html b/templates/webclient/files.html index b5578a83..08f4db52 100644 --- a/templates/webclient/files.html +++ b/templates/webclient/files.html @@ -198,7 +198,7 @@ childReference = window.open(url, '_blank'); if (!checkerStarted){ keepAlive(); - setInterval(checkExternalWindow, 180000); + setInterval(checkExternalWindow, 300000); checkerStarted = true; } } else { @@ -212,7 +212,7 @@ function notifySave(status, message){ if (childReference == null || childReference.closed) { - console.log("external windows null or closed, cannot notify save"); + console.log("external window null or closed, cannot notify save"); return; } @@ -230,7 +230,7 @@ return; } if (childReference == null || childReference.closed) { - console.log("external windows null or closed, refusing message"); + console.log("external window null or closed, refusing message"); return; } switch (event.data.type){ @@ -561,7 +561,7 @@ $("#upload_files_form").submit(function (event){ event.preventDefault(); keepAlive(); - var keepAliveTimer = setInterval(keepAlive, 180000); + var keepAliveTimer = setInterval(keepAlive, 300000); var path = '{{.FilesURL}}?path={{.CurrentDir}}'; var files = $("#files_name")[0].files;