Improve principal restriction logic

README.md

@@ -66,15 +66,22 @@ Runtime configuration options:
 * `group` - string, default, `""`
   If set, the user needs to be a member of this group in order to use this module.
 
+* `no_require_user_principal` - flag, true if present
+  If set, certificates do not have to be valid for a principal matching the local user in addition
+  to one of the principals listed in `authorized_principals` or `authorized_principals_file`.
 
 Example configuration:
 
-the following looks for a certificate on $SSH_AUTH_SOCK that have been signed by user_ca. Additionally,
-the user needs to have a principal on the certificate that's listed in /etc/ssh/root_authorized_principals
+1. The following looks for a certificate on `$SSH_AUTH_SOCK` that has been signed by `user_ca`. The certificate must be valid for at least one principal that's listed in `/etc/ssh/root_authorized_principals`.
+   ```
+   auth [success=1 default=ignore] /lib/security/pam_ussh.so ca_file=/etc/ssh/user_ca authorized_principals_file=/etc/ssh/root_authorized_principals
+   ```
 
-```
-auth [success=1 default=ignore] /lib/security/pam_ussh.so ca_file=/etc/ssh/user_ca authorized_principals_file=/etc/ssh/root_authorized_principals
-```
+1. The following looks for a certificate on `$SSH_AUTH_SOCK` that has been signed by `user_ca`. The certificate must be valid for at least one principal that's listed in `/etc/ssh/root_authorized_principals`. The certificate must also be valid for a principal matching the username of the target user.
+
+   ```
+   auth [success=1 default=ignore] /lib/security/pam_ussh.so ca_file=/etc/ssh/user_ca authorized_principals_file=/etc/ssh/root_authorized_principals
+   ```
 
 FAQ:
 

pam_ussh.go

