[v2] cifscreds: add a check and warnings for session keyring problems

Commit Message

Jeff Layton July 19, 2012, 1:35 p.m. UTC

Many distros do not call into pam_keyinit to set up the session keyring
properly at login time. When cifscreds add is used in such a session,
the kernel will spawn a new session keyring in which to install the
credentials. That keyring will then go away once the cifscreds process
exits.

Check for this situation by looking to see if the session and
user-session keyrings are the same. Throw a warning if so, and add some
verbiage to the cifscreds manpage that explains the issue. Also, if
the session keyring can't be queried for any reason, then cause the
program to error out.

Cc: David Howells <[e-mail]>
Reported-by: Milan Knížek <[e-mail]>
Signed-off-by: Jeff Layton <[e-mail]>
---
 cifscreds.1   |    9 ++++++++-
 cifscreds.c   |   34 ++++++++++++++++++++++++++++++++++
 cifscreds.pod |    8 ++++++++
 3 files changed, 50 insertions(+), 1 deletion(-)

Comments

David Howells July 20, 2012, 2:19 p.m. UTC | #1

Jeff Layton <[e-mail]> wrote:

> Many distros do not call into pam_keyinit to set up the session keyring
> properly at login time. When cifscreds add is used in such a session,
> the kernel will spawn a new session keyring in which to install the
> credentials. That keyring will then go away once the cifscreds process
> exits.
> 
> Check for this situation by looking to see if the session and
> user-session keyrings are the same. Throw a warning if so, and add some
> verbiage to the cifscreds manpage that explains the issue. Also, if
> the session keyring can't be queried for any reason, then cause the
> program to error out.
> 
> Cc: David Howells <[e-mail]>
> Reported-by: Milan Knížek <[e-mail]>
> Signed-off-by: Jeff Layton <[e-mail]>

Acked-by: David Howells <[e-mail]>
--
To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Jeff Layton July 20, 2012, 6:51 p.m. UTC | #2

On Thu, 19 Jul 2012 09:35:16 -0400
Jeff Layton <[e-mail]> wrote:

