Browse Source

go.crypto/ssh: use permissions from public key cache when accepting a key.

Fixes golang/go#7913.

LGTM=hanwen
R=hanwen
CC=golang-codereviews
https://golang.org/cl/96220043
Adam Langley 11 years ago
parent
commit
2dfe547928
2 changed files with 56 additions and 13 deletions
  1. 48 0
      ssh/client_auth_test.go
  2. 8 13
      ssh/server.go

+ 48 - 0
ssh/client_auth_test.go

@@ -343,3 +343,51 @@ func TestClientLoginCert(t *testing.T) {
 		t.Errorf("cert login with source-address succeeded")
 	}
 }
+
+func testPermissionsPassing(withPermissions bool, t *testing.T) {
+	serverConfig := &ServerConfig{
+		PublicKeyCallback: func(conn ConnMetadata, key PublicKey) (*Permissions, error) {
+			if conn.User() == "nopermissions" {
+				return nil, nil
+			} else {
+				return &Permissions{}, nil
+			}
+		},
+	}
+	serverConfig.AddHostKey(testSigners["rsa"])
+
+	clientConfig := &ClientConfig{
+		Auth: []AuthMethod{
+			PublicKeys(testSigners["rsa"]),
+		},
+	}
+	if withPermissions {
+		clientConfig.User = "permissions"
+	} else {
+		clientConfig.User = "nopermissions"
+	}
+
+	c1, c2, err := netPipe()
+	if err != nil {
+		t.Fatalf("netPipe: %v", err)
+	}
+	defer c1.Close()
+	defer c2.Close()
+
+	go NewClientConn(c2, "", clientConfig)
+	serverConn, err := newServer(c1, serverConfig)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if p := serverConn.Permissions; (p != nil) != withPermissions {
+		t.Fatalf("withPermissions is %t, but Permissions object is %#v", withPermissions, p)
+	}
+}
+
+func TestPermissionsPassing(t *testing.T) {
+	testPermissionsPassing(true, t)
+}
+
+func TestNoPermissionsPassing(t *testing.T) {
+	testPermissionsPassing(false, t)
+}

+ 8 - 13
ssh/server.go

@@ -90,10 +90,6 @@ type cachedPubKey struct {
 	perms      *Permissions
 }
 
-func (k1 *cachedPubKey) Equal(k2 *cachedPubKey) bool {
-	return k1.user == k2.user && bytes.Equal(k1.pubKeyData, k2.pubKeyData)
-}
-
 const maxCachedPubKeys = 16
 
 // pubKeyCache caches tests for public keys.  Since SSH clients
@@ -105,13 +101,13 @@ type pubKeyCache struct {
 }
 
 // get returns the result for a given user/algo/key tuple.
-func (c *pubKeyCache) get(candidate cachedPubKey) (result error, ok bool) {
+func (c *pubKeyCache) get(user string, pubKeyData []byte) (cachedPubKey, bool) {
 	for _, k := range c.keys {
-		if k.Equal(&candidate) {
-			return k.result, true
+		if k.user == user && bytes.Equal(k.pubKeyData, pubKeyData) {
+			return k, true
 		}
 	}
-	return errors.New("ssh: not in cache"), false
+	return cachedPubKey{}, false
 }
 
 // add adds the given tuple to the cache.
@@ -333,12 +329,11 @@ userAuthLoop:
 			if err != nil {
 				return nil, err
 			}
-			candidate := cachedPubKey{
-				user:       s.user,
-				pubKeyData: pubKeyData,
-			}
-			candidate.result, ok = cache.get(candidate)
+
+			candidate, ok := cache.get(s.user, pubKeyData)
 			if !ok {
+				candidate.user = s.user
+				candidate.pubKeyData = pubKeyData
 				candidate.perms, candidate.result = config.PublicKeyCallback(s, pubKey)
 				if candidate.result == nil && candidate.perms != nil && candidate.perms.CriticalOptions != nil && candidate.perms.CriticalOptions[sourceAddressCriticalOption] != "" {
 					candidate.result = checkSourceAddress(