@@ -67,7 +67,12 @@ func pamLog(format string, args ...interface{}) {
 
 // authenticate validates certs loaded on the ssh-agent at the other end of
 // AuthSock.
-func authenticate(w io.Writer, uid int, username, ca string, principals map[string]struct{}) AuthResult {
+func authenticate(w io.Writer, uid int, required_principal string, ca string, principals map[string]struct{}) AuthResult {
+	if len(principals) == 0 && len(required_principal) == 0 {
+		pamLog("requesting to authenticate with no required_principal and no principals")
+		return AuthError
+	}
+
 	authSock := os.Getenv("SSH_AUTH_SOCK")
 	if authSock == "" {
 		fmt.Fprint(w, "No SSH_AUTH_SOCK")
@@ -95,8 +100,9 @@ func authenticate(w io.Writer, uid int, username, ca string, principals map[stri
 		// if we're here, we probably can't stat the socket to get the owner uid
 		// to decorate the logs, but we might be able to read the parent directory.
 		ownerUID := ownerUID(path.Dir(authSock))
-		pamLog("error opening auth sock (sock owner: %d/%s) by (caller: %d/%s)",
-			ownerUID, getUsername(ownerUID), os.Getuid(), username)
+		currentUID := os.Getuid()
+		pamLog("error opening auth sock (sock owner uid: %d/%s) by (caller: %d/%s)",
+			ownerUID, getUsername(ownerUID), currentUID, getUsername(currentUID))
 		return AuthError
 	}
 
@@ -154,11 +160,32 @@ func authenticate(w io.Writer, uid int, username, ca string, principals map[stri
 		if !ok {
 			continue
 		}
-
-		if err := c.CheckCert(username, cert); err != nil {
-			continue
+
+		certValidFor := ""
+		// Optionally, we may require that the cert be valid for a
+		// required_principal, typically the local username
+		if len(required_principal) > 0 {
+			if err := c.CheckCert(required_principal, cert); err != nil {
+				pamLog("certificate not valid for required principal")
+				continue
+			}
+			certValidFor = required_principal
 		}
 
+		// If `principals` is non-empty, the cert must be valid for at least one of `principals`
+		if len(principals) > 0 {
+			for p := range principals {
+				if err := c.CheckCert(p, cert); err != nil {
+					continue
+				}
+				certValidFor = p
+				break
+			}
+			if len(certValidFor) == 0 {
+				continue
+			}
+		}
+
 		if !c.IsUserAuthority(cert.SignatureKey) {
 			pamLog("certificate signed by unrecognized authority")
 			continue
@@ -185,7 +212,7 @@ func authenticate(w io.Writer, uid int, username, ca string, principals map[stri
 
 		if len(principals) == 0 {
 			pamLog("Authentication succeeded for %q (cert %q, %d)",
-				username, cert.ValidPrincipals[0], cert.Serial)
+				certValidFor, cert.KeyId, cert.Serial)
 			return AuthSuccess
 		}
 
@@ -222,6 +249,7 @@ func pamAuthenticate(w io.Writer, uid int, username string, argv []string) AuthR
 	userCA := defaultUserCA
 	group := defaultGroup
 	authorizedPrincipals := make(map[string]struct{})
+	required_principal := username
 
 	for _, arg := range argv {
 		opt := strings.Split(arg, "=")
@@ -243,13 +271,15 @@ func pamAuthenticate(w io.Writer, uid int, username string, argv []string) AuthR
 				return AuthError
 			}
 			authorizedPrincipals = ap
+		case "no_require_user_principal":
+			required_principal = ""
 		default:
 			pamLog("unkown option: %s\n", opt[0])
 		}
 	}
-
+	
 	if len(group) == 0 || isMemberOf(group) {
-		return authenticate(w, uid, username, userCA, authorizedPrincipals)
+		return authenticate(w, uid, required_principal, userCA, authorizedPrincipals)
 	}
 
 	return AuthSuccess

pam_ussh_test.go

@@ -57,7 +57,7 @@ func TestNoAuthSock(t *testing.T) {
 	defer os.Setenv("SSH_AUTH_SOCK", oldAgent)
 	os.Unsetenv("SSH_AUTH_SOCK")
 	b := new(bytes.Buffer)
-	require.Equal(t, AuthError, authenticate(b, 0, "", "", nil))
+	require.Equal(t, AuthError, authenticate(b, 0, "r", "", nil))
 	require.Contains(t, b.String(), "No SSH_AUTH_SOCK")
 }
 
@@ -69,7 +69,7 @@ func TestBadAuthSock(t *testing.T) {
 		defer os.Setenv("SSH_AUTH_SOCK", oldAgent)
 		os.Setenv("SSH_AUTH_SOCK", s)
 		b := new(bytes.Buffer)
-		require.Equal(t, AuthError, authenticate(b, 0, "", "", nil))
+		require.Equal(t, AuthError, authenticate(b, 0, "r", "", nil))
 		require.Contains(t, b.String(), "connect: no such file or directory")
 	})
 }
@@ -157,6 +157,21 @@ func TestPamAuthorize(t *testing.T) {
 				"group=nosuchgroup"})
 			require.Equal(t, AuthSuccess, r)
 		})
+
+		c2 := signedCert(userPub, signer, "user", []string{"group:foober"})
+		WithSSHAgent(func(a agent.Agent) {
+			a.Add(agent.AddedKey{PrivateKey: userPriv, Certificate: c2})
+
+			// test without requiring the user principal
+			r := pamAuthenticate(new(bytes.Buffer), getUID(), "foober", []string{caPamOpt, "no_require_user_principal", "authorized_principals=group:foober"})
+			require.Equal(t, AuthSuccess, r,
+				"authenticate failed but no_require_user_principal was true")
+
+			// test without requiring the user principal
+			r = pamAuthenticate(new(bytes.Buffer), getUID(), "foober", []string{caPamOpt, "authorized_principals=group:foober"})
+			require.Equal(t, AuthError, r,
+				"authenticate succeeded despite require_user_principal")
+		})
 	})
 }