> Many distros do not call into pam_keyinit to set up the session keyring
> properly at login time. When cifscreds add is used in such a session,
> the kernel will spawn a new session keyring in which to install the
> credentials. That keyring will then go away once the cifscreds process
> exits.
> 
> Check for this situation by looking to see if the session and
> user-session keyrings are the same. Throw a warning if so, and add some
> verbiage to the cifscreds manpage that explains the issue. Also, if
> the session keyring can't be queried for any reason, then cause the
> program to error out.
> 
> Cc: David Howells <[e-mail]>
> Reported-by: Milan Knížek <[e-mail]>
> Signed-off-by: Jeff Layton <[e-mail]>
> ---
>  cifscreds.1   |    9 ++++++++-
>  cifscreds.c   |   34 ++++++++++++++++++++++++++++++++++
>  cifscreds.pod |    8 ++++++++
>  3 files changed, 50 insertions(+), 1 deletion(-)
> 
> diff --git a/cifscreds.1 b/cifscreds.1
> index 44a02a2..83afae6 100644
> --- a/cifscreds.1
> +++ b/cifscreds.1
> @@ -124,7 +124,7 @@
>  .\" ========================================================================
>  .\"
>  .IX Title "CIFSCREDS 1"
> -.TH CIFSCREDS 1 "2012-01-24" "" ""
> +.TH CIFSCREDS 1 "2012-07-17" "" ""
>  .\" For nroff, turn off justification.  Always turn off hyphenation; it makes
>  .\" way too many mistakes in technical documents.
>  .if n .ad l
> @@ -186,6 +186,13 @@ different username.
>  The cifscreds utility requires a kernel built with support for the
>  \&\fBlogin\fR key type. That key type was added in v3.3 in mainline Linux
>  kernels.
> +.PP
> +Since \fBcifscreds\fR adds keys to the session keyring, it is highly
> +recommended that one use \fBpam_keyinit\fR to ensure that a session keyring
> +is established at login time.
> +.SH "SEE ALSO"
> +.IX Header "SEE ALSO"
> +\&\fIpam_keyinit\fR\|(8)
>  .SH "AUTHORS"
>  .IX Header "AUTHORS"
>  The cifscreds program was originally developed by Igor Druzhinin
> diff --git a/cifscreds.c b/cifscreds.c
> index efc76e6..bb35c02 100644
> --- a/cifscreds.c
> +++ b/cifscreds.c
> @@ -28,6 +28,7 @@
>  #include <ctype.h>
>  #include <keyutils.h>
>  #include <getopt.h>
> +#include <errno.h>
>  #include "mount.h"
>  #include "resolve_host.h"
>  #include "util.h"
> @@ -465,6 +466,36 @@ static int cifscreds_update(struct cmdarg *arg)
>  	return EXIT_SUCCESS;
>  }
>  
> +static int
> +check_session_keyring(void)
> +{
> +	key_serial_t	ses_key, uses_key;
> +
> +	ses_key = keyctl_get_keyring_ID(KEY_SPEC_SESSION_KEYRING, 0);
> +	if (ses_key == -1) {
> +		if (errno == ENOKEY)
> +			fprintf(stderr, "Error: you have no session keyring. "
> +					"Consider using pam_keyinit to "
> +					"install one.\n");
> +		else
> +			fprintf(stderr, "Error: unable to query session "
> +					"keyring: %s\n", strerror(errno));
> +		return (int)ses_key;
> +	}
> +
> +	/* A problem querying the user-session keyring isn't fatal. */
> +	uses_key = keyctl_get_keyring_ID(KEY_SPEC_USER_SESSION_KEYRING, 0);
> +	if (uses_key == -1)
> +		return 0;
> +
> +	if (ses_key == uses_key)
> +		fprintf(stderr, "Warning: you have no persistent session "
> +				"keyring. cifscreds keys will not persist "
> +				"after this process exits. See "
> +				"pam_keyinit(8).\n");
> +	return 0;
> +}
> +
>  int main(int argc, char **argv)
>  {
>  	struct command *cmd, *best;
> @@ -535,5 +566,8 @@ int main(int argc, char **argv)
>  	if (arg.user == NULL)
>  		arg.user = getusername(getuid());
>  
> +	if (check_session_keyring())
> +		return EXIT_FAILURE;
> +
>  	return best->action(&arg);
>  }
> diff --git a/cifscreds.pod b/cifscreds.pod
> index 17e453f..c3bafb5 100644
> --- a/cifscreds.pod
> +++ b/cifscreds.pod
> @@ -79,6 +79,14 @@ The cifscreds utility requires a kernel built with support for the
>  B<login> key type. That key type was added in v3.3 in mainline Linux
>  kernels.
>  
> +Since B<cifscreds> adds keys to the session keyring, it is highly
> +recommended that one use B<pam_keyinit> to ensure that a session keyring
> +is established at login time.
> +
> +=head1 SEE ALSO
> +
> +pam_keyinit(8)
> +
>  =head1 AUTHORS
>  
>  The cifscreds program was originally developed by Igor Druzhinin

Merged...

Milan Knížek Aug. 8, 2012, 5:45 p.m. UTC | #3

Jeff Layton píše v ?t 19. 07. 2012 v 09:35 -0400:
> Many distros do not call into pam_keyinit to set up the session keyring
> properly at login time. When cifscreds add is used in such a session,
> the kernel will spawn a new session keyring in which to install the
> credentials. That keyring will then go away once the cifscreds process
> exits.

How does one arrange that the session keyring is set up properly for
various login methods?

I added "session optional pam_keyinit.so force revoke"
into /etc/pam.d/login and /etc/pam.d/sshd and "cifscreds add" works fine
when logged in console (alt+f2) or via ssh.
Session Keyring
 812231719 --alswrv   1001   100  keyring: _ses
 132272983 --alswrv   1001    -1   \_ keyring: _uid.1001
