ssh: reject a present but empty source-address critical option

checkSourceAddressCriticalOption treated a source-address critical
option with an empty value as if the option was absent, allowing
authentication from any address. A callback (or certificate authority)
building the value dynamically from an empty allowlist would intend to
deny all sources, not to lift the restriction. This also contradicts
the documented contract that the option is validated whenever it is
present, and differs from OpenSSH, where an empty address list matches
nothing.

Change-Id: I1c0de27cbcab8817a7ff446019c25cd19957a7d7
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/797041
LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: David Chase <drchase@google.com>
Reviewed-by: Junyang Shao <shaojunyang@google.com>
Reviewed-by: Filippo Valsorda <filippo@golang.org>

ssh/server.go

@@ -476,12 +476,14 @@ func checkSourceAddress(addr net.Addr, sourceAddrs string) error {
 
 // checkSourceAddressCriticalOption enforces the source-address critical
 // option, if present in perms, as documented in Permissions.CriticalOptions.
+// A present but empty value matches no address, so it denies authentication,
+// consistently with OpenSSH, rather than being treated as absent.
 func checkSourceAddressCriticalOption(addr net.Addr, perms *Permissions) error {
 	if perms == nil {
 		return nil
 	}
-	saco := perms.CriticalOptions[sourceAddressCriticalOption]
-	if saco == "" {
+	saco, ok := perms.CriticalOptions[sourceAddressCriticalOption]
+	if !ok {
 		return nil
 	}
 	return checkSourceAddress(addr, saco)

ssh/server_test.go

@@ -819,6 +819,7 @@ func TestAuthCallbacksSourceAddress(t *testing.T) {
 		}{
 			{"mismatching", "192.168.99.99", true},
 			{"matching", "127.0.0.0/8,::1/128", false},
+			{"present but empty", "", true},
 		} {
 			t.Run(method.name+"/"+tc.name, func(t *testing.T) {
 				clientConf := &ClientConfig{