Re: RFC: pam_krb5: minimum_[ug]id options
On Thu, Nov 09, 2006 at 01:18:44AM +, Shaun Amott wrote:
Thanks for reviewing the patch. Here's an updated version with your
suggestions incorporated.
Please don't remove me from Cc:. I prefer to receive directed
replies, and I didn't ask for non-directed reply via setting the
Mail-Followup-To: header like you seem to prefer. Thanks.
Below are some more comments; it's still not being perfect...
Index: pam_krb5.8
===
RCS file: /home/ncvs/src/lib/libpam/modules/pam_krb5/pam_krb5.8,v
retrieving revision 1.6
diff -u -r1.6 pam_krb5.8
--- pam_krb5.824 Nov 2001 23:41:32 - 1.6
+++ pam_krb5.89 Nov 2006 01:14:18 -
@@ -1,7 +1,7 @@
.\
.\ $Id: pam_krb5.5,v 1.5 2000/01/05 00:59:56 fcusack Exp $
.\ $FreeBSD: src/lib/libpam/modules/pam_krb5/pam_krb5.8,v 1.6 2001/11/24
23:41:32 dd Exp $
-.Dd January 15, 1999
+.Dd Thursday 09, 2006
It should be .Dd November 9, 2006.
Index: pam_krb5.c
===
RCS file: /home/ncvs/src/lib/libpam/modules/pam_krb5/pam_krb5.c,v
retrieving revision 1.23
diff -u -r1.23 pam_krb5.c
--- pam_krb5.c7 Jul 2005 14:16:38 - 1.23
+++ pam_krb5.c9 Nov 2006 01:14:19 -
@@ -88,6 +88,8 @@
#define PAM_OPT_CCACHE ccache
#define PAM_OPT_DEBUGdebug
#define PAM_OPT_FORWARDABLE forwardable
+#define PAM_OPT_MINIMUM_GID minimum_gid
+#define PAM_OPT_MINIMUM_UID minimum_uid
#define PAM_OPT_NO_CCACHEno_ccache
#define PAM_OPT_REUSE_CCACHE reuse_ccache
@@ -110,6 +112,9 @@
const char *user, *pass;
const void *sourceuser, *service;
char *principal, *princ_name, *ccache_name, luser[32], *srvdup;
+ const char *retstr;
+ uid_t minuid = 0;
+ gid_t mingid = 0;
Initializations can be done later, please see below.
retval = pam_get_user(pamh, user, USER_PROMPT);
if (retval != PAM_SUCCESS)
@@ -222,6 +227,39 @@
PAM_LOG(Done getpwnam());
+ retstr = openpam_get_option(pamh, PAM_OPT_MINIMUM_UID);
+ if (retstr != NULL) {
+ if ((minuid = (uid_t)strtoul(retstr, NULL, 10)) == 0) {
+ if (errno == ERANGE || errno == EINVAL) {
Checking for ERANGE here is pointless, as when it's set, the
return value will be ULONG_MAX and not zero.
+ PAM_LOG(Error in minimum_uid: %s,
+ strerror(errno));
+ return (PAM_SERVICE_ERR);
+ }
+ } else if (minuid UID_MAX) {
Err, you should be range checking an uncasted unsigned long
value against UID_MAX because by casting it to (uid_t) this
condition is always false. On 32-bit platforms where long
is 4 bytes it's moot anyway, but on 64-bit platforms with
8-byte longs it will make a difference. I think a correct
code would look something like this (assuming it's guaranteed
that sizeof(uid_t) = sizeof(long) ;-):
: unsigned long val;
:
: val = strtoul(retstr, NULL, 10);
: if ((val == ULONG_MAX errno == ERANGE) ||
: (val == 0 errno == EINVAL))
: /* error1 */
: else if (val UID_MAX)
: /* error2 */
: else
: uid = (uid_t)val;
+ PAM_LOG(Error in minimum_uid: invalid UID);
+ return (PAM_SERVICE_ERR);
+ }
+ }
It probably makes sense to initialize minuid = 0 only here (in
the else clause), rather than doing it in the declaration part.
+
+ retstr = openpam_get_option(pamh, PAM_OPT_MINIMUM_GID);
+ if (retstr != NULL) {
+ if ((mingid = (gid_t)strtoul(retstr, NULL, 10)) == 0) {
+ if (errno == ERANGE || errno == EINVAL) {
+ PAM_LOG(Error in minimum_gid: %s,
+ strerror(errno));
+ return (PAM_SERVICE_ERR);
+ }
+ } else if (mingid GID_MAX) {
+ PAM_LOG(Error in minimum_gid: invalid GID);
+ return (PAM_SERVICE_ERR);
+ }
+ }
+
+ if (pwd-pw_uid minuid || pwd-pw_gid mingid)
+ return (PAM_IGNORE);
Ditto for the GID code.
Cheers,
--
Ruslan Ermilov
[EMAIL PROTECTED]
FreeBSD committer
pgpfD2rHnQDuM.pgp
Description: PGP signature
Re: RFC: pam_krb5: minimum_[ug]id options
On Thu, Nov 09, 2006 at 02:36:19PM +0300, Ruslan Ermilov wrote: On Thu, Nov 09, 2006 at 01:18:44AM +, Shaun Amott wrote: Thanks for reviewing the patch. Here's an updated version with your suggestions incorporated. Please don't remove me from Cc:. I prefer to receive directed replies, and I didn't ask for non-directed reply via setting the Mail-Followup-To: header like you seem to prefer. Thanks. Actually, I prefer to receive two copies of the message too. It seems mutt defaults to adding the header... but somewhat inconsistently. Even so, being omitted from the Cc: is such a rare event that I assumed you also didn't want to be Cc'ed. diff -u -r1.6 pam_krb5.8 --- pam_krb5.8 24 Nov 2001 23:41:32 - 1.6 +++ pam_krb5.8 9 Nov 2006 01:14:18 - @@ -1,7 +1,7 @@ .\ .\ $Id: pam_krb5.5,v 1.5 2000/01/05 00:59:56 fcusack Exp $ .\ $FreeBSD: src/lib/libpam/modules/pam_krb5/pam_krb5.8,v 1.6 2001/11/24 23:41:32 dd Exp $ -.Dd January 15, 1999 +.Dd Thursday 09, 2006 It should be .Dd November 9, 2006. Thanks again for your review... and apologies for such a careless patch. I've submitted a PR with an updated version. -- Shaun Amott // PGP: 0x6B387A9A A foolish consistency is the hobgoblin of little minds. - Ralph Waldo Emerson pgp1bHmigxBAs.pgp Description: PGP signature
RFC: pam_krb5: minimum_[ug]id options
While fiddling with PAM, it came to my attention that the pam_krb5
module in some other (Linux?) PAM implementations supports, amongst
other things, a minimum_uid option. This makes it possible to skip over
Kerberos authentication for local system accounts, like so:
authrequiredpam_krb5.sono_warn minimum_uid=1000
authrequiredpam_unix.sono_warn try_first_pass
I think it'd a nice addition to our pam_krb5 at least.
I've attached an initial patch. Comments/review welcome.
Shaun
--
Shaun Amott // PGP: 0x6B387A9A
A foolish consistency is the hobgoblin
of little minds. - Ralph Waldo Emerson
Index: pam_krb5.8
===
RCS file: /home/ncvs/src/lib/libpam/modules/pam_krb5/pam_krb5.8,v
retrieving revision 1.6
diff -u -r1.6 pam_krb5.8
--- pam_krb5.8 24 Nov 2001 23:41:32 - 1.6
+++ pam_krb5.8 8 Nov 2006 20:50:35 -
@@ -108,6 +108,13 @@
.Ql %p ,
to designate the current process ID; can be used in
.Ar name .
+.It Cm minimum_uid Ns = Ns Ar id
+Do not attempt to authenticate users with a uid below
+.Ar id .
+Instead, simply return; thus allowing a later module to authenticate
+the user.
+.It Cm minimum_gid Ns = Ns Ar id
+As above, but specifies a minimum group.
.El
.Ss Kerberos 5 Account Management Module
The Kerberos 5 account management component
Index: pam_krb5.c
===
RCS file: /home/ncvs/src/lib/libpam/modules/pam_krb5/pam_krb5.c,v
retrieving revision 1.23
diff -u -r1.23 pam_krb5.c
--- pam_krb5.c 7 Jul 2005 14:16:38 - 1.23
+++ pam_krb5.c 8 Nov 2006 20:50:36 -
@@ -90,6 +90,8 @@
#define PAM_OPT_FORWARDABLEforwardable
#define PAM_OPT_NO_CCACHE no_ccache
#define PAM_OPT_REUSE_CCACHE reuse_ccache
+#define PAM_OPT_MINIMUM_UIDminimum_uid
+#define PAM_OPT_MINIMUM_GIDminimum_gid
/*
* authentication management
@@ -110,6 +112,9 @@
const char *user, *pass;
const void *sourceuser, *service;
char *principal, *princ_name, *ccache_name, luser[32], *srvdup;
+ const char *retstr;
+ uid_t minuid = 0;
+ gid_t mingid = 0;
retval = pam_get_user(pamh, user, USER_PROMPT);
if (retval != PAM_SUCCESS)
@@ -222,6 +227,21 @@
PAM_LOG(Done getpwnam());
+ retstr = openpam_get_option(pamh, PAM_OPT_MINIMUM_UID);
+
+ if (retstr)
+ minuid = (uid_t)strtoul(retstr, NULL, 10);
+
+ retstr = openpam_get_option(pamh, PAM_OPT_MINIMUM_GID);
+
+ if (retstr)
+ mingid = (gid_t)strtoul(retstr, NULL, 10);
+
+ if (pwd-pw_uid minuid || pwd-pw_gid mingid)
+ return (PAM_IGNORE);
+
+ PAM_LOG(Checked uid and gid bounds);
+
/* Get a TGT */
memset(creds, 0, sizeof(krb5_creds));
krbret = krb5_get_init_creds_password(pam_context, creds, princ,
@@ -349,6 +369,9 @@
const void *user;
void *cache_data;
char *cache_name_buf = NULL, *p;
+ const char *retstr;
+ uid_t minuid = 0;
+ gid_t mingid = 0;
uid_t euid;
gid_t egid;
@@ -391,6 +414,30 @@
PAM_LOG(Got euid, egid: %d %d, euid, egid);
+ /* Get the uid. This should exist. */
+ pwd = getpwnam(user);
+ if (pwd == NULL) {
+ retval = PAM_USER_UNKNOWN;
+ goto cleanup3;
+ }
+
+ PAM_LOG(Done getpwnam());
+
+ retstr = openpam_get_option(pamh, PAM_OPT_MINIMUM_UID);
+
+ if (retstr)
+ minuid = (uid_t)strtoul(retstr, NULL, 10);
+
+ retstr = openpam_get_option(pamh, PAM_OPT_MINIMUM_GID);
+
+ if (retstr)
+ mingid = (gid_t)strtoul(retstr, NULL, 10);
+
+ if (pwd-pw_uid minuid || pwd-pw_gid mingid)
+ return (PAM_IGNORE);
+
+ PAM_LOG(Checked uid and gid bounds);
+
/* Retrieve the temporary cache */
retval = pam_get_data(pamh, ccache, cache_data);
if (retval != PAM_SUCCESS) {
@@ -405,15 +452,6 @@
goto cleanup3;
}
- /* Get the uid. This should exist. */
- pwd = getpwnam(user);
- if (pwd == NULL) {
- retval = PAM_USER_UNKNOWN;
- goto cleanup3;
- }
-
- PAM_LOG(Done getpwnam());
-
/* Avoid following a symlink as root */
if (setegid(pwd-pw_gid)) {
retval = PAM_SERVICE_ERR;
pgpttGFuoVSpj.pgp
Description: PGP signature
Re: RFC: pam_krb5: minimum_[ug]id options
On Wed, Nov 08, 2006 at 09:28:30PM +, Shaun Amott wrote:
While fiddling with PAM, it came to my attention that the pam_krb5
module in some other (Linux?) PAM implementations supports, amongst
other things, a minimum_uid option. This makes it possible to skip over
Kerberos authentication for local system accounts, like so:
authrequiredpam_krb5.sono_warn minimum_uid=1000
authrequiredpam_unix.sono_warn try_first_pass
I think it'd a nice addition to our pam_krb5 at least.
I've attached an initial patch. Comments/review welcome.
OK.
Index: pam_krb5.8
===
RCS file: /home/ncvs/src/lib/libpam/modules/pam_krb5/pam_krb5.8,v
retrieving revision 1.6
diff -u -r1.6 pam_krb5.8
--- pam_krb5.824 Nov 2001 23:41:32 - 1.6
+++ pam_krb5.88 Nov 2006 20:50:35 -
@@ -108,6 +108,13 @@
.Ql %p ,
to designate the current process ID; can be used in
.Ar name .
+.It Cm minimum_uid Ns = Ns Ar id
+Do not attempt to authenticate users with a uid below
^^^ UID
+.Ar id .
+Instead, simply return; thus allowing a later module to authenticate
+the user.
+.It Cm minimum_gid Ns = Ns Ar id
+As above, but specifies a minimum group.
^ group ID or GID
Also, it could be explicit about this being a primary GID.
.El
.Ss Kerberos 5 Account Management Module
The Kerberos 5 account management component
Document date should be bumped (the .Dd macro).
Index: pam_krb5.c
===
RCS file: /home/ncvs/src/lib/libpam/modules/pam_krb5/pam_krb5.c,v
retrieving revision 1.23
diff -u -r1.23 pam_krb5.c
--- pam_krb5.c7 Jul 2005 14:16:38 - 1.23
+++ pam_krb5.c8 Nov 2006 20:50:36 -
@@ -90,6 +90,8 @@
#define PAM_OPT_FORWARDABLE forwardable
#define PAM_OPT_NO_CCACHEno_ccache
#define PAM_OPT_REUSE_CCACHE reuse_ccache
+#define PAM_OPT_MINIMUM_UID minimum_uid
+#define PAM_OPT_MINIMUM_GID minimum_gid
Defines were sorted alphabetically by a defined name.
/*
* authentication management
@@ -110,6 +112,9 @@
const char *user, *pass;
const void *sourceuser, *service;
char *principal, *princ_name, *ccache_name, luser[32], *srvdup;
+ const char *retstr;
+ uid_t minuid = 0;
+ gid_t mingid = 0;
retval = pam_get_user(pamh, user, USER_PROMPT);
if (retval != PAM_SUCCESS)
@@ -222,6 +227,21 @@
PAM_LOG(Done getpwnam());
+ retstr = openpam_get_option(pamh, PAM_OPT_MINIMUM_UID);
+
Extraneous empty line.
+ if (retstr)
^ missing != NULL
+ minuid = (uid_t)strtoul(retstr, NULL, 10);
Errors are silently ignored; limit (UID_MAX) isn't checked.
+
+ retstr = openpam_get_option(pamh, PAM_OPT_MINIMUM_GID);
+
+ if (retstr)
+ mingid = (gid_t)strtoul(retstr, NULL, 10);
+
Ditto but s/UID_MAX/GID_MAX/.
+ if (pwd-pw_uid minuid || pwd-pw_gid mingid)
+ return (PAM_IGNORE);
+
+ PAM_LOG(Checked uid and gid bounds);
+
/* Get a TGT */
memset(creds, 0, sizeof(krb5_creds));
krbret = krb5_get_init_creds_password(pam_context, creds, princ,
@@ -349,6 +369,9 @@
const void *user;
void *cache_data;
char *cache_name_buf = NULL, *p;
+ const char *retstr;
+ uid_t minuid = 0;
+ gid_t mingid = 0;
uid_t euid;
gid_t egid;
@@ -391,6 +414,30 @@
PAM_LOG(Got euid, egid: %d %d, euid, egid);
+ /* Get the uid. This should exist. */
+ pwd = getpwnam(user);
+ if (pwd == NULL) {
+ retval = PAM_USER_UNKNOWN;
+ goto cleanup3;
+ }
+
+ PAM_LOG(Done getpwnam());
+
+ retstr = openpam_get_option(pamh, PAM_OPT_MINIMUM_UID);
+
+ if (retstr)
+ minuid = (uid_t)strtoul(retstr, NULL, 10);
+
+ retstr = openpam_get_option(pamh, PAM_OPT_MINIMUM_GID);
+
+ if (retstr)
+ mingid = (gid_t)strtoul(retstr, NULL, 10);
+
+ if (pwd-pw_uid minuid || pwd-pw_gid mingid)
+ return (PAM_IGNORE);
+
+ PAM_LOG(Checked uid and gid bounds);
+
/* Retrieve the temporary cache */
retval = pam_get_data(pamh, ccache, cache_data);
if (retval != PAM_SUCCESS) {
@@ -405,15 +452,6 @@
goto cleanup3;
}
- /* Get the uid. This should exist. */
- pwd = getpwnam(user);
- if (pwd == NULL) {
- retval = PAM_USER_UNKNOWN;
- goto cleanup3;
- }
-
- PAM_LOG(Done getpwnam());
-
/* Avoid following a symlink as root */
if (setegid(pwd-pw_gid)) {
retval = PAM_SERVICE_ERR;
Cheers,
--
Ruslan Ermilov
[EMAIL PROTECTED]
FreeBSD committer
pgpEGzGpWOdwY.pgp
Description: PGP signature
Re: RFC: pam_krb5: minimum_[ug]id options
Thanks for reviewing the patch. Here's an updated version with your
suggestions incorporated.
Shaun
--
Shaun Amott // PGP: 0x6B387A9A
A foolish consistency is the hobgoblin
of little minds. - Ralph Waldo Emerson
Index: pam_krb5.8
===
RCS file: /home/ncvs/src/lib/libpam/modules/pam_krb5/pam_krb5.8,v
retrieving revision 1.6
diff -u -r1.6 pam_krb5.8
--- pam_krb5.8 24 Nov 2001 23:41:32 - 1.6
+++ pam_krb5.8 9 Nov 2006 01:14:18 -
@@ -1,7 +1,7 @@
.\
.\ $Id: pam_krb5.5,v 1.5 2000/01/05 00:59:56 fcusack Exp $
.\ $FreeBSD: src/lib/libpam/modules/pam_krb5/pam_krb5.8,v 1.6 2001/11/24
23:41:32 dd Exp $
-.Dd January 15, 1999
+.Dd Thursday 09, 2006
.Dt PAM_KRB5 8
.Os
.Sh NAME
@@ -108,6 +108,13 @@
.Ql %p ,
to designate the current process ID; can be used in
.Ar name .
+.It Cm minimum_uid Ns = Ns Ar id
+Do not attempt to authenticate users with a UID below
+.Ar id .
+Instead, simply return; thus allowing a later module to authenticate
+the user.
+.It Cm minimum_gid Ns = Ns Ar id
+As above, but specifies a minimum primary GID.
.El
.Ss Kerberos 5 Account Management Module
The Kerberos 5 account management component
Index: pam_krb5.c
===
RCS file: /home/ncvs/src/lib/libpam/modules/pam_krb5/pam_krb5.c,v
retrieving revision 1.23
diff -u -r1.23 pam_krb5.c
--- pam_krb5.c 7 Jul 2005 14:16:38 - 1.23
+++ pam_krb5.c 9 Nov 2006 01:14:19 -
@@ -88,6 +88,8 @@
#define PAM_OPT_CCACHE ccache
#define PAM_OPT_DEBUG debug
#define PAM_OPT_FORWARDABLEforwardable
+#define PAM_OPT_MINIMUM_GIDminimum_gid
+#define PAM_OPT_MINIMUM_UIDminimum_uid
#define PAM_OPT_NO_CCACHE no_ccache
#define PAM_OPT_REUSE_CCACHE reuse_ccache
@@ -110,6 +112,9 @@
const char *user, *pass;
const void *sourceuser, *service;
char *principal, *princ_name, *ccache_name, luser[32], *srvdup;
+ const char *retstr;
+ uid_t minuid = 0;
+ gid_t mingid = 0;
retval = pam_get_user(pamh, user, USER_PROMPT);
if (retval != PAM_SUCCESS)
@@ -222,6 +227,39 @@
PAM_LOG(Done getpwnam());
+ retstr = openpam_get_option(pamh, PAM_OPT_MINIMUM_UID);
+ if (retstr != NULL) {
+ if ((minuid = (uid_t)strtoul(retstr, NULL, 10)) == 0) {
+ if (errno == ERANGE || errno == EINVAL) {
+ PAM_LOG(Error in minimum_uid: %s,
+ strerror(errno));
+ return (PAM_SERVICE_ERR);
+ }
+ } else if (minuid UID_MAX) {
+ PAM_LOG(Error in minimum_uid: invalid UID);
+ return (PAM_SERVICE_ERR);
+ }
+ }
+
+ retstr = openpam_get_option(pamh, PAM_OPT_MINIMUM_GID);
+ if (retstr != NULL) {
+ if ((mingid = (gid_t)strtoul(retstr, NULL, 10)) == 0) {
+ if (errno == ERANGE || errno == EINVAL) {
+ PAM_LOG(Error in minimum_gid: %s,
+ strerror(errno));
+ return (PAM_SERVICE_ERR);
+ }
+ } else if (mingid GID_MAX) {
+ PAM_LOG(Error in minimum_gid: invalid GID);
+ return (PAM_SERVICE_ERR);
+ }
+ }
+
+ if (pwd-pw_uid minuid || pwd-pw_gid mingid)
+ return (PAM_IGNORE);
+
+ PAM_LOG(Checked uid and gid bounds);
+
/* Get a TGT */
memset(creds, 0, sizeof(krb5_creds));
krbret = krb5_get_init_creds_password(pam_context, creds, princ,
@@ -349,6 +387,9 @@
const void *user;
void *cache_data;
char *cache_name_buf = NULL, *p;
+ const char *retstr;
+ uid_t minuid = 0;
+ gid_t mingid = 0;
uid_t euid;
gid_t egid;
@@ -391,6 +432,48 @@
PAM_LOG(Got euid, egid: %d %d, euid, egid);
+ /* Get the uid. This should exist. */
+ pwd = getpwnam(user);
+ if (pwd == NULL) {
+ retval = PAM_USER_UNKNOWN;
+ goto cleanup3;
+ }
+
+ PAM_LOG(Done getpwnam());
+
+ retstr = openpam_get_option(pamh, PAM_OPT_MINIMUM_UID);
+ if (retstr != NULL) {
+ if ((minuid = (uid_t)strtoul(retstr, NULL, 10)) == 0) {
+ if (errno == ERANGE || errno == EINVAL) {
+ PAM_LOG(Error in minimum_uid: %s,
+ strerror(errno));
+ return (PAM_SERVICE_ERR);
+ }
+ } else if (minuid UID_MAX) {
+ PAM_LOG(Error in minimum_uid: invalid UID);
+ return (PAM_SERVICE_ERR);
+ }
+ }
+
+ retstr = openpam_get_option(pamh, PAM_OPT_MINIMUM_GID);
+