1046511393 ----sw-v   1001   100   \_ logon: cifs:a:192.168.1.3


pam_keyinit.so was already in /etc/pam.d/gdm-password, though when
logged in into Xfce from GDM, then "cifscreds add" typed in
xfce4-terminal complains about non-persistent keyring.

I can see that the name of the top level keyring differs for Xfce session:
Session Keyring
 666176370 --alswrv   1001    -1  keyring: _uid_ses.1001
 132272983 --alswrv   1001    -1   \_ keyring: _uid.1001

Does anyone know if that is some GDM bug/feature and how avoid it?

Sorry if being off-topic here, though I guess it might help others who
will run into the problems with cifscreds, too.

Regards,
Milan



--
To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Jeff Layton Aug. 9, 2012, 12:19 a.m. UTC | #4

On Wed, 08 Aug 2012 19:45:33 +0200
Milan Knížek <[e-mail]> wrote:

> Jeff Layton píše v ?t 19. 07. 2012 v 09:35 -0400:
> > Many distros do not call into pam_keyinit to set up the session keyring
> > properly at login time. When cifscreds add is used in such a session,
> > the kernel will spawn a new session keyring in which to install the
> > credentials. That keyring will then go away once the cifscreds process
> > exits.
> 
> How does one arrange that the session keyring is set up properly for
> various login methods?
> 
> I added "session optional pam_keyinit.so force revoke"
> into /etc/pam.d/login and /etc/pam.d/sshd and "cifscreds add" works fine
> when logged in console (alt+f2) or via ssh.
> Session Keyring
>  812231719 --alswrv   1001   100  keyring: _ses
>  132272983 --alswrv   1001    -1   \_ keyring: _uid.1001
> 1046511393 ----sw-v   1001   100   \_ logon: cifs:a:192.168.1.3
> 
> 
> pam_keyinit.so was already in /etc/pam.d/gdm-password, though when
> logged in into Xfce from GDM, then "cifscreds add" typed in
> xfce4-terminal complains about non-persistent keyring.
> 
> I can see that the name of the top level keyring differs for Xfce session:
> Session Keyring
>  666176370 --alswrv   1001    -1  keyring: _uid_ses.1001
>  132272983 --alswrv   1001    -1   \_ keyring: _uid.1001
> 
> Does anyone know if that is some GDM bug/feature and how avoid it?
> 
> Sorry if being off-topic here, though I guess it might help others who
> will run into the problems with cifscreds, too.
> 

An excellent question. I see the same behavior on a fairly stock Fedora
17 host too. I can only assume that the actual desktop session is
ending up with a different keyring session than gdm had.

Milan Knížek Aug. 9, 2012, noon UTC | #5

Jeff Layton writes:

> On Wed, 08 Aug 2012 19:45:33 +0200
> Milan Knížek <[e-mail]> wrote:
>
> > pam_keyinit.so was already in /etc/pam.d/gdm-password, though when
> > logged in into Xfce from GDM, then "cifscreds add" typed in
> > xfce4-terminal complains about non-persistent keyring.
> >
> > I can see that the name of the top level keyring differs for Xfce session:
> > Session Keyring
> >  666176370 --alswrv   1001    -1  keyring: _uid_ses.1001
> >  132272983 --alswrv   1001    -1   \_ keyring: _uid.1001
> >
> > Does anyone know if that is some GDM bug/feature and how avoid it?
> >
> An excellent question. I see the same behavior on a fairly stock Fedora
> 17 host too. I can only assume that the actual desktop session is
> ending up with a different keyring session than gdm had.

For the sake of curiosity, when logging in remotely with x2go (based on NX),  
the session keyring is okay:
$ keyctl show
Session Keyring
 420490248 --alswrv   1000   100  keyring: _ses
 318990990 --alswrv   1000    -1   \_ keyring: _uid.1000
 909936426 --alswrv   1000   100   |   \_ user: 75fbf6399a9cf084
 909201030 --alswrv   1000   100   |   \_ user: aa10afc0620e9893
 256104206 ----sw-v   1000   100   \_ logon: cifs:a:192.168.1.3

