certificate auth: fix source address checking inside crypto/ssh

So we can avoid to check source address ourself

81aafe6d26

Signed-off-by: Nicola Murino <nicola.murino@gmail.com>

go.mod

@@ -34,9 +34,9 @@ require (
 	github.com/spf13/viper v1.6.3
 	github.com/stretchr/testify v1.5.1
 	go.etcd.io/bbolt v1.3.4
-	golang.org/x/crypto v0.0.0-20200323165209-0ec3e9974c59
+	golang.org/x/crypto v0.0.0-20200510223506-06a226fb4e37
-	golang.org/x/sys v0.0.0-20200331124033-c3d80250170d
+	golang.org/x/sys v0.0.0-20200515095857-1151b9dac4a9
-	golang.org/x/tools v0.0.0-20200403170748-4480df5f1627 // indirect
+	golang.org/x/tools v0.0.0-20200515220128-d3bf790afa53 // indirect
 	google.golang.org/api v0.20.0
 	google.golang.org/genproto v0.0.0-20200403120447-c50568487044 // indirect
 	gopkg.in/ini.v1 v1.55.0 // indirect
@@ -45,5 +45,5 @@ require (
 
 replace (
 	github.com/pkg/sftp => github.com/drakkan/sftp v0.0.0-20200319122022-2fc68482d27f
-	golang.org/x/crypto => github.com/drakkan/crypto v0.0.0-20200409210311-95730af1ff98
+	golang.org/x/crypto => github.com/drakkan/crypto v0.0.0-20200516130408-81aafe6d26b9
 )

go.sum

@@ -63,8 +63,8 @@ github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c
 github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
 github.com/dgrijalva/jwt-go v3.2.0+incompatible/go.mod h1:E3ru+11k8xSBh+hMPgOLZmtrrCbhqsmaPHjLKYnJCaQ=
 github.com/dgryski/go-sip13 v0.0.0-20181026042036-e10d5fee7954/go.mod h1:vAd38F8PWV+bWy6jNmig1y/TA+kYO4g3RSRF0IAv0no=
-github.com/drakkan/crypto v0.0.0-20200409210311-95730af1ff98 h1:5yTRWoqE1GhPfctw9E0wh1d3+xHajMyhQAQuxuNqwQ8=
+github.com/drakkan/crypto v0.0.0-20200516130408-81aafe6d26b9 h1:H+Pr8rDH54EXtBEA5ejYdO9yhFC2xKlCRidiyseuIAg=
-github.com/drakkan/crypto v0.0.0-20200409210311-95730af1ff98/go.mod h1:v3bhWOXGYda7H5d2s5t9XA6th3fxW3s0MQxU1R96G/w=
+github.com/drakkan/crypto v0.0.0-20200516130408-81aafe6d26b9/go.mod h1:v3bhWOXGYda7H5d2s5t9XA6th3fxW3s0MQxU1R96G/w=
 github.com/drakkan/sftp v0.0.0-20200319122022-2fc68482d27f h1:hD7R/tb8VF5Lwj0J3PnPZwN1twzpwWS2QJQYkliJ/Jw=
 github.com/drakkan/sftp v0.0.0-20200319122022-2fc68482d27f/go.mod h1:PIrgHN0+qgDmYTNiwryjoEqmXo9tv8aMwQ//Yg1xwIs=
 github.com/eikenb/pipeat v0.0.0-20200430215831-470df5986b6d h1:8RvCRWer7TB2n+DKhW4uW15hRiqPmabSnSyYhju/Nuw=
@@ -367,8 +367,9 @@ golang.org/x/sys v0.0.0-20200202164722-d101bd2416d5/go.mod h1:h1NjWce9XRLGQEsW7w
 golang.org/x/sys v0.0.0-20200212091648-12a6c2dcc1e4/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
 golang.org/x/sys v0.0.0-20200223170610-d5e6a3e2c0ae/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
 golang.org/x/sys v0.0.0-20200323222414-85ca7c5b95cd/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
-golang.org/x/sys v0.0.0-20200331124033-c3d80250170d h1:nc5K6ox/4lTFbMVSL9WRR81ixkcwXThoiF6yf+R9scA=
 golang.org/x/sys v0.0.0-20200331124033-c3d80250170d/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
+golang.org/x/sys v0.0.0-20200515095857-1151b9dac4a9 h1:YTzHMGlqJu67/uEo1lBv0n3wBXhXNeUbB1XfN2vmTm0=
+golang.org/x/sys v0.0.0-20200515095857-1151b9dac4a9/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
 golang.org/x/text v0.0.0-20170915032832-14c0d48ead0c/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
 golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
 golang.org/x/text v0.3.1-0.20180807135948-17ff2d5776d2/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
@@ -411,7 +412,7 @@ golang.org/x/tools v0.0.0-20200207183749-b753a1ba74fa/go.mod h1:TB2adYChydJhpapK
 golang.org/x/tools v0.0.0-20200212150539-ea181f53ac56/go.mod h1:TB2adYChydJhpapKDTa4BR/hXlZSLoq2Wpct/0txZ28=
 golang.org/x/tools v0.0.0-20200224181240-023911ca70b2/go.mod h1:TB2adYChydJhpapKDTa4BR/hXlZSLoq2Wpct/0txZ28=
 golang.org/x/tools v0.0.0-20200331025713-a30bf2db82d4/go.mod h1:Sl4aGygMT6LrqrWclx+PTx3U+LnKx/seiNR+3G19Ar8=
-golang.org/x/tools v0.0.0-20200403170748-4480df5f1627/go.mod h1:EkVYQZoAsY45+roYkvgYkIh4xh/qjgUK9TdY2XT94GE=
+golang.org/x/tools v0.0.0-20200515220128-d3bf790afa53/go.mod h1:EkVYQZoAsY45+roYkvgYkIh4xh/qjgUK9TdY2XT94GE=
 golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
 golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
 golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=

sftpd/internal_test.go

@@ -1739,6 +1739,9 @@ func TestProxyProtocolVersion(t *testing.T) {
 func TestLoadHostKeys(t *testing.T) {
 	c := Configuration{}
 	c.Keys = []Key{
+		{
+			PrivateKey: ".",
+		},
 		{
 			PrivateKey: "missing file",
 		},
@@ -1761,7 +1764,7 @@ func TestLoadHostKeys(t *testing.T) {
 
 func TestCertCheckerInitErrors(t *testing.T) {
 	c := Configuration{}
-	c.TrustedUserCAKeys = append(c.TrustedUserCAKeys, "missing file")
+	c.TrustedUserCAKeys = []string{".", "missing file"}
 	err := c.initializeCertChecker("")
 	assert.Error(t, err)
 	testfile := filepath.Join(os.TempDir(), "invalidkey")

sftpd/server.go

@@ -167,7 +167,7 @@ func (c Configuration) Initialize(configDir string) error {
 		PublicKeyCallback: func(conn ssh.ConnMetadata, pubKey ssh.PublicKey) (*ssh.Permissions, error) {
 			sp, err := c.validatePublicKeyCredentials(conn, pubKey)
 			if err == ssh.ErrPartialSuccess {
-				return nil, err
+				return sp, err
 			}
 			if err != nil {
 				return nil, &authenticationError{err: fmt.Sprintf("could not validate public key credentials: %v", err)}
@@ -530,6 +530,11 @@ func (c *Configuration) checkAndLoadHostKeys(configDir string, serverConfig *ssh
 	}
 	for _, k := range c.Keys {
 		privateFile := k.PrivateKey
+		if !utils.IsFileInputValid(privateFile) {
+			logger.Warn(logSender, "", "unable to load invalid host key: %#v", privateFile)
+			logger.WarnToConsole("unable to load invalid host key: %#v", privateFile)
+			continue
+		}
 		if !filepath.IsAbs(privateFile) {
 			privateFile = filepath.Join(configDir, privateFile)
 		}
@@ -553,6 +558,11 @@ func (c *Configuration) checkAndLoadHostKeys(configDir string, serverConfig *ssh
 
 func (c *Configuration) initializeCertChecker(configDir string) error {
 	for _, keyPath := range c.TrustedUserCAKeys {
+		if !utils.IsFileInputValid(keyPath) {
+			logger.Warn(logSender, "", "unable to load invalid trusted user CA key: %#v", keyPath)
+			logger.WarnToConsole("unable to load invalid trusted user CA key: %#v", keyPath)
+			continue
+		}
 		if !filepath.IsAbs(keyPath) {
 			keyPath = filepath.Join(configDir, keyPath)
 		}
@@ -611,19 +621,12 @@ func (c Configuration) validatePublicKeyCredentials(conn ssh.ConnMetadata, pubKe
 			updateLoginMetrics(conn, method, err)
 			return nil, err
 		}
-		// we need to check source address ourself since crypto/ssh will skip this check if we return partial success
-		if cert.Permissions.CriticalOptions != nil && cert.Permissions.CriticalOptions[sourceAddressCriticalOption] != "" {
-			if err := utils.CheckSourceAddress(conn.RemoteAddr(), cert.Permissions.CriticalOptions[sourceAddressCriticalOption]); err != nil {
-				updateLoginMetrics(conn, method, err)
-				return nil, err
-			}
-		}
 		certPerm = &cert.Permissions
 	}
 	if user, keyID, err = dataprovider.CheckUserAndPubKey(dataProvider, conn.User(), pubKey.Marshal()); err == nil {
 		if user.IsPartialAuth(method) {
 			logger.Debug(logSender, connectionID, "user %#v authenticated with partial success", conn.User())
-			return nil, ssh.ErrPartialSuccess
+			return certPerm, ssh.ErrPartialSuccess
 		}
 		sshPerm, err = loginUser(user, method, keyID, conn)
 		if err == nil && certPerm != nil {

sftpd/sftpd_test.go

@@ -833,7 +833,7 @@ func TestLoginUserCert(t *testing.T) {
 	err = os.RemoveAll(user.GetHomeDir())
 	assert.NoError(t, err)
 
-	// now login with a username not in the set of valid principals for given certificate
+	// now login with a username not in the set of valid principals for the given certificate
 	u.Username += "1"
 	user, _, err = httpd.AddUser(u, http.StatusOK)
 	assert.NoError(t, err)

utils/utils.go

@@ -317,34 +317,3 @@ func CleanDirInput(dirInput string) string {
 	}
 	return filepath.Clean(dirInput)
 }
-
-// CheckSourceAddress check the source address against the one defined inside an SSH user certificate
-func CheckSourceAddress(addr net.Addr, sourceAddrs string) error {
-	if addr == nil {
-		return errors.New("ssh: no address known for client, but source-address match required")
-	}
-
-	tcpAddr, ok := addr.(*net.TCPAddr)
-	if !ok {
-		return fmt.Errorf("ssh: remote address %v is not an TCP address when checking source-address match", addr)
-	}
-
-	for _, sourceAddr := range strings.Split(sourceAddrs, ",") {
-		if allowedIP := net.ParseIP(sourceAddr); allowedIP != nil {
-			if allowedIP.Equal(tcpAddr.IP) {
-				return nil
-			}
-		} else {
-			_, ipNet, err := net.ParseCIDR(sourceAddr)
-			if err != nil {
-				return fmt.Errorf("ssh: error parsing source-address restriction %q: %v", sourceAddr, err)
-			}
-
-			if ipNet.Contains(tcpAddr.IP) {
-				return nil
-			}
-		}
-	}
-
-	return fmt.Errorf("ssh: remote address %v is not allowed because of source-address restriction", addr)
-}