ssh/tailssh: remove unused public key authentication logic

In preparation for unforking golang.org/x/crypto/ssh.

Updates #8593

Signed-off-by: Percy Wegmann <percy@tailscale.com>
This commit is contained in:
Percy Wegmann 2024-06-05 12:12:31 -05:00
parent cf1e6c6e55
commit 51f7cb0903
No known key found for this signature in database
GPG Key ID: 29D8CDEB4C13D48B
5 changed files with 30 additions and 352 deletions

View File

@ -10,7 +10,6 @@
"bytes"
"context"
"crypto/rand"
"encoding/base64"
"encoding/json"
"errors"
"fmt"
@ -80,16 +79,14 @@ type server struct {
logf logger.Logf
tailscaledPath string
pubKeyHTTPClient *http.Client // or nil for http.DefaultClient
timeNow func() time.Time // or nil for time.Now
timeNow func() time.Time // or nil for time.Now
sessionWaitGroup sync.WaitGroup
// mu protects the following
mu sync.Mutex
activeConns map[*conn]bool // set; value is always true
fetchPublicKeysCache map[string]pubKeyCacheEntry // by https URL
shutdownCalled bool
mu sync.Mutex
activeConns map[*conn]bool // set; value is always true
shutdownCalled bool
}
func (srv *server) now() time.Time {
@ -204,7 +201,6 @@ func (srv *server) OnPolicyChange() {
//
// Do the user auth
// - NoClientAuthHandler
// - PublicKeyHandler (only if NoClientAuthHandler returns errPubKeyRequired)
//
// Once auth is done, the conn can be multiplexed with multiple sessions and
// channels concurrently. At which point any of the following can be called
@ -234,10 +230,9 @@ type conn struct {
finalAction *tailcfg.SSHAction // set by doPolicyAuth or resolveNextAction
finalActionErr error // set by doPolicyAuth or resolveNextAction
info *sshConnInfo // set by setInfo
localUser *userMeta // set by doPolicyAuth
userGroupIDs []string // set by doPolicyAuth
pubKey gossh.PublicKey // set by doPolicyAuth
info *sshConnInfo // set by setInfo
localUser *userMeta // set by doPolicyAuth
userGroupIDs []string // set by doPolicyAuth
// mu protects the following fields.
//
@ -267,9 +262,6 @@ func (c *conn) isAuthorized(ctx ssh.Context) error {
action := c.currentAction
for {
if action.Accept {
if c.pubKey != nil {
metricPublicKeyAccepts.Add(1)
}
return nil
}
if action.Reject || action.HoldAndDelegate == "" {
@ -292,10 +284,6 @@ func (c *conn) isAuthorized(ctx ssh.Context) error {
// policy.
var errDenied = errors.New("ssh: access denied")
// errPubKeyRequired is returned by NoClientAuthCallback to make the client
// resort to public-key auth; not user visible.
var errPubKeyRequired = errors.New("ssh publickey required")
// NoClientAuthCallback implements gossh.NoClientAuthCallback and is called by
// the ssh.Server when the client first connects with the "none"
// authentication method.
@ -304,13 +292,13 @@ func (c *conn) isAuthorized(ctx ssh.Context) error {
// starting it afresh). It returns an error if the policy evaluation fails, or
// if the decision is "reject"
//
// It either returns nil (accept) or errPubKeyRequired or errDenied
// (reject). The errors may be wrapped.
// It either returns nil (accept) or errDenied (reject). The errors may be
// wrapped.
func (c *conn) NoClientAuthCallback(ctx ssh.Context) error {
if c.insecureSkipTailscaleAuth {
return nil
}
if err := c.doPolicyAuth(ctx, nil /* no pub key */); err != nil {
if err := c.doPolicyAuth(ctx); err != nil {
return err
}
if err := c.isAuthorized(ctx); err != nil {
@ -328,11 +316,8 @@ func (c *conn) NoClientAuthCallback(ctx ssh.Context) error {
}
func (c *conn) nextAuthMethodCallback(cm gossh.ConnMetadata, prevErrors []error) (nextMethod []string) {
switch {
case c.anyPasswordIsOkay:
if c.anyPasswordIsOkay {
nextMethod = append(nextMethod, "password")
case len(prevErrors) > 0 && prevErrors[len(prevErrors)-1] == errPubKeyRequired:
nextMethod = append(nextMethod, "publickey")
}
// The fake "tailscale" method is always appended to next so OpenSSH renders
@ -352,41 +337,19 @@ func (c *conn) fakePasswordHandler(ctx ssh.Context, password string) bool {
return c.anyPasswordIsOkay
}
// PublicKeyHandler implements ssh.PublicKeyHandler is called by the
// ssh.Server when the client presents a public key.
func (c *conn) PublicKeyHandler(ctx ssh.Context, pubKey ssh.PublicKey) error {
if err := c.doPolicyAuth(ctx, pubKey); err != nil {
// TODO(maisem/bradfitz): surface the error here.
c.logf("rejecting SSH public key %s: %v", bytes.TrimSpace(gossh.MarshalAuthorizedKey(pubKey)), err)
return err
}
if err := c.isAuthorized(ctx); err != nil {
return err
}
c.logf("accepting SSH public key %s", bytes.TrimSpace(gossh.MarshalAuthorizedKey(pubKey)))
return nil
}
// doPolicyAuth verifies that conn can proceed with the specified (optional)
// pubKey. It returns nil if the matching policy action is Accept or
// HoldAndDelegate. If pubKey is nil, there was no policy match but there is a
// policy that might match a public key it returns errPubKeyRequired. Otherwise,
// it returns errDenied.
func (c *conn) doPolicyAuth(ctx ssh.Context, pubKey ssh.PublicKey) error {
// doPolicyAuth verifies that conn can proceed. It returns nil if the matching
// policy action is Accept or HoldAndDelegate. Otherwise, it returns errDenied.
func (c *conn) doPolicyAuth(ctx ssh.Context) error {
if err := c.setInfo(ctx); err != nil {
c.logf("failed to get conninfo: %v", err)
return errDenied
}
a, localUser, err := c.evaluatePolicy(pubKey)
a, localUser, err := c.evaluatePolicy()
if err != nil {
if pubKey == nil && c.havePubKeyPolicy() {
return errPubKeyRequired
}
return fmt.Errorf("%w: %v", errDenied, err)
}
c.action0 = a
c.currentAction = a
c.pubKey = pubKey
if a.Message != "" {
if err := ctx.SendAuthBanner(a.Message); err != nil {
return fmt.Errorf("SendBanner: %w", err)
@ -446,7 +409,6 @@ func (srv *server) newConn() (*conn, error) {
ServerConfigCallback: c.ServerConfig,
NoClientAuthHandler: c.NoClientAuthCallback,
PublicKeyHandler: c.PublicKeyHandler,
PasswordHandler: c.fakePasswordHandler,
Handler: c.handleSessionPostSSHAuth,
@ -514,34 +476,6 @@ func (c *conn) mayForwardLocalPortTo(ctx ssh.Context, destinationHost string, de
return false
}
// havePubKeyPolicy reports whether any policy rule may provide access by means
// of a ssh.PublicKey.
func (c *conn) havePubKeyPolicy() bool {
if c.info == nil {
panic("havePubKeyPolicy called before setInfo")
}
// Is there any rule that looks like it'd require a public key for this
// sshUser?
pol, ok := c.sshPolicy()
if !ok {
return false
}
for _, r := range pol.Rules {
if c.ruleExpired(r) {
continue
}
if mapLocalUser(r.SSHUsers, c.info.sshUser) == "" {
continue
}
for _, p := range r.Principals {
if len(p.PubKeys) > 0 && c.principalMatchesTailscaleIdentity(p) {
return true
}
}
}
return false
}
// sshPolicy returns the SSHPolicy for current node.
// If there is no SSHPolicy in the netmap, it returns a debugPolicy
// if one is defined.
@ -618,117 +552,19 @@ func (c *conn) setInfo(ctx ssh.Context) error {
}
// evaluatePolicy returns the SSHAction and localUser after evaluating
// the SSHPolicy for this conn. The pubKey may be nil for "none" auth.
func (c *conn) evaluatePolicy(pubKey gossh.PublicKey) (_ *tailcfg.SSHAction, localUser string, _ error) {
// the SSHPolicy for this conn.
func (c *conn) evaluatePolicy() (_ *tailcfg.SSHAction, localUser string, _ error) {
pol, ok := c.sshPolicy()
if !ok {
return nil, "", fmt.Errorf("tailssh: rejecting connection; no SSH policy")
}
a, localUser, ok := c.evalSSHPolicy(pol, pubKey)
a, localUser, ok := c.evalSSHPolicy(pol)
if !ok {
return nil, "", fmt.Errorf("tailssh: rejecting connection; no matching policy")
}
return a, localUser, nil
}
// pubKeyCacheEntry is the cache value for an HTTPS URL of public keys (like
// "https://github.com/foo.keys")
type pubKeyCacheEntry struct {
lines []string
etag string // if sent by server
at time.Time
}
const (
pubKeyCacheDuration = time.Minute // how long to cache non-empty public keys
pubKeyCacheEmptyDuration = 15 * time.Second // how long to cache empty responses
)
func (srv *server) fetchPublicKeysURLCached(url string) (ce pubKeyCacheEntry, ok bool) {
srv.mu.Lock()
defer srv.mu.Unlock()
// Mostly don't care about the size of this cache. Clean rarely.
if m := srv.fetchPublicKeysCache; len(m) > 50 {
tooOld := srv.now().Add(pubKeyCacheDuration * 10)
for k, ce := range m {
if ce.at.Before(tooOld) {
delete(m, k)
}
}
}
ce, ok = srv.fetchPublicKeysCache[url]
if !ok {
return ce, false
}
maxAge := pubKeyCacheDuration
if len(ce.lines) == 0 {
maxAge = pubKeyCacheEmptyDuration
}
return ce, srv.now().Sub(ce.at) < maxAge
}
func (srv *server) pubKeyClient() *http.Client {
if srv.pubKeyHTTPClient != nil {
return srv.pubKeyHTTPClient
}
return http.DefaultClient
}
// fetchPublicKeysURL fetches the public keys from a URL. The strings are in the
// the typical public key "type base64-string [comment]" format seen at e.g.
// https://github.com/USER.keys
func (srv *server) fetchPublicKeysURL(url string) ([]string, error) {
if !strings.HasPrefix(url, "https://") {
return nil, errors.New("invalid URL scheme")
}
ce, ok := srv.fetchPublicKeysURLCached(url)
if ok {
return ce.lines, nil
}
ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
defer cancel()
req, err := http.NewRequestWithContext(ctx, "GET", url, nil)
if err != nil {
return nil, err
}
if ce.etag != "" {
req.Header.Add("If-None-Match", ce.etag)
}
res, err := srv.pubKeyClient().Do(req)
if err != nil {
return nil, err
}
defer res.Body.Close()
var lines []string
var etag string
switch res.StatusCode {
default:
err = fmt.Errorf("unexpected status %v", res.Status)
srv.logf("fetching public keys from %s: %v", url, err)
case http.StatusNotModified:
lines = ce.lines
etag = ce.etag
case http.StatusOK:
var all []byte
all, err = io.ReadAll(io.LimitReader(res.Body, 4<<10))
if s := strings.TrimSpace(string(all)); s != "" {
lines = strings.Split(s, "\n")
}
etag = res.Header.Get("Etag")
}
srv.mu.Lock()
defer srv.mu.Unlock()
mak.Set(&srv.fetchPublicKeysCache, url, pubKeyCacheEntry{
at: srv.now(),
lines: lines,
etag: etag,
})
return lines, err
}
// handleSessionPostSSHAuth runs an SSH session after the SSH-level authentication,
// but not necessarily before all the Tailscale-level extra verification has
// completed. It also handles SFTP requests.
@ -830,18 +666,6 @@ func (c *conn) expandDelegateURLLocked(actionURL string) string {
).Replace(actionURL)
}
func (c *conn) expandPublicKeyURL(pubKeyURL string) string {
if !strings.Contains(pubKeyURL, "$") {
return pubKeyURL
}
loginName := c.info.uprof.LoginName
localPart, _, _ := strings.Cut(loginName, "@")
return strings.NewReplacer(
"$LOGINNAME_EMAIL", loginName,
"$LOGINNAME_LOCALPART", localPart,
).Replace(pubKeyURL)
}
// sshSession is an accepted Tailscale SSH session.
type sshSession struct {
ssh.Session
@ -892,7 +716,7 @@ func (c *conn) newSSHSession(s ssh.Session) *sshSession {
// isStillValid reports whether the conn is still valid.
func (c *conn) isStillValid() bool {
a, localUser, err := c.evaluatePolicy(c.pubKey)
a, localUser, err := c.evaluatePolicy()
c.vlogf("stillValid: %+v %v %v", a, localUser, err)
if err != nil {
return false
@ -1275,9 +1099,9 @@ func (c *conn) ruleExpired(r *tailcfg.SSHRule) bool {
return r.RuleExpires.Before(c.srv.now())
}
func (c *conn) evalSSHPolicy(pol *tailcfg.SSHPolicy, pubKey gossh.PublicKey) (a *tailcfg.SSHAction, localUser string, ok bool) {
func (c *conn) evalSSHPolicy(pol *tailcfg.SSHPolicy) (a *tailcfg.SSHAction, localUser string, ok bool) {
for _, r := range pol.Rules {
if a, localUser, err := c.matchRule(r, pubKey); err == nil {
if a, localUser, err := c.matchRule(r); err == nil {
return a, localUser, true
}
}
@ -1294,7 +1118,7 @@ func (c *conn) evalSSHPolicy(pol *tailcfg.SSHPolicy, pubKey gossh.PublicKey) (a
errInvalidConn = errors.New("invalid connection state")
)
func (c *conn) matchRule(r *tailcfg.SSHRule, pubKey gossh.PublicKey) (a *tailcfg.SSHAction, localUser string, err error) {
func (c *conn) matchRule(r *tailcfg.SSHRule) (a *tailcfg.SSHAction, localUser string, err error) {
defer func() {
c.vlogf("matchRule(%+v): %v", r, err)
}()
@ -1324,7 +1148,7 @@ func (c *conn) matchRule(r *tailcfg.SSHRule, pubKey gossh.PublicKey) (a *tailcfg
return nil, "", errUserMatch
}
}
if ok, err := c.anyPrincipalMatches(r.Principals, pubKey); err != nil {
if ok, err := c.anyPrincipalMatches(r.Principals); err != nil {
return nil, "", err
} else if !ok {
return nil, "", errPrincipalMatch
@ -1343,30 +1167,20 @@ func mapLocalUser(ruleSSHUsers map[string]string, reqSSHUser string) (localUser
return v
}
func (c *conn) anyPrincipalMatches(ps []*tailcfg.SSHPrincipal, pubKey gossh.PublicKey) (bool, error) {
func (c *conn) anyPrincipalMatches(ps []*tailcfg.SSHPrincipal) (bool, error) {
for _, p := range ps {
if p == nil {
continue
}
if ok, err := c.principalMatches(p, pubKey); err != nil {
return false, err
} else if ok {
if c.principalMatchesTailscaleIdentity(p) {
return true, nil
}
}
return false, nil
}
func (c *conn) principalMatches(p *tailcfg.SSHPrincipal, pubKey gossh.PublicKey) (bool, error) {
if !c.principalMatchesTailscaleIdentity(p) {
return false, nil
}
return c.principalMatchesPubKey(p, pubKey)
}
// principalMatchesTailscaleIdentity reports whether one of p's four fields
// that match the Tailscale identity match (Node, NodeIP, UserLogin, Any).
// This function does not consider PubKeys.
func (c *conn) principalMatchesTailscaleIdentity(p *tailcfg.SSHPrincipal) bool {
ci := c.info
if p.Any {
@ -1386,42 +1200,6 @@ func (c *conn) principalMatchesTailscaleIdentity(p *tailcfg.SSHPrincipal) bool {
return false
}
func (c *conn) principalMatchesPubKey(p *tailcfg.SSHPrincipal, clientPubKey gossh.PublicKey) (bool, error) {
if len(p.PubKeys) == 0 {
return true, nil
}
if clientPubKey == nil {
return false, nil
}
knownKeys := p.PubKeys
if len(knownKeys) == 1 && strings.HasPrefix(knownKeys[0], "https://") {
var err error
knownKeys, err = c.srv.fetchPublicKeysURL(c.expandPublicKeyURL(knownKeys[0]))
if err != nil {
return false, err
}
}
for _, knownKey := range knownKeys {
if pubKeyMatchesAuthorizedKey(clientPubKey, knownKey) {
return true, nil
}
}
return false, nil
}
func pubKeyMatchesAuthorizedKey(pubKey ssh.PublicKey, wantKey string) bool {
wantKeyType, rest, ok := strings.Cut(wantKey, " ")
if !ok {
return false
}
if pubKey.Type() != wantKeyType {
return false
}
wantKeyB64, _, _ := strings.Cut(rest, " ")
wantKeyData, _ := base64.StdEncoding.DecodeString(wantKeyB64)
return len(wantKeyData) > 0 && bytes.Equal(pubKey.Marshal(), wantKeyData)
}
func randBytes(n int) []byte {
b := make([]byte, n)
if _, err := rand.Read(b); err != nil {
@ -1918,7 +1696,6 @@ func envEq(a, b string) bool {
var (
metricActiveSessions = clientmetric.NewGauge("ssh_active_sessions")
metricIncomingConnections = clientmetric.NewCounter("ssh_incoming_connections")
metricPublicKeyAccepts = clientmetric.NewCounter("ssh_publickey_accepts") // accepted subset of ssh_publickey_connections
metricTerminalAccept = clientmetric.NewCounter("ssh_terminalaction_accept")
metricTerminalReject = clientmetric.NewCounter("ssh_terminalaction_reject")
metricTerminalMalformed = clientmetric.NewCounter("ssh_terminalaction_malformed")

View File

@ -10,7 +10,6 @@
"context"
"crypto/ed25519"
"crypto/rand"
"crypto/sha256"
"encoding/json"
"errors"
"fmt"
@ -209,7 +208,7 @@ func TestMatchRule(t *testing.T) {
info: tt.ci,
srv: &server{logf: t.Logf},
}
got, gotUser, err := c.matchRule(tt.rule, nil)
got, gotUser, err := c.matchRule(tt.rule)
if err != tt.wantErr {
t.Errorf("err = %v; want %v", err, tt.wantErr)
}
@ -990,89 +989,6 @@ func parseEnv(out []byte) map[string]string {
return e
}
func TestPublicKeyFetching(t *testing.T) {
var reqsTotal, reqsIfNoneMatchHit, reqsIfNoneMatchMiss int32
ts := httptest.NewUnstartedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
atomic.AddInt32((&reqsTotal), 1)
etag := fmt.Sprintf("W/%q", sha256.Sum256([]byte(r.URL.Path)))
w.Header().Set("Etag", etag)
if v := r.Header.Get("If-None-Match"); v != "" {
if v == etag {
atomic.AddInt32(&reqsIfNoneMatchHit, 1)
w.WriteHeader(304)
return
}
atomic.AddInt32(&reqsIfNoneMatchMiss, 1)
}
io.WriteString(w, "foo\nbar\n"+string(r.URL.Path)+"\n")
}))
ts.StartTLS()
defer ts.Close()
keys := ts.URL
clock := &tstest.Clock{}
srv := &server{
pubKeyHTTPClient: ts.Client(),
timeNow: clock.Now,
}
for range 2 {
got, err := srv.fetchPublicKeysURL(keys + "/alice.keys")
if err != nil {
t.Fatal(err)
}
if want := []string{"foo", "bar", "/alice.keys"}; !reflect.DeepEqual(got, want) {
t.Errorf("got %q; want %q", got, want)
}
}
if got, want := atomic.LoadInt32(&reqsTotal), int32(1); got != want {
t.Errorf("got %d requests; want %d", got, want)
}
if got, want := atomic.LoadInt32(&reqsIfNoneMatchHit), int32(0); got != want {
t.Errorf("got %d etag hits; want %d", got, want)
}
clock.Advance(5 * time.Minute)
got, err := srv.fetchPublicKeysURL(keys + "/alice.keys")
if err != nil {
t.Fatal(err)
}
if want := []string{"foo", "bar", "/alice.keys"}; !reflect.DeepEqual(got, want) {
t.Errorf("got %q; want %q", got, want)
}
if got, want := atomic.LoadInt32(&reqsTotal), int32(2); got != want {
t.Errorf("got %d requests; want %d", got, want)
}
if got, want := atomic.LoadInt32(&reqsIfNoneMatchHit), int32(1); got != want {
t.Errorf("got %d etag hits; want %d", got, want)
}
if got, want := atomic.LoadInt32(&reqsIfNoneMatchMiss), int32(0); got != want {
t.Errorf("got %d etag misses; want %d", got, want)
}
}
func TestExpandPublicKeyURL(t *testing.T) {
c := &conn{
info: &sshConnInfo{
uprof: tailcfg.UserProfile{
LoginName: "bar@baz.tld",
},
},
}
if got, want := c.expandPublicKeyURL("foo"), "foo"; got != want {
t.Errorf("basic: got %q; want %q", got, want)
}
if got, want := c.expandPublicKeyURL("https://example.com/$LOGINNAME_LOCALPART.keys"), "https://example.com/bar.keys"; got != want {
t.Errorf("localpart: got %q; want %q", got, want)
}
if got, want := c.expandPublicKeyURL("https://example.com/keys?email=$LOGINNAME_EMAIL"), "https://example.com/keys?email=bar@baz.tld"; got != want {
t.Errorf("email: got %q; want %q", got, want)
}
c.info = new(sshConnInfo)
if got, want := c.expandPublicKeyURL("https://example.com/keys?email=$LOGINNAME_EMAIL"), "https://example.com/keys?email="; got != want {
t.Errorf("on empty: got %q; want %q", got, want)
}
}
func TestAcceptEnvPair(t *testing.T) {
tests := []struct {
in string

View File

@ -2403,17 +2403,6 @@ type SSHPrincipal struct {
UserLogin string `json:"userLogin,omitempty"` // email-ish: foo@example.com, bar@github
Any bool `json:"any,omitempty"` // if true, match any connection
// TODO(bradfitz): add StableUserID, once that exists
// PubKeys, if non-empty, means that this SSHPrincipal only
// matches if one of these public keys is presented by the user.
//
// As a special case, if len(PubKeys) == 1 and PubKeys[0] starts
// with "https://", then it's fetched (like https://github.com/username.keys).
// In that case, the following variable expansions are also supported
// in the URL:
// * $LOGINNAME_EMAIL ("foo@bar.com" or "foo@github")
// * $LOGINNAME_LOCALPART (the "foo" from either of the above)
PubKeys []string `json:"pubKeys,omitempty"`
}
// SSHAction is how to handle an incoming connection.

View File

@ -529,7 +529,6 @@ func (src *SSHPrincipal) Clone() *SSHPrincipal {
}
dst := new(SSHPrincipal)
*dst = *src
dst.PubKeys = append(src.PubKeys[:0:0], src.PubKeys...)
return dst
}
@ -539,7 +538,6 @@ func (src *SSHPrincipal) Clone() *SSHPrincipal {
NodeIP string
UserLogin string
Any bool
PubKeys []string
}{})
// Clone makes a deep copy of ControlDialPlan.

View File

@ -1258,11 +1258,10 @@ func (v *SSHPrincipalView) UnmarshalJSON(b []byte) error {
return nil
}
func (v SSHPrincipalView) Node() StableNodeID { return v.ж.Node }
func (v SSHPrincipalView) NodeIP() string { return v.ж.NodeIP }
func (v SSHPrincipalView) UserLogin() string { return v.ж.UserLogin }
func (v SSHPrincipalView) Any() bool { return v.ж.Any }
func (v SSHPrincipalView) PubKeys() views.Slice[string] { return views.SliceOf(v.ж.PubKeys) }
func (v SSHPrincipalView) Node() StableNodeID { return v.ж.Node }
func (v SSHPrincipalView) NodeIP() string { return v.ж.NodeIP }
func (v SSHPrincipalView) UserLogin() string { return v.ж.UserLogin }
func (v SSHPrincipalView) Any() bool { return v.ж.Any }
// A compilation failure here means this code must be regenerated, with the command at the top of this file.
var _SSHPrincipalViewNeedsRegeneration = SSHPrincipal(struct {
@ -1270,7 +1269,6 @@ func (v SSHPrincipalView) PubKeys() views.Slice[string] { return views.SliceOf(v
NodeIP string
UserLogin string
Any bool
PubKeys []string
}{})
// View returns a readonly view of ControlDialPlan.