(That is for another user, the extra keys are for ecryptfs).

Later on I will try to run startx from console as well to see if GDM is the  
suspect.

Milan
--
To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

David Howells Aug. 13, 2012, 9:21 p.m. UTC | #6

Milan Knížek <[e-mail]> wrote:

> How does one arrange that the session keyring is set up properly for
> various login methods?

These are all the places I have it:

/etc/pam.d/fingerprint-auth	session optional pam_keyinit.so revoke
/etc/pam.d/fingerprint-auth-ac	session optional pam_keyinit.so revoke
/etc/pam.d/gdm-autologin	session optional pam_keyinit.so force revoke
/etc/pam.d/gdm-fingerprint	session optional pam_keyinit.so force revoke
/etc/pam.d/gdm-password		session optional pam_keyinit.so force revoke
/etc/pam.d/gdm-smartcard	session optional pam_keyinit.so force revoke
/etc/pam.d/gdm-welcome		session optional pam_keyinit.so force revoke
/etc/pam.d/kdm			session optional pam_keyinit.so force revoke
/etc/pam.d/kdm-np		session optional pam_keyinit.so force revoke
/etc/pam.d/login		session optional pam_keyinit.so force revoke
/etc/pam.d/password-auth	session optional pam_keyinit.so revoke
/etc/pam.d/password-auth-ac	session optional pam_keyinit.so revoke
/etc/pam.d/remote		session optional pam_keyinit.so force revoke
/etc/pam.d/runuser		session optional pam_keyinit.so revoke
/etc/pam.d/runuser-l		session optional pam_keyinit.so force revoke
/etc/pam.d/smartcard-auth	session optional pam_keyinit.so revoke
/etc/pam.d/smartcard-auth-ac	session optional pam_keyinit.so revoke
/etc/pam.d/sshd			session optional pam_keyinit.so force revoke
/etc/pam.d/sudo			session optional pam_keyinit.so revoke
/etc/pam.d/sudo-i		session optional pam_keyinit.so force revoke
/etc/pam.d/su-l			session optional pam_keyinit.so force revoke
/etc/pam.d/system-auth		session optional pam_keyinit.so revoke
/etc/pam.d/system-auth-ac	session optional pam_keyinit.so revoke
/etc/pam.d/vsftpd		session optional pam_keyinit.so force revoke
/etc/pam.d/xdm			session optional pam_keyinit.so force revoke
/etc/pam.d/xserver		session optional pam_keyinit.so force revoke

> pam_keyinit.so was already in /etc/pam.d/gdm-password, though when
> logged in into Xfce from GDM, then "cifscreds add" typed in
> xfce4-terminal complains about non-persistent keyring.

What name does gdm use when logging in?  I see five different names in the
list above.

David
--
To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Milan Knížek Aug. 15, 2012, 8:39 p.m. UTC | #7

David Howells píše v Po 13. 08. 2012 v 22:21 +0100:
> Milan Knížek <[e-mail]> wrote:
> 
> > How does one arrange that the session keyring is set up properly for
> > various login methods?
> ...
> > pam_keyinit.so was already in /etc/pam.d/gdm-password, though when
> > logged in into Xfce from GDM, then "cifscreds add" typed in
> > xfce4-terminal complains about non-persistent keyring.
> 
> What name does gdm use when logging in?  I see five different names in the
> list above.
> 
In Arch, gdm-welcome is used for GDM greeter (run under user gdm) and
gdm-password for the user about to log in.

I tried to change the "optional" to "required" in gdm-password:
  session required pam_keyinit.so force revoke
and the user was not able to login then. With "optional", the user logs
in but the keyring is then probably created by some other process w/o
pam_keyinit.

