|
| 1 | +From 66fd5d19c5ea8c7f4f7ff69bcc93a7c8231ce4cf Mon Sep 17 00:00:00 2001 |
| 2 | +From: Roland Shoemaker < [email protected]> |
| 3 | +Date: Tue, 3 Dec 2024 09:03:03 -0800 |
| 4 | +Subject: [PATCH] ssh: make the public key cache a 1-entry FIFO cache |
| 5 | + |
| 6 | +Users of the the ssh package seem to extremely commonly misuse the |
| 7 | +PublicKeyCallback API, assuming that the key passed in the last call |
| 8 | +before a connection is established is the key used for authentication. |
| 9 | +Some users then make authorization decisions based on this key. This |
| 10 | +property is not documented, and may not be correct, due to the caching |
| 11 | +behavior of the package, resulting in users making incorrect |
| 12 | +authorization decisions about the connection. |
| 13 | + |
| 14 | +This change makes the cache a one entry FIFO cache, making the assumed |
| 15 | +property, that the last call to PublicKeyCallback represents the key |
| 16 | +actually used for authentication, actually hold. |
| 17 | + |
| 18 | +Thanks to Damien Tournoud, Patrick Dawkins, Vince Parker, and |
| 19 | +Jules Duvivier from the Platform.sh / Upsun engineering team |
| 20 | +for reporting this issue. |
| 21 | + |
| 22 | +Fixes golang/go#70779 |
| 23 | +Fixes CVE-2024-45337 |
| 24 | + |
| 25 | +Change-Id: Ife7c7b4045d8b6bcd7e3a417bdfae370c709797f |
| 26 | +Reviewed-on: https://go-review.googlesource.com/c/crypto/+/635315 |
| 27 | +Reviewed-by: Roland Shoemaker < [email protected]> |
| 28 | +Auto-Submit: Gopher Robot < [email protected]> |
| 29 | +Reviewed-by: Damien Neil < [email protected]> |
| 30 | +Reviewed-by: Nicola Murino < [email protected]> |
| 31 | +LUCI-TryBot-Result: Go LUCI < [email protected]> |
| 32 | +Signed-off-by: Muhammad Falak R Wani < [email protected]> |
| 33 | +--- |
| 34 | + vendor/golang.org/x/crypto/ssh/server.go | 15 +++++++++++---- |
| 35 | + 1 file changed, 11 insertions(+), 4 deletions(-) |
| 36 | + |
| 37 | +diff --git a/vendor/golang.org/x/crypto/ssh/server.go b/vendor/golang.org/x/crypto/ssh/server.go |
| 38 | +index c2dfe32..39dcc09 100644 |
| 39 | +--- a/vendor/golang.org/x/crypto/ssh/server.go |
| 40 | ++++ b/vendor/golang.org/x/crypto/ssh/server.go |
| 41 | +@@ -149,7 +149,7 @@ func (s *ServerConfig) AddHostKey(key Signer) { |
| 42 | + } |
| 43 | + |
| 44 | + // cachedPubKey contains the results of querying whether a public key is |
| 45 | +-// acceptable for a user. |
| 46 | ++// acceptable for a user. This is a FIFO cache. |
| 47 | + type cachedPubKey struct { |
| 48 | + user string |
| 49 | + pubKeyData []byte |
| 50 | +@@ -157,7 +157,13 @@ type cachedPubKey struct { |
| 51 | + perms *Permissions |
| 52 | + } |
| 53 | + |
| 54 | +-const maxCachedPubKeys = 16 |
| 55 | ++// maxCachedPubKeys is the number of cache entries we store. |
| 56 | ++// |
| 57 | ++// Due to consistent misuse of the PublicKeyCallback API, we have reduced this |
| 58 | ++// to 1, such that the only key in the cache is the most recently seen one. This |
| 59 | ++// forces the behavior that the last call to PublicKeyCallback will always be |
| 60 | ++// with the key that is used for authentication. |
| 61 | ++const maxCachedPubKeys = 1 |
| 62 | + |
| 63 | + // pubKeyCache caches tests for public keys. Since SSH clients |
| 64 | + // will query whether a public key is acceptable before attempting to |
| 65 | +@@ -179,9 +185,10 @@ func (c *pubKeyCache) get(user string, pubKeyData []byte) (cachedPubKey, bool) { |
| 66 | + |
| 67 | + // add adds the given tuple to the cache. |
| 68 | + func (c *pubKeyCache) add(candidate cachedPubKey) { |
| 69 | +- if len(c.keys) < maxCachedPubKeys { |
| 70 | +- c.keys = append(c.keys, candidate) |
| 71 | ++ if len(c.keys) >= maxCachedPubKeys { |
| 72 | ++ c.keys = c.keys[1:] |
| 73 | + } |
| 74 | ++ c.keys = append(c.keys, candidate) |
| 75 | + } |
| 76 | + |
| 77 | + // ServerConn is an authenticated SSH connection, as seen from the |
| 78 | +-- |
| 79 | +2.34.1 |
| 80 | + |
0 commit comments