From 0814f928bdaabe4ed3b4103bd0ccaf55be168358 Mon Sep 17 00:00:00 2001 From: Uriel Corfa Date: Sun, 18 Sep 2022 14:12:20 +0200 Subject: [PATCH] Make optional requiring that certs contain a principal matching the local username. --- README.md | 17 ++++++++++++----- pam_ussh.go | 48 +++++++++++++++++++++++++++++++++++++++--------- pam_ussh_test.go | 19 +++++++++++++++++-- 3 files changed, 68 insertions(+), 16 deletions(-) diff --git a/README.md b/README.md index e346dc1..f2d4f3e 100644 --- a/README.md +++ b/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: diff --git a/pam_ussh.go b/pam_ussh.go index 11ce4d0..601beea 100644 --- a/pam_ussh.go +++ b/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 diff --git a/pam_ussh_test.go b/pam_ussh_test.go index 50decde..f8bb272 100644 --- a/pam_ussh_test.go +++ b/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") + }) }) }