With
   session optional pam_keyinit.so force revoke debug
the /var/log/gdm/:0-slave.log shows:
gdm-password][19678]: pam_keyinit(gdm-password:session): OPEN 1
gdm-password][19678]: pam_keyinit(gdm-password:session): UID:1000 [0]
GID:100 [100]
gdm-password][19678]: pam_keyinit(gdm-password:session): JOIN = -1

A bit of googling revealed some info (comment 13):
https://bugs.freedesktop.org/show_bug.cgi?id=49211

I do not understand much of the comments there and what is causing the
failure (gdm, kernel, pam_keyinit), however I can see that you - David -
got involved (comment 26) and provided a patch to kernel.

If it is related, in which version of kernel was this patch released?

My versions are: linux 3.4.8-1-ARCH, gdm 3.4.1-2


regards,
Milan


--
To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Milan Knížek Aug. 17, 2012, 3:54 p.m. UTC | #8

Milan Knížek píše v St 15. 08. 2012 v 22:39 +0200:
> David Howells píše v Po 13. 08. 2012 v 22:21 +0100:
> > Milan Knížek <[e-mail]> wrote:
> > 
> With
>    session optional pam_keyinit.so force revoke debug
> the /var/log/gdm/:0-slave.log shows:
> gdm-password][19678]: pam_keyinit(gdm-password:session): OPEN 1
> gdm-password][19678]: pam_keyinit(gdm-password:session): UID:1000 [0]
> GID:100 [100]
> gdm-password][19678]: pam_keyinit(gdm-password:session): JOIN = -1
> 
> A bit of googling revealed some info (comment 13):
> https://bugs.freedesktop.org/show_bug.cgi?id=49211
> I do not understand much of the comments there and what is causing the
> failure (gdm, kernel, pam_keyinit), however I can see that you - David
> - got involved (comment 26) and provided a patch to kernel.

Recompiling 3.4.8 kernel with the patch applied (plus another one
mentioned in that patch description) solved the problem - in Xfce
session opened from GDM the session keyring exists and "cifscreds add
server" works. (And mounting CIFS shares with multiuser option as well.)

Thanks!

regards,
Milan



--
To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Jeff Layton Aug. 17, 2012, 4:02 p.m. UTC | #9

On Fri, 17 Aug 2012 17:54:26 +0200
Milan Knížek <[e-mail]> wrote:

> Milan Knížek píše v St 15. 08. 2012 v 22:39 +0200:
> > David Howells píše v Po 13. 08. 2012 v 22:21 +0100:
> > > Milan Knížek <[e-mail]> wrote:
> > > 
> > With
> >    session optional pam_keyinit.so force revoke debug
> > the /var/log/gdm/:0-slave.log shows:
> > gdm-password][19678]: pam_keyinit(gdm-password:session): OPEN 1
> > gdm-password][19678]: pam_keyinit(gdm-password:session): UID:1000 [0]
> > GID:100 [100]
> > gdm-password][19678]: pam_keyinit(gdm-password:session): JOIN = -1
> > 
> > A bit of googling revealed some info (comment 13):
> > https://bugs.freedesktop.org/show_bug.cgi?id=49211
> > I do not understand much of the comments there and what is causing the
> > failure (gdm, kernel, pam_keyinit), however I can see that you - David
> > - got involved (comment 26) and provided a patch to kernel.
> 
> Recompiling 3.4.8 kernel with the patch applied (plus another one
> mentioned in that patch description) solved the problem - in Xfce
> session opened from GDM the session keyring exists and "cifscreds add
> server" works. (And mounting CIFS shares with multiuser option as well.)
> 
> Thanks!
> 
> regards,
> Milan
> 

Awesome! David, are there any plans to push this patch to mainline?

David Howells Aug. 17, 2012, 9:02 p.m. UTC | #10

Jeff Layton <[e-mail]> wrote:

> Awesome! David, are there any plans to push this patch to mainline?

