[crypto] ssh: fix support for partial success authentication responses in client
Gobot Gobot (Gerrit)
Congratulations on opening your first change. Thank you for your contribution!
Next steps:
Within the next week or so, a maintainer will review your change and provide
feedback. See https://golang.org/doc/contribute.html#review for more info and
tips to get your patch through code review.
Most changes in the Go project go through a few rounds of revision. This can be
surprising to people new to the project. The careful, iterative review process
is our way of helping mentor contributors and ensuring that their contributions
have a lasting impact.
During May-July and Nov-Jan the Go project is in a code freeze, during which
little code gets reviewed or merged. If a reviewer responds with a comment like
R=go1.11, it means that this CL will be reviewed as part of the next development
cycle. See https://golang.org/s/release for more details.
To view, visit change 88035. To unsubscribe, or for help writing mail filters, visit settings.
Sami Pönkänen (Gerrit)
Sami Pönkänen has uploaded this change for review.
ssh: fix support for partial success authentication responses in client
The existing client side authentication does not handle correctly
the partial success flag in SSH_MSG_USERAUTH_FAILURE authentication
responses.
This commit fixes two problems:
1) RetryableAuthMethod() now breaks out from the retry loop and
returns when underlying auth method fails with partial success
set to true.
2) Book keeping of tried (and failed) auth methods in
clientAuthenticate() does not mark an auth method failed if it
fails with partial success set to true.
Fixes golang/go#23461
Change-Id: Ib2e1a1d54bfe2549496199bb2f66ebbce58d130d
---
M ssh/client_auth.go
M ssh/keys_test.go
2 files changed, 67 insertions(+), 52 deletions(-)
diff --git a/ssh/client_auth.go b/ssh/client_auth.go
index a1252cb..5f44b77 100644
--- a/ssh/client_auth.go
+++ b/ssh/client_auth.go
@@ -11,6 +11,14 @@
"io"
)
+type authResult int
+
+const (
+ authFailure authResult = iota
+ authPartialSuccess
+ authSuccess
+)
+
// clientAuthenticate authenticates with the remote server. See RFC 4252.
func (c *connection) clientAuthenticate(config *ClientConfig) error {
// initiate user auth session
@@ -37,11 +45,12 @@
if err != nil {
return err
}
- if ok {
+ if ok == authSuccess {
// success
return nil
+ } else if ok == authFailure {
+ tried[auth.method()] = true
}
- tried[auth.method()] = true
if methods == nil {
methods = lastMethods
}
@@ -82,7 +91,7 @@
// If authentication is not successful, a []string of alternative
// method names is returned. If the slice is nil, it will be ignored
// and the previous set of possible methods will be reused.
- auth(session []byte, user string, p packetConn, rand io.Reader) (bool, []string, error)
+ auth(session []byte, user string, p packetConn, rand io.Reader) (authResult, []string, error)
// method returns the RFC 4252 method name.
method() string
@@ -91,13 +100,13 @@
// "none" authentication, RFC 4252 section 5.2.
type noneAuth int
-func (n *noneAuth) auth(session []byte, user string, c packetConn, rand io.Reader) (bool, []string, error) {
+func (n *noneAuth) auth(session []byte, user string, c packetConn, rand io.Reader) (authResult, []string, error) {
if err := c.writePacket(Marshal(&userAuthRequestMsg{
User: user,
Service: serviceSSH,
Method: "none",
})); err != nil {
- return false, nil, err
+ return authFailure, nil, err
}
return handleAuthResponse(c)
@@ -111,7 +120,7 @@
// a function call, e.g. by prompting the user.
type passwordCallback func() (password string, err error)
-func (cb passwordCallback) auth(session []byte, user string, c packetConn, rand io.Reader) (bool, []string, error) {
+func (cb passwordCallback) auth(session []byte, user string, c packetConn, rand io.Reader) (authResult, []string, error) {
type passwordAuthMsg struct {
User string `sshtype:"50"`
Service string
@@ -125,7 +134,7 @@
// The program may only find out that the user doesn't have a password
// when prompting.
if err != nil {
- return false, nil, err
+ return authFailure, nil, err
}
if err := c.writePacket(Marshal(&passwordAuthMsg{
@@ -135,7 +144,7 @@
Reply: false,
Password: pw,
})); err != nil {
- return false, nil, err
+ return authFailure, nil, err
}
return handleAuthResponse(c)
@@ -178,7 +187,7 @@
return "publickey"
}
-func (cb publicKeyCallback) auth(session []byte, user string, c packetConn, rand io.Reader) (bool, []string, error) {
+func (cb publicKeyCallback) auth(session []byte, user string, c packetConn, rand io.Reader) (authResult, []string, error) {
// Authentication is performed by sending an enquiry to test if a key is
// acceptable to the remote. If the key is acceptable, the client will
// attempt to authenticate with the valid key. If not the client will repeat
@@ -186,13 +195,13 @@
signers, err := cb()
if err != nil {
- return false, nil, err
+ return authFailure, nil, err
}
var methods []string
for _, signer := range signers {
ok, err := validateKey(signer.PublicKey(), user, c)
if err != nil {
- return false, nil, err
+ return authFailure, nil, err
}
if !ok {
continue
@@ -206,7 +215,7 @@
Method: cb.method(),
}, []byte(pub.Type()), pubKey))
if err != nil {
- return false, nil, err
+ return authFailure, nil, err
}
// manually wrap the serialized signature in a string
@@ -224,24 +233,24 @@
}
p := Marshal(&msg)
if err := c.writePacket(p); err != nil {
- return false, nil, err
+ return authFailure, nil, err
}
- var success bool
+ var success authResult
success, methods, err = handleAuthResponse(c)
if err != nil {
- return false, nil, err
+ return authFailure, nil, err
}
// If authentication succeeds or the list of available methods does not
// contain the "publickey" method, do not attempt to authenticate with any
// other keys. According to RFC 4252 Section 7, the latter can occur when
// additional authentication methods are required.
- if success || !containsMethod(methods, cb.method()) {
+ if success == authSuccess || !containsMethod(methods, cb.method()) {
return success, methods, err
}
}
- return false, methods, nil
+ return authFailure, methods, nil
}
func containsMethod(methods []string, method string) bool {
@@ -318,28 +327,31 @@
// handleAuthResponse returns whether the preceding authentication request succeeded
// along with a list of remaining authentication methods to try next and
// an error if an unexpected response was received.
-func handleAuthResponse(c packetConn) (bool, []string, error) {
+func handleAuthResponse(c packetConn) (authResult, []string, error) {
for {
packet, err := c.readPacket()
if err != nil {
- return false, nil, err
+ return authFailure, nil, err
}
switch packet[0] {
case msgUserAuthBanner:
if err := handleBannerResponse(c, packet); err != nil {
- return false, nil, err
+ return authFailure, nil, err
}
case msgUserAuthFailure:
var msg userAuthFailureMsg
if err := Unmarshal(packet, &msg); err != nil {
- return false, nil, err
+ return authFailure, nil, err
}
- return false, msg.Methods, nil
+ if msg.PartialSuccess {
+ return authPartialSuccess, msg.Methods, nil
+ }
+ return authFailure, msg.Methods, nil
case msgUserAuthSuccess:
- return true, nil, nil
+ return authSuccess, nil, nil
default:
- return false, nil, unexpectedMessageError(msgUserAuthSuccess, packet[0])
+ return authFailure, nil, unexpectedMessageError(msgUserAuthSuccess, packet[0])
}
}
}
@@ -381,7 +393,7 @@
return "keyboard-interactive"
}
-func (cb KeyboardInteractiveChallenge) auth(session []byte, user string, c packetConn, rand io.Reader) (bool, []string, error) {
+func (cb KeyboardInteractiveChallenge) auth(session []byte, user string, c packetConn, rand io.Reader) (authResult, []string, error) {
type initiateMsg struct {
User string `sshtype:"50"`
Service string
@@ -395,20 +407,20 @@
Service: serviceSSH,
Method: "keyboard-interactive",
})); err != nil {
- return false, nil, err
+ return authFailure, nil, err
}
for {
packet, err := c.readPacket()
if err != nil {
- return false, nil, err
+ return authFailure, nil, err
}
// like handleAuthResponse, but with less options.
switch packet[0] {
case msgUserAuthBanner:
if err := handleBannerResponse(c, packet); err != nil {
- return false, nil, err
+ return authFailure, nil, err
}
continue
case msgUserAuthInfoRequest:
@@ -416,18 +428,21 @@
case msgUserAuthFailure:
var msg userAuthFailureMsg
if err := Unmarshal(packet, &msg); err != nil {
- return false, nil, err
+ return authFailure, nil, err
}
- return false, msg.Methods, nil
+ if msg.PartialSuccess {
+ return authPartialSuccess, msg.Methods, nil
+ }
+ return authFailure, msg.Methods, nil
case msgUserAuthSuccess:
- return true, nil, nil
+ return authSuccess, nil, nil
default:
- return false, nil, unexpectedMessageError(msgUserAuthInfoRequest, packet[0])
+ return authFailure, nil, unexpectedMessageError(msgUserAuthInfoRequest, packet[0])
}
var msg userAuthInfoRequestMsg
if err := Unmarshal(packet, &msg); err != nil {
- return false, nil, err
+ return authFailure, nil, err
}
// Manually unpack the prompt/echo pairs.
@@ -437,7 +452,7 @@
for i := 0; i < int(msg.NumPrompts); i++ {
prompt, r, ok := parseString(rest)
if !ok || len(r) == 0 {
- return false, nil, errors.New("ssh: prompt format error")
+ return authFailure, nil, errors.New("ssh: prompt format error")
}
prompts = append(prompts, string(prompt))
echos = append(echos, r[0] != 0)
@@ -445,16 +460,16 @@
}
if len(rest) != 0 {
- return false, nil, errors.New("ssh: extra data following keyboard-interactive pairs")
+ return authFailure, nil, errors.New("ssh: extra data following keyboard-interactive pairs")
}
answers, err := cb(msg.User, msg.Instruction, prompts, echos)
if err != nil {
- return false, nil, err
+ return authFailure, nil, err
}
if len(answers) != len(prompts) {
- return false, nil, errors.New("ssh: not enough answers from keyboard-interactive callback")
+ return authFailure, nil, errors.New("ssh: not enough answers from keyboard-interactive callback")
}
responseLength := 1 + 4
for _, a := range answers {
@@ -470,7 +485,7 @@
}
if err := c.writePacket(serialized); err != nil {
- return false, nil, err
+ return authFailure, nil, err
}
}
}
@@ -480,10 +495,10 @@
maxTries int
}
-func (r *retryableAuthMethod) auth(session []byte, user string, c packetConn, rand io.Reader) (ok bool, methods []string, err error) {
+func (r *retryableAuthMethod) auth(session []byte, user string, c packetConn, rand io.Reader) (ok authResult, methods []string, err error) {
for i := 0; r.maxTries <= 0 || i < r.maxTries; i++ {
ok, methods, err = r.authMethod.auth(session, user, c, rand)
- if ok || err != nil { // either success or error terminate
+ if ok != authFailure || err != nil { // either success, partial success or error terminate
return ok, methods, err
}
}
diff --git a/ssh/keys_test.go b/ssh/keys_test.go
index 20ab954..9a90abc 100644
--- a/ssh/keys_test.go
+++ b/ssh/keys_test.go
@@ -234,7 +234,7 @@
}
}
-type authResult struct {
+type testAuthResult struct {
pubKey PublicKey
options []string
comments string
@@ -242,11 +242,11 @@
ok bool
}
-func testAuthorizedKeys(t *testing.T, authKeys []byte, expected []authResult) {
+func testAuthorizedKeys(t *testing.T, authKeys []byte, expected []testAuthResult) {
rest := authKeys
- var values []authResult
+ var values []testAuthResult
for len(rest) > 0 {
- var r authResult
+ var r testAuthResult
var err error
r.pubKey, r.comments, r.options, rest, err = ParseAuthorizedKey(rest)
r.ok = (err == nil)
@@ -264,7 +264,7 @@
pub, pubSerialized := getTestKey()
line := "ssh-rsa " + pubSerialized + " user@host"
testAuthorizedKeys(t, []byte(line),
- []authResult{
+ []testAuthResult{
{pub, nil, "user@host", "", true},
})
}
@@ -286,7 +286,7 @@
authOptions := strings.Join(authWithOptions, eol)
rest2 := strings.Join(authWithOptions[3:], eol)
rest3 := strings.Join(authWithOptions[6:], eol)
- testAuthorizedKeys(t, []byte(authOptions), []authResult{
+ testAuthorizedKeys(t, []byte(authOptions), []testAuthResult{
{pub, []string{`env="HOME=/home/root"`, "no-port-forwarding"}, "user@host", rest2, true},
{pub, []string{`env="HOME=/home/root2"`}, "user2@host2", rest3, true},
{nil, nil, "", "", false},
@@ -297,7 +297,7 @@
func TestAuthWithQuotedSpaceInEnv(t *testing.T) {
pub, pubSerialized := getTestKey()
authWithQuotedSpaceInEnv := []byte(`env="HOME=/home/root dir",no-port-forwarding ssh-rsa ` + pubSerialized + ` user@host`)
- testAuthorizedKeys(t, []byte(authWithQuotedSpaceInEnv), []authResult{
+ testAuthorizedKeys(t, []byte(authWithQuotedSpaceInEnv), []testAuthResult{
{pub, []string{`env="HOME=/home/root dir"`, "no-port-forwarding"}, "user@host", "", true},
})
}
@@ -305,7 +305,7 @@
func TestAuthWithQuotedCommaInEnv(t *testing.T) {
pub, pubSerialized := getTestKey()
authWithQuotedCommaInEnv := []byte(`env="HOME=/home/root,dir",no-port-forwarding ssh-rsa ` + pubSerialized + ` user@host`)
- testAuthorizedKeys(t, []byte(authWithQuotedCommaInEnv), []authResult{
+ testAuthorizedKeys(t, []byte(authWithQuotedCommaInEnv), []testAuthResult{
{pub, []string{`env="HOME=/home/root,dir"`, "no-port-forwarding"}, "user@host", "", true},
})
}
@@ -314,11 +314,11 @@
pub, pubSerialized := getTestKey()
authWithQuotedQuoteInEnv := []byte(`env="HOME=/home/\"root dir",no-port-forwarding` + "\t" + `ssh-rsa` + "\t" + pubSerialized + ` user@host`)
authWithDoubleQuotedQuote := []byte(`no-port-forwarding,env="HOME=/home/ \"root dir\"" ssh-rsa ` + pubSerialized + "\t" + `user@host`)
- testAuthorizedKeys(t, []byte(authWithQuotedQuoteInEnv), []authResult{
+ testAuthorizedKeys(t, []byte(authWithQuotedQuoteInEnv), []testAuthResult{
{pub, []string{`env="HOME=/home/\"root dir"`, "no-port-forwarding"}, "user@host", "", true},
})
- testAuthorizedKeys(t, []byte(authWithDoubleQuotedQuote), []authResult{
+ testAuthorizedKeys(t, []byte(authWithDoubleQuotedQuote), []testAuthResult{
{pub, []string{"no-port-forwarding", `env="HOME=/home/ \"root dir\""`}, "user@host", "", true},
})
}
@@ -327,7 +327,7 @@
_, pubSerialized := getTestKey()
authWithInvalidSpace := []byte(`env="HOME=/home/root dir", no-port-forwarding ssh-rsa ` + pubSerialized + ` user@host
#more to follow but still no valid keys`)
- testAuthorizedKeys(t, []byte(authWithInvalidSpace), []authResult{
+ testAuthorizedKeys(t, []byte(authWithInvalidSpace), []testAuthResult{
{nil, nil, "", "", false},
})
}
@@ -337,7 +337,7 @@
authWithMissingQuote := []byte(`env="HOME=/home/root,no-port-forwarding ssh-rsa ` + pubSerialized + ` user@host
env="HOME=/home/root",shared-control ssh-rsa ` + pubSerialized + ` user@host`)
- testAuthorizedKeys(t, []byte(authWithMissingQuote), []authResult{
+ testAuthorizedKeys(t, []byte(authWithMissingQuote), []testAuthResult{
{pub, []string{`env="HOME=/home/root"`, `shared-control`}, "user@host", "", true},
})
}
To view, visit change 88035. To unsubscribe, or for help writing mail filters, visit settings.
Han-Wen Nienhuys (Gerrit)
Is there a way to trigger this condition with an openssh configuration? If so, I'd really like to see such a configuration being tested under test/
Sami Pönkänen (Gerrit)
Patch Set 1:
Is there a way to trigger this condition with an openssh configuration? If so, I'd really like to see such a configuration being tested under test/
Yes, see https://github.com/golang/go/issues/23461 for details, but in a nut shell use RetryableAuthMethod(PasswordCallback, 2) and put this to /etc/ssh/sshd_config:
AuthenticationMethods password,publickey
Sami Pönkänen (Gerrit)
Patch Set 1:
Patch Set 1:
Is there a way to trigger this condition with an openssh configuration? If so, I'd really like to see such a configuration being tested under test/
Yes, see https://github.com/golang/go/issues/23461 for details, but in a nut shell use RetryableAuthMethod(PasswordCallback, 2) and put this to /etc/ssh/sshd_config:
AuthenticationMethods password,publickey
And below is the output of 'ssh -v' to an OpenSSH server with the above config:
$ ssh -v bu...@192.168.200.101
OpenSSH_7.2p2 Ubuntu-4ubuntu2.2, OpenSSL 1.0.2g 1 Mar 2016
debug1: Reading configuration data /etc/ssh/ssh_config
debug1: /etc/ssh/ssh_config line 19: Applying options for *
debug1: Connecting to 192.168.200.101 [192.168.200.101] port 22.
debug1: Connection established.
debug1: identity file /home/ext-sponkane/.ssh/id_rsa type 1
debug1: key_load_public: No such file or directory
debug1: identity file /home/ext-sponkane/.ssh/id_rsa-cert type -1
debug1: key_load_public: No such file or directory
debug1: identity file /home/ext-sponkane/.ssh/id_dsa type -1
debug1: key_load_public: No such file or directory
debug1: identity file /home/ext-sponkane/.ssh/id_dsa-cert type -1
debug1: key_load_public: No such file or directory
debug1: identity file /home/ext-sponkane/.ssh/id_ecdsa type -1
debug1: key_load_public: No such file or directory
debug1: identity file /home/ext-sponkane/.ssh/id_ecdsa-cert type -1
debug1: key_load_public: No such file or directory
debug1: identity file /home/ext-sponkane/.ssh/id_ed25519 type -1
debug1: key_load_public: No such file or directory
debug1: identity file /home/ext-sponkane/.ssh/id_ed25519-cert type -1
debug1: Enabling compatibility mode for protocol 2.0
debug1: Local version string SSH-2.0-OpenSSH_7.2p2 Ubuntu-4ubuntu2.2
debug1: Remote protocol version 2.0, remote software version OpenSSH_7.4
debug1: match: OpenSSH_7.4 pat OpenSSH* compat 0x04000000
debug1: Authenticating to 192.168.200.101:22 as 'burt'
debug1: SSH2_MSG_KEXINIT sent
debug1: SSH2_MSG_KEXINIT received
debug1: kex: algorithm: curve255...@libssh.org
debug1: kex: host key algorithm: ecdsa-sha2-nistp256
debug1: kex: server->client cipher: chacha20...@openssh.com MAC: <implicit> compression: none
debug1: kex: client->server cipher: chacha20...@openssh.com MAC: <implicit> compression: none
debug1: expecting SSH2_MSG_KEX_ECDH_REPLY
debug1: Server host key: ecdsa-sha2-nistp256 SHA256:czx7JiH9M++oXzx4UlbQzI5pEkgilfF8gdsGiZ3Gps4
debug1: Host '192.168.200.101' is known and matches the ECDSA host key.
debug1: Found key in /home/ext-sponkane/.ssh/known_hosts:2
debug1: rekey after 134217728 blocks
debug1: SSH2_MSG_NEWKEYS sent
debug1: expecting SSH2_MSG_NEWKEYS
debug1: rekey after 134217728 blocks
debug1: SSH2_MSG_NEWKEYS received
debug1: SSH2_MSG_EXT_INFO received
debug1: kex_input_ext_info: server-sig-algs=<rsa-sha2-256,rsa-sha2-512>
debug1: SSH2_MSG_SERVICE_ACCEPT received
debug1: Authentications that can continue: password
debug1: Next authentication method: password
bu...@192.168.200.101's password:
Authenticated with partial success.
debug1: Authentications that can continue: publickey
debug1: Next authentication method: publickey
debug1: Trying private key: /home/ext-sponkane/.ssh/id_rsa
Enter passphrase for key '/home/ext-sponkane/.ssh/id_rsa':
debug1: Authentication succeeded (publickey).
Authenticated to 192.168.200.101 ([192.168.200.101]:22).
debug1: channel 0: new [client-session]
debug1: Requesting no-more-...@openssh.com
debug1: Entering interactive session.
debug1: pledge: network
debug1: client_input_global_request: rtype hostk...@openssh.com want_reply 0
debug1: Sending environment.
debug1: Sending env LC_PAPER = fi_FI.UTF-8
debug1: Sending env LC_ADDRESS = fi_FI.UTF-8
debug1: Sending env LC_MONETARY = fi_FI.UTF-8
debug1: Sending env LC_NUMERIC = fi_FI.UTF-8
debug1: Sending env LC_TELEPHONE = fi_FI.UTF-8
debug1: Sending env LC_IDENTIFICATION = fi_FI.UTF-8
debug1: Sending env LANG = en_US.UTF-8
debug1: Sending env LC_MEASUREMENT = fi_FI.UTF-8
debug1: Sending env LC_TIME = fi_FI.UTF-8
debug1: Sending env LC_NAME = fi_FI.UTF-8
Han-Wen Nienhuys (Gerrit)
nice. Can you create a testcase for this? There is code to run a Go client against openssh's sshd under test/
Sami Pönkänen (Gerrit)
Sami Pönkänen uploaded patch set #2 to this change.
ssh: fix support for partial success authentication responses in client
The existing client side authentication does not handle correctly
the partial success flag in SSH_MSG_USERAUTH_FAILURE authentication
responses.
This commit fixes two problems:
1) RetryableAuthMethod() now breaks out from the retry loop and
returns when underlying auth method fails with partial success
set to true.
2) Book keeping of tried (and failed) auth methods in
clientAuthenticate() does not mark an auth method failed if it
fails with partial success set to true.
Fixes golang/go#23461
Change-Id: Ib2e1a1d54bfe2549496199bb2f66ebbce58d130d
---
M ssh/client_auth.go
M ssh/keys_test.go
A ssh/test/multi_auth_test.go
M ssh/test/test_unix_test.go
4 files changed, 319 insertions(+), 59 deletions(-)
To view, visit change 88035. To unsubscribe, or for help writing mail filters, visit settings.
Han-Wen Nienhuys (Gerrit)
thanks. This is just a quick look. I'll look more closely tomorrow. Hopefully I'll get something back from djm too.
3 comments:
File ssh/test/multi_auth_test.go:
Patch Set #2, Line 11:
these tests must be run as root.that is a shame, but better than nothing. I'll double check with damien if there is no way around this.
Patch Set #2, Line 38:
multiAuthPasswordthis is the password for a live user on the system, right? If so, drop this.
The command-line is publicly visible through ps -ef.
Patch Set #2, Line 83:
numKeyboardInteractiveCallbacksis it necessary for these to be global? If not, can you encapsulate this in struct that each test creates from scratch?
To view, visit change 88035. To unsubscribe, or for help writing mail filters, visit settings.
Sami Pönkänen (Gerrit)
Patch Set 1:
nice. Can you create a testcase for this? There is code to run a Go client against openssh's sshd under test/
Hi,
I had some trouble creating proper test cases for this bug, but I managed to create tests for:
1) "AuthenticationMethods password,publickey" with RetryableAuthMethod(Password)
2) "AuthenticationMethods keyboard-interactive,publickey" with RetryableAuthMethod(KeyboardInteractiveChallenge)
3) "AuthenticationMethods publickey,password" with RetryableAuthMethod(Password)
4) "AuthenticationMethods publickey,keyboard-interactive" with RetryableAuthMethod(KeyboardInteractiveChallenge)
Without patch set 1 test 1 fails as expected, and tests 3 and 4 pass as expected.
Also test 2 passes - although the underlying problem is there - because there is no way to detect the actual bug outside of clientAuthenticate() and retryableAuthMethod.auth(). It is visible in the OpenSSH server logs though; look for these lines repeating:
debug1: userauth-request for user sami service ssh-connection method keyboard-interactive [preauth]
debug1: attempt 2 failures 0 [preauth]
debug2: Unrecognized authentication method name: keyboard-interactive [preauth]
Another problem with the tests was that the OpenSSH server validates passwords using PAM, and therefore the tests must be run as root/sudo and with real username/password input data. Because of this restriction the multi-auth tests are skipped unless the tests are run with command line argument '-user'. The password for the tests can be given with command line argument '-password' or otherwise it will be prompted interactively while running the tests.
So, not perfect but better than no tests, right?
Let me know if there is a more elegant way of implementing password/keyboard-interactive tests with OpenSSH server.
Sami
Sami Pönkänen (Gerrit)
Patch Set 2:
(3 comments)
thanks. This is just a quick look. I'll look more closely tomorrow. Hopefully I'll get something back from djm too.
Line 11: Yes, agree. Let me know if you know a way to work around this.
Line 38: Ok. Do you need another way to define password (besides prompted password)? Thinking of making test automation easier. Maybe os.Getenv()?
Line 83: Ok, I can move these to per test case context. After that change there would still be the global var for the prompted password so that it needs to be prompted only once per test run and reused in following test cases.
Sami Pönkänen (Gerrit)
Patch Set 2:
Patch Set 2:
(3 comments)
thanks. This is just a quick look. I'll look more closely tomorrow. Hopefully I'll get something back from djm too.
Line 11: Yes, agree. Let me know if you know a way to work around this.
Line 38: Ok. Do you need another way to define password (besides prompted password)? Thinking of making test automation easier. Maybe os.Getenv()?
Line 83: Ok, I can move these to per test case context. After that change there would still be the global var for the prompted password so that it needs to be prompted only once per test run and reused in following test cases.
As per your suggestion I have created a sshd_test_pw.so wrapper library for injecting test password data for sshd + PAM.
I am planning to add the source code to test/lib/linux/src/sshd_test_pw.so and the compiled library to test/lib/linux/amd64/sshd_test_pw.so. Are these the correct places for them?
Han-Wen Nienhuys (Gerrit)
Patch Set 2:
Patch Set 2:
Patch Set 2:
(3 comments)
thanks. This is just a quick look. I'll look more closely tomorrow. Hopefully I'll get something back from djm too.
Line 11: Yes, agree. Let me know if you know a way to work around this.
Line 38: Ok. Do you need another way to define password (besides prompted password)? Thinking of making test automation easier. Maybe os.Getenv()?
Line 83: Ok, I can move these to per test case context. After that change there would still be the global var for the prompted password so that it needs to be prompted only once per test run and reused in following test cases.
As per your suggestion I have created a sshd_test_pw.so wrapper library for injecting test password data for sshd + PAM.
I am planning to add the source code to test/lib/linux/src/sshd_test_pw.so and the compiled library to test/lib/linux/amd64/sshd_test_pw.so. Are these the correct places for them?
I suggest to simply put it in the same directory, and to leave out the binary artifact. That will require people to have a C compiler, but that's less of requirement than that they use AMD64.
Sami Pönkänen (Gerrit)
Patch Set 2:
Patch Set 2:
Patch Set 2:
Patch Set 2:
(3 comments)
thanks. This is just a quick look. I'll look more closely tomorrow. Hopefully I'll get something back from djm too.
Line 11: Yes, agree. Let me know if you know a way to work around this.
Line 38: Ok. Do you need another way to define password (besides prompted password)? Thinking of making test automation easier. Maybe os.Getenv()?
Line 83: Ok, I can move these to per test case context. After that change there would still be the global var for the prompted password so that it needs to be prompted only once per test run and reused in following test cases.
As per your suggestion I have created a sshd_test_pw.so wrapper library for injecting test password data for sshd + PAM.
I am planning to add the source code to test/lib/linux/src/sshd_test_pw.so and the compiled library to test/lib/linux/amd64/sshd_test_pw.so. Are these the correct places for them?
I suggest to simply put it in the same directory, and to leave out the binary artifact. That will require people to have a C compiler, but that's less of requirement than that they use AMD64.
Placing the C src file in test/ is a bit problematic:
$ go fmt
can't load package: package golang.org/x/crypto/ssh/test: C source files not allowed when not using cgo or SWIG: sshd_test_pw.c
Would subdirectory sshd_test_pw/ be better?
Sami Pönkänen (Gerrit)
Patch Set 2:
Patch Set 2:
Patch Set 2:
Patch Set 2:
Patch Set 2:
(3 comments)
thanks. This is just a quick look. I'll look more closely tomorrow. Hopefully I'll get something back from djm too.
Line 11: Yes, agree. Let me know if you know a way to work around this.
Line 38: Ok. Do you need another way to define password (besides prompted password)? Thinking of making test automation easier. Maybe os.Getenv()?
Line 83: Ok, I can move these to per test case context. After that change there would still be the global var for the prompted password so that it needs to be prompted only once per test run and reused in following test cases.
As per your suggestion I have created a sshd_test_pw.so wrapper library for injecting test password data for sshd + PAM.
I am planning to add the source code to test/lib/linux/src/sshd_test_pw.so and the compiled library to test/lib/linux/amd64/sshd_test_pw.so. Are these the correct places for them?
I suggest to simply put it in the same directory, and to leave out the binary artifact. That will require people to have a C compiler, but that's less of requirement than that they use AMD64.
Placing the C src file in test/ is a bit problematic:
$ go fmt
can't load package: package golang.org/x/crypto/ssh/test: C source files not allowed when not using cgo or SWIG: sshd_test_pw.cWould subdirectory sshd_test_pw/ be better?
Please ignore; adding '// +build ignore' solves the issue.
Sami Pönkänen (Gerrit)
Sami Pönkänen uploaded patch set #3 to this change.
ssh: fix support for partial success authentication responses in client
The existing client side authentication does not handle correctly
the partial success flag in SSH_MSG_USERAUTH_FAILURE authentication
responses.
This commit fixes two problems in ssh library:
1) RetryableAuthMethod() now breaks out from the retry loop and
returns when underlying auth method fails with partial success
set to true.
2) Book keeping of tried (and failed) auth methods in
clientAuthenticate() does not mark an auth method failed if it
fails with partial success set to true.
Fixes golang/go#23461
Change-Id: Ib2e1a1d54bfe2549496199bb2f66ebbce58d130d
---
M ssh/client_auth.go
M ssh/keys_test.go
A ssh/test/multi_auth_test.go
A ssh/test/sshd_test_pw.c
M ssh/test/test_unix_test.go
5 files changed, 628 insertions(+), 59 deletions(-)
To view, visit change 88035. To unsubscribe, or for help writing mail filters, visit settings.
Han-Wen Nienhuys (Gerrit)
(+ damien)
4 comments:
Patch Set #3, Line 25:
_GNU_SOURCEthis looks considerably more complicate than the poc we got from Damien; why? Is this because of BSD vs Linux?
abort if the env vars aren't defined.
Patch Set #3, Line 147:
real_getpwnam_rwhy? This never happens for our test?
Patch Set #3, Line 151:
strncpycan you use
strdup()
instead? That will get rid of the hardcoded lengths and \0 meddling.
also, I don't understand why you copy to pw_name from pwd->pw_name, to copy it into pwd->pw_name down below again.
also, why get the passwd from the real function anyway? You could copy strings from the environment?
To view, visit change 88035. To unsubscribe, or for help writing mail filters, visit settings.
Sami Pönkänen (Gerrit)
Sami Pönkänen uploaded patch set #4 to this change.
ssh: fix support for partial success authentication responses in client
The existing client side authentication does not handle correctly
the partial success flag in SSH_MSG_USERAUTH_FAILURE authentication
responses.
This commit fixes two problems in ssh library:
1) RetryableAuthMethod() now breaks out from the retry loop and
returns when underlying auth method fails with partial success
set to true.
2) Book keeping of tried (and failed) auth methods in
clientAuthenticate() does not mark an auth method failed if it
fails with partial success set to true.
Fixes golang/go#23461
Change-Id: Ib2e1a1d54bfe2549496199bb2f66ebbce58d130d
---
M ssh/client_auth.go
M ssh/keys_test.go
A ssh/test/multi_auth_test.go
A ssh/test/sshd_test_pw.c
M ssh/test/test_unix_test.go
5 files changed, 530 insertions(+), 59 deletions(-)
To view, visit change 88035. To unsubscribe, or for help writing mail filters, visit settings.
Han-Wen Nienhuys (Gerrit)
sorry for the delay. This week had some unexpected things pop up.
3 comments:
pid_t pid = getpid();
time_t now = time(NULL);
since we generate the test password, is there a reason put a non-zero salt here?
0 , right?
File ssh/test/test_unix_test.go:
PasswordAuthentication yescan you do somethign like
baseTpl = ` ... `"default": baseTpl,
"multi": baseTpl + ` ... `
To view, visit change 88035. To unsubscribe, or for help writing mail filters, visit settings.
Sami Pönkänen (Gerrit)
Sami Pönkänen uploaded patch set #5 to this change.
ssh: fix support for partial success authentication responses in client
The existing client side authentication does not handle correctly
the partial success flag in SSH_MSG_USERAUTH_FAILURE authentication
responses.
This commit fixes two problems in ssh library:
1) RetryableAuthMethod() now breaks out from the retry loop and
returns when underlying auth method fails with partial success
set to true.
2) Book keeping of tried (and failed) auth methods in
clientAuthenticate() does not mark an auth method failed if it
fails with partial success set to true.
Fixes golang/go#23461
Change-Id: Ib2e1a1d54bfe2549496199bb2f66ebbce58d130d
---
M ssh/client_auth.go
M ssh/keys_test.go
A ssh/test/multi_auth_test.go
A ssh/test/sshd_test_pw.c
M ssh/test/test_unix_test.go
5 files changed, 500 insertions(+), 57 deletions(-)
To view, visit change 88035. To unsubscribe, or for help writing mail filters, visit settings.
Han-Wen Nienhuys (Gerrit)
15 comments:
File ssh/test/multi_auth_test.go:
Patch Set #5, Line 27:
MultiAuthTestCtxmultiAuthTestContext
Patch Set #5, Line 34:
resetsince you only call this on construction, can you make
func newMultiAuthContext() (*multiAuthContext, error) {instead?
Patch Set #5, Line 41:
Fatal(fmt.Errorft.Fatalf()
drop blank line
Patch Set #5, Line 55:
[]string{},nil
Patch Set #5, Line 64:
answerscan return nil here, I think.
Patch Set #5, Line 80:
passwordCallbackyou should be able to cast ctx.passwordCallback to the function type directly,
ssh.PasswordCallback(ctx.pwCallback)
here and below.
Patch Set #5, Line 88:
"passwordCallback was unexpected called %d times"pwCb called %d times, want %d
here and below.
shouldn't this assert that the login attempt as successful? here and below.
Patch Set #5, Line 104:
RetryableAuthMethodwhy is it necessary to use RetryableAuthMethod ?
This is probably a daft question by me; if it is, there should be documentation on how to setup multiauth for the client.
same as above
all of these tests look very alike in terms of code. Can you parameterize this using t.Run()
and looping over the different multi auth configurations?
File ssh/test/test_unix_test.go:
Patch Set #5, Line 66:
Sprintfcan just use + , I think.
Patch Set #5, Line 77:
sshdTestPwSo// dynamic library to inject a custom password into sshd.
drop blank
To view, visit change 88035. To unsubscribe, or for help writing mail filters, visit settings.
Sami Pönkänen (Gerrit)
Sami Pönkänen uploaded patch set #6 to this change.
ssh: fix support for partial success authentication responses in client
The existing client side authentication does not handle correctly
the partial success flag in SSH_MSG_USERAUTH_FAILURE authentication
responses.
This commit fixes two problems in ssh library:
1) RetryableAuthMethod() now breaks out from the retry loop and
returns when underlying auth method fails with partial success
set to true.
2) Book keeping of tried (and failed) auth methods in
clientAuthenticate() does not mark an auth method failed if it
fails with partial success set to true.
Fixes golang/go#23461
Change-Id: Ib2e1a1d54bfe2549496199bb2f66ebbce58d130d
---
M ssh/client_auth.go
M ssh/keys_test.go
A ssh/test/multi_auth_test.go
A ssh/test/sshd_test_pw.c
M ssh/test/test_unix_test.go
5 files changed, 442 insertions(+), 57 deletions(-)
To view, visit change 88035. To unsubscribe, or for help writing mail filters, visit settings.
Han-Wen Nienhuys (Gerrit)
5 comments:
File ssh/test/multi_auth_test.go:
Patch Set #6, Line 29:
numPwCbsnit: pwcbs is a bit too abbreviated.
passwordCount ?
Patch Set #6, Line 32:
authMethodsthis and below is part of the test case specification, and should not be in your mutable context class.
struct multiAuthTestCase {
auth []string
numPasswd int
numKbInt int
}Patch Set #6, Line 46:
numPwCbsno need to explicitly init to 0.
Patch Set #6, Line 98:
case "password,publickey":if you store this as []string{"password","publickey"} in the test case specification, you can easily generate clientConfig.Auth programmatically.
File ssh/test/test_unix_test.go:
Patch Set #5, Line 282:
wd, _ := os.Getwd()drop blank
did you forget to post the "Done" comments?
To view, visit change 88035. To unsubscribe, or for help writing mail filters, visit settings.
Sami Pönkänen (Gerrit)
Apparently I have not saved my draft comments before, so here's the old ones too.
Sami
27 comments:
File ssh/test/multi_auth_test.go:
Patch Set #5, Line 27:
multiAuthTestCtxmultiAuthTestContext
Done
since you only call this on construction, can you make […]
Yes, makes sense.
Patch Set #5, Line 41:
Fatalf("Failed tt. […]
Done
drop blank line
Done
Patch Set #5, Line 55:
*multiAuthnil
Done
can return nil here, I think.
Yes, looks like KeyboardInteractiveChallenge.auth() handles nil when returning err != nil.
Patch Set #5, Line 80:
hTestCtx(t, "keyyou should be able to cast ctx.passwordCallback to the function type directly, […]
Done
pwCb called %d times, want %d […]
Done
Patch Set #5, Line 89:
or _, ctx := range testContexts {
shouldn't this assert that the login attempt as successful? here and below.
server.Dial() calls t.Fatalf() if ssh.NewClientConn() returns error, so auth failures are already checked there. Do you want an explicit check here?
why is it necessary to use RetryableAuthMethod ? […]
The bug in client_auth.go appears (I think) only if RetryableAuthMethod() is used. And in real world it is used in most cases where user is prompted for password, right?
same as above
Done
all of these tests look very alike in terms of code. Can you parameterize this using t.Run() […]
Done
File ssh/test/multi_auth_test.go:
Patch Set #6, Line 29:
numPwCbs
nit: pwcbs is a bit too abbreviated. […]
Ok, changed to numPasswordCbs
Patch Set #6, Line 32:
authMethods
this and below is part of the test case specification, and should not be in your mutable context cla […]
Ok, split test parameters from multiAuthTestCtx to multiAuthTestCase.
Patch Set #6, Line 46:
numPwCbsno need to explicitly init to 0.
Done
Patch Set #6, Line 98:
case "password,publickey":
if you store this as []string{"password","publickey"} in the test case specification, you can easily […]
Yes, makes sense.
Patch Set #3, Line 25:
_GNU_SOURCE
this looks considerably more complicate than the poc we got from Damien; why? Is this because of BSD […]
Yeah, maybe not using strdup() makes the code more messy. It is simpler now.
As to why implement getpwnam_r() and getspnam_r()? On linux PAM calls both getpwnam() and getpwnam_r(). I do not see any calls to getspnam() or getspnam_r(), but decided to implement then anyway as PAM source code has references to them.
Patch Set #3, Line 87:
return 1;
abort if the env vars aren't defined.
Done
Patch Set #3, Line 147:
pwdp = strdup(t
why? This never happens for our test?
Why, as in why call real_getpwnam_r(), or why to implement getpwnam_r()?
Answer to the latter is that PAM needs it on my ubuntu 16.04.
can you use […]
Ok. Changed to use strdup() everywhere where applicable.
* Pointers to real functions in libc */
static struct passwd * (*real_getpwnam)(const char *) = NULL;
st
since we generate the test password, is there a reason put a non-zero salt here?
Reason is just trying to do things the way they should be done, although I guess in this test program it would not be mandatory to do so.
Ok, I'll replace this with a zero salt.
0 , right?
Yes
File ssh/test/test_unix_test.go:
n net.Conn
}can you do somethign like […]
Yes, makes sense.
File ssh/test/test_unix_test.go:
Patch Set #5, Line 66:
ultSshd
can just use + , I think.
Done
Patch Set #5, Line 77:
sshdTestPwSo// dynamic library to inject a custom password into sshd.
Done
Patch Set #5, Line 282:
wd, _ := os.Getwd()drop blank
Done
Patch Set #5, Line 282:
wd, _ := os.Getwd()
did you forget to post the "Done" comments?
Done
To view, visit change 88035. To unsubscribe, or for help writing mail filters, visit settings.
Sami Pönkänen (Gerrit)
Sami Pönkänen uploaded patch set #7 to this change.
ssh: fix support for partial success authentication responses in client
The existing client side authentication does not handle correctly
the partial success flag in SSH_MSG_USERAUTH_FAILURE authentication
responses.
This commit fixes two problems in ssh library:
1) RetryableAuthMethod() now breaks out from the retry loop and
returns when underlying auth method fails with partial success
set to true.
2) Book keeping of tried (and failed) auth methods in
clientAuthenticate() does not mark an auth method failed if it
fails with partial success set to true.
Fixes golang/go#23461
Change-Id: Ib2e1a1d54bfe2549496199bb2f66ebbce58d130d
---
M ssh/client_auth.go
M ssh/keys_test.go
A ssh/test/multi_auth_test.go
A ssh/test/sshd_test_pw.c
M ssh/test/test_unix_test.go
5 files changed, 450 insertions(+), 57 deletions(-)
To view, visit change 88035. To unsubscribe, or for help writing mail filters, visit settings.
Han-Wen Nienhuys (Gerrit)
there are some stylistic nits, but this looks mostly OK.
Unfortunately, I can't make this work locally. Let me try this on my home machine which runs a different linux version.
Patch set 7:Run-TryBot +1Code-Review +1
1 comment:
File ssh/test/multi_auth_test.go:
Patch Set #7, Line 114:
[]ssh.AuthMethod{}nil
To view, visit change 88035. To unsubscribe, or for help writing mail filters, visit settings.
Sami Pönkänen (Gerrit)
Sami Pönkänen uploaded patch set #8 to this change.
ssh: fix support for partial success authentication responses in client
The existing client side authentication does not handle correctly
the partial success flag in SSH_MSG_USERAUTH_FAILURE authentication
responses.
This commit fixes two problems in ssh library:
1) RetryableAuthMethod() now breaks out from the retry loop and
returns when underlying auth method fails with partial success
set to true.
2) Book keeping of tried (and failed) auth methods in
clientAuthenticate() does not mark an auth method failed if it
fails with partial success set to true.
Fixes golang/go#23461
Change-Id: Ib2e1a1d54bfe2549496199bb2f66ebbce58d130d
---
M ssh/client_auth.go
M ssh/keys_test.go
A ssh/test/multi_auth_test.go
A ssh/test/sshd_test_pw.c
M ssh/test/test_unix_test.go
5 files changed, 450 insertions(+), 57 deletions(-)
To view, visit change 88035. To unsubscribe, or for help writing mail filters, visit settings.
Sami Pönkänen (Gerrit)
Done
1 comment:
To view, visit change 88035. To unsubscribe, or for help writing mail filters, visit settings.
Han-Wen Nienhuys (Gerrit)
Patch set 8:Run-TryBot +1Code-Review +2
Gobot Gobot (Gerrit)
TryBots beginning. Status page: https://farmer.golang.org/try?commit=ac9bf65e
Han-Wen Nienhuys (Gerrit)
Tested sshd test on Fedora 26.
Gobot Gobot (Gerrit)
Build is still in progress...
This change failed on windows-amd64-2016:
See https://storage.googleapis.com/go-build-log/104445e3/windows-amd64-2016_c0bb6414.log
Consult https://build.golang.org/ to see whether it's a new failure. Other builds still in progress; subsequent failure notices suppressed until final report.
Gobot Gobot (Gerrit)
2 of 7 TryBots failed:
Failed on windows-amd64-2016: https://storage.googleapis.com/go-build-log/104445e3/windows-amd64-2016_c0bb6414.log
Failed on windows-386-2008: https://storage.googleapis.com/go-build-log/104445e3/windows-386-2008_f42d1606.log
Consult https://build.golang.org/ to see whether they are new failures.
Patch set 8:TryBot-Result -1
Han-Wen Nienhuys (Gerrit)
Han-Wen Nienhuys merged this change.
ssh: fix support for partial success authentication responses in client
The existing client side authentication does not handle correctly
the partial success flag in SSH_MSG_USERAUTH_FAILURE authentication
responses.
This commit fixes two problems in ssh library:
1) RetryableAuthMethod() now breaks out from the retry loop and
returns when underlying auth method fails with partial success
set to true.
2) Book keeping of tried (and failed) auth methods in
clientAuthenticate() does not mark an auth method failed if it
fails with partial success set to true.
Fixes golang/go#23461
Change-Id: Ib2e1a1d54bfe2549496199bb2f66ebbce58d130d
Reviewed-on: https://go-review.googlesource.com/88035
Reviewed-by: Han-Wen Nienhuys <han...@google.com>
Run-TryBot: Han-Wen Nienhuys <han...@google.com>
---
M ssh/client_auth.go
M ssh/keys_test.go
A ssh/test/multi_auth_test.go
A ssh/test/sshd_test_pw.c
M ssh/test/test_unix_test.go
5 files changed, 450 insertions(+), 57 deletions(-)
diff --git a/ssh/client_auth.go b/ssh/client_auth.go
index a1252cb..5f44b77 100644
--- a/ssh/client_auth.go
+++ b/ssh/client_auth.go
@@ -11,6 +11,14 @@
"io"
)
+type authResult int
+
+const (
+ authFailure authResult = iota
+ authPartialSuccess
+ authSuccess
+)
+
// clientAuthenticate authenticates with the remote server. See RFC 4252.
func (c *connection) clientAuthenticate(config *ClientConfig) error {
// initiate user auth session
@@ -37,11 +45,12 @@
if err != nil {
return err
}
- if ok {
+ if ok == authSuccess {
// success
return nil
+ } else if ok == authFailure {
+ tried[auth.method()] = true
}
- tried[auth.method()] = true
if methods == nil {
methods = lastMethods
}
@@ -82,7 +91,7 @@
// If authentication is not successful, a []string of alternative
// method names is returned. If the slice is nil, it will be ignored
// and the previous set of possible methods will be reused.
- auth(session []byte, user string, p packetConn, rand io.Reader) (bool, []string, error)
+ auth(session []byte, user string, p packetConn, rand io.Reader) (authResult, []string, error)
// method returns the RFC 4252 method name.
method() string
@@ -91,13 +100,13 @@
// "none" authentication, RFC 4252 section 5.2.
type noneAuth int
-func (n *noneAuth) auth(session []byte, user string, c packetConn, rand io.Reader) (bool, []string, error) {
+func (n *noneAuth) auth(session []byte, user string, c packetConn, rand io.Reader) (authResult, []string, error) {
if err := c.writePacket(Marshal(&userAuthRequestMsg{
User: user,
Service: serviceSSH,
Method: "none",
})); err != nil {
- return false, nil, err
+ return authFailure, nil, err
}
return handleAuthResponse(c)
@@ -111,7 +120,7 @@
// a function call, e.g. by prompting the user.
type passwordCallback func() (password string, err error)
-func (cb passwordCallback) auth(session []byte, user string, c packetConn, rand io.Reader) (bool, []string, error) {
+func (cb passwordCallback) auth(session []byte, user string, c packetConn, rand io.Reader) (authResult, []string, error) {
type passwordAuthMsg struct {
User string `sshtype:"50"`
Service string
@@ -125,7 +134,7 @@
// The program may only find out that the user doesn't have a password
// when prompting.
if err != nil {
- return false, nil, err
+ return authFailure, nil, err
}
if err := c.writePacket(Marshal(&passwordAuthMsg{
@@ -135,7 +144,7 @@
Reply: false,
Password: pw,
})); err != nil {
- return false, nil, err
+ return authFailure, nil, err
}
return handleAuthResponse(c)
@@ -178,7 +187,7 @@
return "publickey"
}
-func (cb publicKeyCallback) auth(session []byte, user string, c packetConn, rand io.Reader) (bool, []string, error) {
+func (cb publicKeyCallback) auth(session []byte, user string, c packetConn, rand io.Reader) (authResult, []string, error) {
// Authentication is performed by sending an enquiry to test if a key is
// acceptable to the remote. If the key is acceptable, the client will
// attempt to authenticate with the valid key. If not the client will repeat
@@ -186,13 +195,13 @@
signers, err := cb()
if err != nil {
- return false, nil, err
+ return authFailure, nil, err
}
var methods []string
for _, signer := range signers {
ok, err := validateKey(signer.PublicKey(), user, c)
if err != nil {
- return false, nil, err
+ return authFailure, nil, err
}
if !ok {
continue
@@ -206,7 +215,7 @@
Method: cb.method(),
}, []byte(pub.Type()), pubKey))
if err != nil {
- return false, nil, err
+ return authFailure, nil, err
}
// manually wrap the serialized signature in a string
@@ -224,24 +233,24 @@
}
p := Marshal(&msg)
if err := c.writePacket(p); err != nil {
- return false, nil, err
+ return authFailure, nil, err
}
- var success bool
+ var success authResult
success, methods, err = handleAuthResponse(c)
if err != nil {
- return false, nil, err
+ return authFailure, nil, err
}
// If authentication succeeds or the list of available methods does not
// contain the "publickey" method, do not attempt to authenticate with any
// other keys. According to RFC 4252 Section 7, the latter can occur when
// additional authentication methods are required.
- if success || !containsMethod(methods, cb.method()) {
+ if success == authSuccess || !containsMethod(methods, cb.method()) {
return success, methods, err
}
}
- return false, methods, nil
+ return authFailure, methods, nil
}
func containsMethod(methods []string, method string) bool {
@@ -318,28 +327,31 @@
// handleAuthResponse returns whether the preceding authentication request succeeded
// along with a list of remaining authentication methods to try next and
// an error if an unexpected response was received.
-func handleAuthResponse(c packetConn) (bool, []string, error) {
+func handleAuthResponse(c packetConn) (authResult, []string, error) {
for {
packet, err := c.readPacket()
if err != nil {
- return false, nil, err
+ return authFailure, nil, err
}
switch packet[0] {
case msgUserAuthBanner:
if err := handleBannerResponse(c, packet); err != nil {
- return false, nil, err
+ return authFailure, nil, err
}
case msgUserAuthFailure:
var msg userAuthFailureMsg
if err := Unmarshal(packet, &msg); err != nil {
- return false, nil, err
+ return authFailure, nil, err
}
- return false, msg.Methods, nil
+ if msg.PartialSuccess {
+ return authPartialSuccess, msg.Methods, nil
+ }
+ return authFailure, msg.Methods, nil
case msgUserAuthSuccess:
- return true, nil, nil
+ return authSuccess, nil, nil
default:
- return false, nil, unexpectedMessageError(msgUserAuthSuccess, packet[0])
+ return authFailure, nil, unexpectedMessageError(msgUserAuthSuccess, packet[0])
}
}
}
@@ -381,7 +393,7 @@
return "keyboard-interactive"
}
-func (cb KeyboardInteractiveChallenge) auth(session []byte, user string, c packetConn, rand io.Reader) (bool, []string, error) {
+func (cb KeyboardInteractiveChallenge) auth(session []byte, user string, c packetConn, rand io.Reader) (authResult, []string, error) {
type initiateMsg struct {
User string `sshtype:"50"`
Service string
@@ -395,20 +407,20 @@
Service: serviceSSH,
Method: "keyboard-interactive",
})); err != nil {
- return false, nil, err
+ return authFailure, nil, err
}
for {
packet, err := c.readPacket()
if err != nil {
- return false, nil, err
+ return authFailure, nil, err
}
// like handleAuthResponse, but with less options.
switch packet[0] {
case msgUserAuthBanner:
if err := handleBannerResponse(c, packet); err != nil {
- return false, nil, err
+ return authFailure, nil, err
}
continue
case msgUserAuthInfoRequest:
@@ -416,18 +428,21 @@
case msgUserAuthFailure:
var msg userAuthFailureMsg
if err := Unmarshal(packet, &msg); err != nil {
- return false, nil, err
+ return authFailure, nil, err
}
- return false, msg.Methods, nil
+ if msg.PartialSuccess {
+ return authPartialSuccess, msg.Methods, nil
+ }
+ return authFailure, msg.Methods, nil
case msgUserAuthSuccess:
- return true, nil, nil
+ return authSuccess, nil, nil
default:
- return false, nil, unexpectedMessageError(msgUserAuthInfoRequest, packet[0])
+ return authFailure, nil, unexpectedMessageError(msgUserAuthInfoRequest, packet[0])
}
var msg userAuthInfoRequestMsg
if err := Unmarshal(packet, &msg); err != nil {
- return false, nil, err
+ return authFailure, nil, err
}
// Manually unpack the prompt/echo pairs.
@@ -437,7 +452,7 @@
for i := 0; i < int(msg.NumPrompts); i++ {
prompt, r, ok := parseString(rest)
if !ok || len(r) == 0 {
- return false, nil, errors.New("ssh: prompt format error")
+ return authFailure, nil, errors.New("ssh: prompt format error")
}
prompts = append(prompts, string(prompt))
echos = append(echos, r[0] != 0)
@@ -445,16 +460,16 @@
}
if len(rest) != 0 {
- return false, nil, errors.New("ssh: extra data following keyboard-interactive pairs")
+ return authFailure, nil, errors.New("ssh: extra data following keyboard-interactive pairs")
}
answers, err := cb(msg.User, msg.Instruction, prompts, echos)
if err != nil {
- return false, nil, err
+ return authFailure, nil, err
}
if len(answers) != len(prompts) {
- return false, nil, errors.New("ssh: not enough answers from keyboard-interactive callback")
+ return authFailure, nil, errors.New("ssh: not enough answers from keyboard-interactive callback")
}
responseLength := 1 + 4
for _, a := range answers {
@@ -470,7 +485,7 @@
}
if err := c.writePacket(serialized); err != nil {
- return false, nil, err
+ return authFailure, nil, err
}
}
}
@@ -480,10 +495,10 @@
maxTries int
}
-func (r *retryableAuthMethod) auth(session []byte, user string, c packetConn, rand io.Reader) (ok bool, methods []string, err error) {
+func (r *retryableAuthMethod) auth(session []byte, user string, c packetConn, rand io.Reader) (ok authResult, methods []string, err error) {
for i := 0; r.maxTries <= 0 || i < r.maxTries; i++ {
ok, methods, err = r.authMethod.auth(session, user, c, rand)
- if ok || err != nil { // either success or error terminate
+ if ok != authFailure || err != nil { // either success, partial success or error terminate
return ok, methods, err
}
}
diff --git a/ssh/keys_test.go b/ssh/keys_test.go
index 20ab954..9a90abc 100644
--- a/ssh/keys_test.go
+++ b/ssh/keys_test.go
@@ -234,7 +234,7 @@
}
}
-type authResult struct {
+type testAuthResult struct {
pubKey PublicKey
options []string
comments string
@@ -242,11 +242,11 @@
ok bool
}
-func testAuthorizedKeys(t *testing.T, authKeys []byte, expected []authResult) {
+func testAuthorizedKeys(t *testing.T, authKeys []byte, expected []testAuthResult) {
rest := authKeys
- var values []authResult
+ var values []testAuthResult
for len(rest) > 0 {
- var r authResult
+ var r testAuthResult
var err error
r.pubKey, r.comments, r.options, rest, err = ParseAuthorizedKey(rest)
r.ok = (err == nil)
@@ -264,7 +264,7 @@
pub, pubSerialized := getTestKey()
line := "ssh-rsa " + pubSerialized + " user@host"
testAuthorizedKeys(t, []byte(line),
- []authResult{
+ []testAuthResult{
{pub, nil, "user@host", "", true},
})
}
@@ -286,7 +286,7 @@
authOptions := strings.Join(authWithOptions, eol)
rest2 := strings.Join(authWithOptions[3:], eol)
rest3 := strings.Join(authWithOptions[6:], eol)
- testAuthorizedKeys(t, []byte(authOptions), []authResult{
+ testAuthorizedKeys(t, []byte(authOptions), []testAuthResult{
{pub, []string{`env="HOME=/home/root"`, "no-port-forwarding"}, "user@host", rest2, true},
{pub, []string{`env="HOME=/home/root2"`}, "user2@host2", rest3, true},
{nil, nil, "", "", false},
@@ -297,7 +297,7 @@
func TestAuthWithQuotedSpaceInEnv(t *testing.T) {
pub, pubSerialized := getTestKey()
authWithQuotedSpaceInEnv := []byte(`env="HOME=/home/root dir",no-port-forwarding ssh-rsa ` + pubSerialized + ` user@host`)
- testAuthorizedKeys(t, []byte(authWithQuotedSpaceInEnv), []authResult{
+ testAuthorizedKeys(t, []byte(authWithQuotedSpaceInEnv), []testAuthResult{
{pub, []string{`env="HOME=/home/root dir"`, "no-port-forwarding"}, "user@host", "", true},
})
}
@@ -305,7 +305,7 @@
func TestAuthWithQuotedCommaInEnv(t *testing.T) {
pub, pubSerialized := getTestKey()
authWithQuotedCommaInEnv := []byte(`env="HOME=/home/root,dir",no-port-forwarding ssh-rsa ` + pubSerialized + ` user@host`)
- testAuthorizedKeys(t, []byte(authWithQuotedCommaInEnv), []authResult{
+ testAuthorizedKeys(t, []byte(authWithQuotedCommaInEnv), []testAuthResult{
{pub, []string{`env="HOME=/home/root,dir"`, "no-port-forwarding"}, "user@host", "", true},
})
}
@@ -314,11 +314,11 @@
pub, pubSerialized := getTestKey()
authWithQuotedQuoteInEnv := []byte(`env="HOME=/home/\"root dir",no-port-forwarding` + "\t" + `ssh-rsa` + "\t" + pubSerialized + ` user@host`)
authWithDoubleQuotedQuote := []byte(`no-port-forwarding,env="HOME=/home/ \"root dir\"" ssh-rsa ` + pubSerialized + "\t" + `user@host`)
- testAuthorizedKeys(t, []byte(authWithQuotedQuoteInEnv), []authResult{
+ testAuthorizedKeys(t, []byte(authWithQuotedQuoteInEnv), []testAuthResult{
{pub, []string{`env="HOME=/home/\"root dir"`, "no-port-forwarding"}, "user@host", "", true},
})
- testAuthorizedKeys(t, []byte(authWithDoubleQuotedQuote), []authResult{
+ testAuthorizedKeys(t, []byte(authWithDoubleQuotedQuote), []testAuthResult{
{pub, []string{"no-port-forwarding", `env="HOME=/home/ \"root dir\""`}, "user@host", "", true},
})
}
@@ -327,7 +327,7 @@
_, pubSerialized := getTestKey()
authWithInvalidSpace := []byte(`env="HOME=/home/root dir", no-port-forwarding ssh-rsa ` + pubSerialized + ` user@host
#more to follow but still no valid keys`)
- testAuthorizedKeys(t, []byte(authWithInvalidSpace), []authResult{
+ testAuthorizedKeys(t, []byte(authWithInvalidSpace), []testAuthResult{
{nil, nil, "", "", false},
})
}
@@ -337,7 +337,7 @@
authWithMissingQuote := []byte(`env="HOME=/home/root,no-port-forwarding ssh-rsa ` + pubSerialized + ` user@host
env="HOME=/home/root",shared-control ssh-rsa ` + pubSerialized + ` user@host`)
- testAuthorizedKeys(t, []byte(authWithMissingQuote), []authResult{
+ testAuthorizedKeys(t, []byte(authWithMissingQuote), []testAuthResult{
{pub, []string{`env="HOME=/home/root"`, `shared-control`}, "user@host", "", true},
})
}
diff --git a/ssh/test/multi_auth_test.go b/ssh/test/multi_auth_test.go
new file mode 100644
index 0000000..16fb1f6
--- /dev/null
+++ b/ssh/test/multi_auth_test.go
@@ -0,0 +1,142 @@
+// Copyright 2017 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+// Tests for ssh client multi-auth
+//
+// These tests run a simple go ssh client against OpenSSH server
+// over unix domain sockets. The tests use multiple combinations
+// of password, keyboard-interactive and publickey authentication
+// methods.
+//
+// A wrapper library for making sshd PAM authentication use test
+// passwords is required in ./sshd_test_pw.so. If the library does
+// not exist these tests will be skipped. See compile instructions
+// (for linux) in file ./sshd_test_pw.c.
+
+package test
+
+import (
+ "fmt"
+ "strings"
+ "testing"
+
+ "golang.org/x/crypto/ssh"
+)
+
+// test cases
+type multiAuthTestCase struct {
+ authMethods []string
+ expectedPasswordCbs int
+ expectedKbdIntCbs int
+}
+
+// test context
+type multiAuthTestCtx struct {
+ password string
+ numPasswordCbs int
+ numKbdIntCbs int
+}
+
+// create test context
+func newMultiAuthTestCtx(t *testing.T) *multiAuthTestCtx {
+ password, err := randomPassword()
+ if err != nil {
+ t.Fatalf("Failed to generate random test password: %s", err.Error())
+ }
+
+ return &multiAuthTestCtx{
+ password: password,
+ }
+}
+
+// password callback
+func (ctx *multiAuthTestCtx) passwordCb() (secret string, err error) {
+ ctx.numPasswordCbs++
+ return ctx.password, nil
+}
+
+// keyboard-interactive callback
+func (ctx *multiAuthTestCtx) kbdIntCb(user, instruction string, questions []string, echos []bool) (answers []string, err error) {
+ if len(questions) == 0 {
+ return nil, nil
+ }
+
+ ctx.numKbdIntCbs++
+ if len(questions) == 1 {
+ return []string{ctx.password}, nil
+ }
+
+ return nil, fmt.Errorf("unsupported keyboard-interactive flow")
+}
+
+// TestMultiAuth runs several subtests for different combinations of password, keyboard-interactive and publickey authentication methods
+func TestMultiAuth(t *testing.T) {
+ testCases := []multiAuthTestCase{
+ // Test password,publickey authentication, assert that password callback is called 1 time
+ multiAuthTestCase{
+ authMethods: []string{"password", "publickey"},
+ expectedPasswordCbs: 1,
+ },
+ // Test keyboard-interactive,publickey authentication, assert that keyboard-interactive callback is called 1 time
+ multiAuthTestCase{
+ authMethods: []string{"keyboard-interactive", "publickey"},
+ expectedKbdIntCbs: 1,
+ },
+ // Test publickey,password authentication, assert that password callback is called 1 time
+ multiAuthTestCase{
+ authMethods: []string{"publickey", "password"},
+ expectedPasswordCbs: 1,
+ },
+ // Test publickey,keyboard-interactive authentication, assert that keyboard-interactive callback is called 1 time
+ multiAuthTestCase{
+ authMethods: []string{"publickey", "keyboard-interactive"},
+ expectedKbdIntCbs: 1,
+ },
+ // Test password,password authentication, assert that password callback is called 2 times
+ multiAuthTestCase{
+ authMethods: []string{"password", "password"},
+ expectedPasswordCbs: 2,
+ },
+ }
+
+ for _, testCase := range testCases {
+ t.Run(strings.Join(testCase.authMethods, ","), func(t *testing.T) {
+ ctx := newMultiAuthTestCtx(t)
+
+ server := newServerForConfig(t, "MultiAuth", map[string]string{"AuthMethods": strings.Join(testCase.authMethods, ",")})
+ defer server.Shutdown()
+
+ clientConfig := clientConfig()
+ server.setTestPassword(clientConfig.User, ctx.password)
+
+ publicKeyAuthMethod := clientConfig.Auth[0]
+ clientConfig.Auth = nil
+ for _, authMethod := range testCase.authMethods {
+ switch authMethod {
+ case "publickey":
+ clientConfig.Auth = append(clientConfig.Auth, publicKeyAuthMethod)
+ case "password":
+ clientConfig.Auth = append(clientConfig.Auth,
+ ssh.RetryableAuthMethod(ssh.PasswordCallback(ctx.passwordCb), 5))
+ case "keyboard-interactive":
+ clientConfig.Auth = append(clientConfig.Auth,
+ ssh.RetryableAuthMethod(ssh.KeyboardInteractive(ctx.kbdIntCb), 5))
+ default:
+ t.Fatalf("Unknown authentication method %s", authMethod)
+ }
+ }
+
+ conn := server.Dial(clientConfig)
+ defer conn.Close()
+
+ if ctx.numPasswordCbs != testCase.expectedPasswordCbs {
+ t.Fatalf("passwordCallback was called %d times, expected %d times", ctx.numPasswordCbs, testCase.expectedPasswordCbs)
+ }
+
+ if ctx.numKbdIntCbs != testCase.expectedKbdIntCbs {
+ t.Fatalf("keyboardInteractiveCallback was called %d times, expected %d times", ctx.numKbdIntCbs, testCase.expectedKbdIntCbs)
+ }
+ })
+ }
+}
diff --git a/ssh/test/sshd_test_pw.c b/ssh/test/sshd_test_pw.c
new file mode 100644
index 0000000..2794a56
--- /dev/null
+++ b/ssh/test/sshd_test_pw.c
@@ -0,0 +1,173 @@
+// Copyright 2017 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+// sshd_test_pw.c
+// Wrapper to inject test password data for sshd PAM authentication
+//
+// This wrapper implements custom versions of getpwnam, getpwnam_r,
+// getspnam and getspnam_r. These functions first call their real
+// libc versions, then check if the requested user matches test user
+// specified in env variable TEST_USER and if so replace the password
+// with crypted() value of TEST_PASSWD env variable.
+//
+// Compile:
+// gcc -Wall -shared -o sshd_test_pw.so -fPIC sshd_test_pw.c
+//
+// Compile with debug:
+// gcc -DVERBOSE -Wall -shared -o sshd_test_pw.so -fPIC sshd_test_pw.c
+//
+// Run sshd:
+// LD_PRELOAD="sshd_test_pw.so" TEST_USER="..." TEST_PASSWD="..." sshd ...
+
+// +build ignore
+
+#define _GNU_SOURCE
+#include <string.h>
+#include <pwd.h>
+#include <shadow.h>
+#include <dlfcn.h>
+#include <stdlib.h>
+#include <unistd.h>
+#include <stdio.h>
+
+#ifdef VERBOSE
+#define DEBUG(X...) fprintf(stderr, X)
+#else
+#define DEBUG(X...) while (0) { }
+#endif
+
+/* crypt() password */
+static char *
+pwhash(char *passwd) {
+ return strdup(crypt(passwd, "$6$"));
+}
+
+/* Pointers to real functions in libc */
+static struct passwd * (*real_getpwnam)(const char *) = NULL;
+static int (*real_getpwnam_r)(const char *, struct passwd *, char *, size_t, struct passwd **) = NULL;
+static struct spwd * (*real_getspnam)(const char *) = NULL;
+static int (*real_getspnam_r)(const char *, struct spwd *, char *, size_t, struct spwd **) = NULL;
+
+/* Cached test user and test password */
+static char *test_user = NULL;
+static char *test_passwd_hash = NULL;
+
+static void
+init(void) {
+ /* Fetch real libc function pointers */
+ real_getpwnam = dlsym(RTLD_NEXT, "getpwnam");
+ real_getpwnam_r = dlsym(RTLD_NEXT, "getpwnam_r");
+ real_getspnam = dlsym(RTLD_NEXT, "getspnam");
+ real_getspnam_r = dlsym(RTLD_NEXT, "getspnam_r");
+
+ /* abort if env variables are not defined */
+ if (getenv("TEST_USER") == NULL || getenv("TEST_PASSWD") == NULL) {
+ fprintf(stderr, "env variables TEST_USER and TEST_PASSWD are missing\n");
+ abort();
+ }
+
+ /* Fetch test user and test password from env */
+ test_user = strdup(getenv("TEST_USER"));
+ test_passwd_hash = pwhash(getenv("TEST_PASSWD"));
+
+ DEBUG("sshd_test_pw init():\n");
+ DEBUG("\treal_getpwnam: %p\n", real_getpwnam);
+ DEBUG("\treal_getpwnam_r: %p\n", real_getpwnam_r);
+ DEBUG("\treal_getspnam: %p\n", real_getspnam);
+ DEBUG("\treal_getspnam_r: %p\n", real_getspnam_r);
+ DEBUG("\tTEST_USER: '%s'\n", test_user);
+ DEBUG("\tTEST_PASSWD: '%s'\n", getenv("TEST_PASSWD"));
+ DEBUG("\tTEST_PASSWD_HASH: '%s'\n", test_passwd_hash);
+}
+
+static int
+is_test_user(const char *name) {
+ if (test_user != NULL && strcmp(test_user, name) == 0)
+ return 1;
+ return 0;
+}
+
+/* getpwnam */
+
+struct passwd *
+getpwnam(const char *name) {
+ struct passwd *pw;
+
+ DEBUG("sshd_test_pw getpwnam(%s)\n", name);
+
+ if (real_getpwnam == NULL)
+ init();
+ if ((pw = real_getpwnam(name)) == NULL)
+ return NULL;
+
+ if (is_test_user(name))
+ pw->pw_passwd = strdup(test_passwd_hash);
+
+ return pw;
+}
+
+/* getpwnam_r */
+
+int
+getpwnam_r(const char *name,
+ struct passwd *pwd,
+ char *buf,
+ size_t buflen,
+ struct passwd **result) {
+ int r;
+
+ DEBUG("sshd_test_pw getpwnam_r(%s)\n", name);
+
+ if (real_getpwnam_r == NULL)
+ init();
+ if ((r = real_getpwnam_r(name, pwd, buf, buflen, result)) != 0 || *result == NULL)
+ return r;
+
+ if (is_test_user(name))
+ pwd->pw_passwd = strdup(test_passwd_hash);
+
+ return 0;
+}
+
+/* getspnam */
+
+struct spwd *
+getspnam(const char *name) {
+ struct spwd *sp;
+
+ DEBUG("sshd_test_pw getspnam(%s)\n", name);
+
+ if (real_getspnam == NULL)
+ init();
+ if ((sp = real_getspnam(name)) == NULL)
+ return NULL;
+
+ if (is_test_user(name))
+ sp->sp_pwdp = strdup(test_passwd_hash);
+
+ return sp;
+}
+
+/* getspnam_r */
+
+int
+getspnam_r(const char *name,
+ struct spwd *spbuf,
+ char *buf,
+ size_t buflen,
+ struct spwd **spbufp) {
+ int r;
+
+ DEBUG("sshd_test_pw getspnam_r(%s)\n", name);
+
+ if (real_getspnam_r == NULL)
+ init();
+ if ((r = real_getspnam_r(name, spbuf, buf, buflen, spbufp)) != 0)
+ return r;
+
+ if (is_test_user(name))
+ spbuf->sp_pwdp = strdup(test_passwd_hash);
+
+ return r;
+}
diff --git a/ssh/test/test_unix_test.go b/ssh/test/test_unix_test.go
index 15b879d..3960786 100644
--- a/ssh/test/test_unix_test.go
+++ b/ssh/test/test_unix_test.go
@@ -10,6 +10,8 @@
import (
"bytes"
+ "crypto/rand"
+ "encoding/base64"
"fmt"
"io/ioutil"
"log"
@@ -25,7 +27,8 @@
"golang.org/x/crypto/ssh/testdata"
)
-const sshdConfig = `
+const (
+ defaultSshdConfig = `
Protocol 2
Banner {{.Dir}}/banner
HostKey {{.Dir}}/id_rsa
@@ -50,8 +53,17 @@
HostbasedAuthentication no
PubkeyAcceptedKeyTypes=*
`
+ multiAuthSshdConfigTail = `
+UsePAM yes
+PasswordAuthentication yes
+ChallengeResponseAuthentication yes
+AuthenticationMethods {{.AuthMethods}}
+`
+)
-var configTmpl = template.Must(template.New("").Parse(sshdConfig))
+var configTmpl = map[string]*template.Template{
+ "default": template.Must(template.New("").Parse(defaultSshdConfig)),
+ "MultiAuth": template.Must(template.New("").Parse(defaultSshdConfig + multiAuthSshdConfigTail))}
type server struct {
t *testing.T
@@ -60,6 +72,10 @@
cmd *exec.Cmd
output bytes.Buffer // holds stderr from sshd process
+ testUser string // test username for sshd
+ testPasswd string // test password for sshd
+ sshdTestPwSo string // dynamic library to inject a custom password into sshd
+
// Client half of the network connection.
clientConn net.Conn
}
@@ -186,6 +202,20 @@
s.cmd.Stdin = f
s.cmd.Stdout = f
s.cmd.Stderr = &s.output
+
+ if s.sshdTestPwSo != "" {
+ if s.testUser == "" {
+ s.t.Fatal("user missing from sshd_test_pw.so config")
+ }
+ if s.testPasswd == "" {
+ s.t.Fatal("password missing from sshd_test_pw.so config")
+ }
+ s.cmd.Env = append(os.Environ(),
+ fmt.Sprintf("LD_PRELOAD=%s", s.sshdTestPwSo),
+ fmt.Sprintf("TEST_USER=%s", s.testUser),
+ fmt.Sprintf("TEST_PASSWD=%s", s.testPasswd))
+ }
+
if err := s.cmd.Start(); err != nil {
s.t.Fail()
s.Shutdown()
@@ -236,8 +266,39 @@
}
}
+// generate random password
+func randomPassword() (string, error) {
+ b := make([]byte, 12)
+ _, err := rand.Read(b)
+ if err != nil {
+ return "", err
+ }
+ return base64.RawURLEncoding.EncodeToString(b), nil
+}
+
+// setTestPassword is used for setting user and password data for sshd_test_pw.so
+// This function also checks that ./sshd_test_pw.so exists and if not calls s.t.Skip()
+func (s *server) setTestPassword(user, passwd string) error {
+ wd, _ := os.Getwd()
+ wrapper := filepath.Join(wd, "sshd_test_pw.so")
+ if _, err := os.Stat(wrapper); err != nil {
+ s.t.Skip(fmt.Errorf("sshd_test_pw.so is not available"))
+ return err
+ }
+
+ s.sshdTestPwSo = wrapper
+ s.testUser = user
+ s.testPasswd = passwd
+ return nil
+}
+
// newServer returns a new mock ssh server.
func newServer(t *testing.T) *server {
+ return newServerForConfig(t, "default", map[string]string{})
+}
+
+// newServerForConfig returns a new mock ssh server.
+func newServerForConfig(t *testing.T, config string, configVars map[string]string) *server {
if testing.Short() {
t.Skip("skipping test due to -short")
}
@@ -249,9 +310,11 @@
if err != nil {
t.Fatal(err)
}
- err = configTmpl.Execute(f, map[string]string{
- "Dir": dir,
- })
+ if _, ok := configTmpl[config]; ok == false {
+ t.Fatal(fmt.Errorf("Invalid server config '%s'", config))
+ }
+ configVars["Dir"] = dir
+ err = configTmpl[config].Execute(f, configVars)
if err != nil {
t.Fatal(err)
}
To view, visit change 88035. To unsubscribe, or for help writing mail filters, visit settings.