I was hoping someone would test it, review it or comment on it, but to this
point, no one did.  Now, however, ...

David
--
To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/cifscreds.1 b/cifscreds.1
index 44a02a2..83afae6 100644
--- a/cifscreds.1
+++ b/cifscreds.1
@@ -124,7 +124,7 @@ 
 .\" ========================================================================
 .\"
 .IX Title "CIFSCREDS 1"
-.TH CIFSCREDS 1 "2012-01-24" "" ""
+.TH CIFSCREDS 1 "2012-07-17" "" ""
 .\" For nroff, turn off justification.  Always turn off hyphenation; it makes
 .\" way too many mistakes in technical documents.
 .if n .ad l
@@ -186,6 +186,13 @@  different username.
 The cifscreds utility requires a kernel built with support for the
 \&\fBlogin\fR key type. That key type was added in v3.3 in mainline Linux
 kernels.
+.PP
+Since \fBcifscreds\fR adds keys to the session keyring, it is highly
+recommended that one use \fBpam_keyinit\fR to ensure that a session keyring
+is established at login time.
+.SH "SEE ALSO"
+.IX Header "SEE ALSO"
+\&\fIpam_keyinit\fR\|(8)
 .SH "AUTHORS"
 .IX Header "AUTHORS"
 The cifscreds program was originally developed by Igor Druzhinin
diff --git a/cifscreds.c b/cifscreds.c
index efc76e6..bb35c02 100644
--- a/cifscreds.c
+++ b/cifscreds.c
@@ -28,6 +28,7 @@ 
 #include <ctype.h>
 #include <keyutils.h>
 #include <getopt.h>
+#include <errno.h>
 #include "mount.h"
 #include "resolve_host.h"
 #include "util.h"
@@ -465,6 +466,36 @@  static int cifscreds_update(struct cmdarg *arg)
 	return EXIT_SUCCESS;
 }
 
+static int
+check_session_keyring(void)
+{
+	key_serial_t	ses_key, uses_key;
+
+	ses_key = keyctl_get_keyring_ID(KEY_SPEC_SESSION_KEYRING, 0);
+	if (ses_key == -1) {
+		if (errno == ENOKEY)
+			fprintf(stderr, "Error: you have no session keyring. "
+					"Consider using pam_keyinit to "
+					"install one.\n");
+		else
+			fprintf(stderr, "Error: unable to query session "
+					"keyring: %s\n", strerror(errno));
+		return (int)ses_key;
+	}
+
+	/* A problem querying the user-session keyring isn't fatal. */
+	uses_key = keyctl_get_keyring_ID(KEY_SPEC_USER_SESSION_KEYRING, 0);
+	if (uses_key == -1)
+		return 0;
+
+	if (ses_key == uses_key)
+		fprintf(stderr, "Warning: you have no persistent session "
+				"keyring. cifscreds keys will not persist "
+				"after this process exits. See "
+				"pam_keyinit(8).\n");
+	return 0;
+}
+
 int main(int argc, char **argv)
 {
 	struct command *cmd, *best;
@@ -535,5 +566,8 @@  int main(int argc, char **argv)
 	if (arg.user == NULL)
 		arg.user = getusername(getuid());
 
+	if (check_session_keyring())
+		return EXIT_FAILURE;
+
 	return best->action(&arg);
 }
diff --git a/cifscreds.pod b/cifscreds.pod
index 17e453f..c3bafb5 100644
--- a/cifscreds.pod
+++ b/cifscreds.pod
@@ -79,6 +79,14 @@  The cifscreds utility requires a kernel built with support for the
 B<login> key type. That key type was added in v3.3 in mainline Linux
 kernels.
 
+Since B<cifscreds> adds keys to the session keyring, it is highly
+recommended that one use B<pam_keyinit> to ensure that a session keyring
+is established at login time.
+
+=head1 SEE ALSO
+
+pam_keyinit(8)
+
 =head1 AUTHORS
 
 The cifscreds program was originally developed by Igor Druzhinin