Pluggable Auth Modules

I think pam_lastlog could create /var/log/lastlog, but ask the user to
type "yes" to a conv like:
"/var/log/lastlog does not exist on your system. If you have not erased
it, please make sure you investigate why. Type "yes" to login."


Thorsten Kukuk wrote:
> Hi,
>
> if /var/log/lastlog is missing (for exmaple a clever admin deleted
> all files in /var/log), pam_lastlog denies logins because it cannot
> open the old file.
>
> Should pam_lastlog.so really return PAM_SERVICE_ERR in that case?
> Wouldn't it be better, if pam_lastlog creates /var/log/lastlog if
> this file does not exist?
>
>   Thorsten
>
>

On Thu, 2006-08-24 at 15:40 +0200, Thorsten Kukuk wrote:
> Hi,
> 
> if /var/log/lastlog is missing (for exmaple a clever admin deleted
> all files in /var/log), pam_lastlog denies logins because it cannot
> open the old file.
> 
> Should pam_lastlog.so really return PAM_SERVICE_ERR in that case?
> Wouldn't it be better, if pam_lastlog creates /var/log/lastlog if
> this file does not exist?
Probably yes with an appropriate message to syslog. I don't think that
conversation with user in this case is necessary or even useful.

-- 
Tomas Mraz
No matter how far down the wrong road you've gone, turn back.
                                              Turkish proverb

On Thu, Aug 24, Tomas Mraz wrote:

> On Thu, 2006-08-24 at 15:40 +0200, Thorsten Kukuk wrote:
> > Hi,
> > 
> > if /var/log/lastlog is missing (for exmaple a clever admin deleted
> > all files in /var/log), pam_lastlog denies logins because it cannot
> > open the old file.
> > 
> > Should pam_lastlog.so really return PAM_SERVICE_ERR in that case?
> > Wouldn't it be better, if pam_lastlog creates /var/log/lastlog if
> > this file does not exist?
> Probably yes with an appropriate message to syslog. I don't think that
> conversation with user in this case is necessary or even useful.

I agree. I will prepare a patch.

-- 
Thorsten Kukuk         http://www.suse.de/~kukuk/      kukuk@...
SUSE LINUX Products GmbH       Maxfeldstr. 5       D-90409 Nuernberg
--------------------------------------------------------------------    
Key fingerprint = 8C6B FD92 EE0F 42ED F91A  6A73 6D1A 7F05 2E59 24BB

Hi,

here is a proposed patch by me:

--- modules/pam_lastlog/pam_lastlog.c	16 Jun 2006 06:58:26 -0000	1.21
+++ modules/pam_lastlog/pam_lastlog.c	24 Aug 2006 14:41:28 -0000
@@ -12,6 +12,7 @@
 
 #include <fcntl.h>
 #include <time.h>
+#include <errno.h>
 #ifdef HAVE_UTMP_H
 # include <utmp.h>
 #else
@@ -327,9 +328,22 @@
     /* obtain the last login date and all the relevant info */
     last_fd = open(_PATH_LASTLOG, O_RDWR);
     if (last_fd < 0) {
-	pam_syslog(pamh, LOG_ERR, "unable to open %s: %m", _PATH_LASTLOG);
-	D(("unable to open %s file", _PATH_LASTLOG));
-	return PAM_SERVICE_ERR;
+        if (errno = ENOENT) {
+	     last_fd = open(_PATH_LASTLOG, O_RDWR|O_CREAT);
+             if (last_fd < 0) {
+	          pam_syslog(pamh, LOG_ERR,
+                             "unable to create %s: %m", _PATH_LASTLOG);
+		  D(("unable to create %s file", _PATH_LASTLOG));
+		  return PAM_SERVICE_ERR;
+	     }
+	     pam_syslog(pamh, LOG_INFO,
+			"file %s created", _PATH_LASTLOG);
+	     D(("file %s created", _PATH_LASTLOG));
+	} else {
+	  pam_syslog(pamh, LOG_ERR, "unable to open %s: %m", _PATH_LASTLOG);
+	  D(("unable to open %s file", _PATH_LASTLOG));
+	  return PAM_SERVICE_ERR;
+	}
     }
 
     if (lseek(last_fd, sizeof(struct lastlog) * (off_t) uid, SEEK_SET) < 0) {


-- 
Thorsten Kukuk         http://www.suse.de/~kukuk/      kukuk@...
SUSE LINUX Products GmbH       Maxfeldstr. 5       D-90409 Nuernberg
--------------------------------------------------------------------    
Key fingerprint = 8C6B FD92 EE0F 42ED F91A  6A73 6D1A 7F05 2E59 24BB

On Thu, 2006-08-24 at 16:43 +0200, Thorsten Kukuk wrote:
> Hi,
> 
> here is a proposed patch by me:

The patch is OK, however there is a minor problem with creating the file
- it can be created with different permissions than which would be in
the default system installation. Also I'd use LOG_WARN level probably
rather than LOG_INFO.

-- 
Tomas Mraz
No matter how far down the wrong road you've gone, turn back.
                                              Turkish proverb

On Thu, Aug 24, Tomas Mraz wrote:

> On Thu, 2006-08-24 at 16:43 +0200, Thorsten Kukuk wrote:
> > Hi,
> > 
> > here is a proposed patch by me:
> 
> The patch is OK, however there is a minor problem with creating the file
> - it can be created with different permissions than which would be in
> the default system installation. 

Yes, on SUSE Linux that's not a problem, but I have no idea how
to solve it for other distributions.

> Also I'd use LOG_WARN level probably rather than LOG_INFO.

I will do that.

  Thorsten

-- 
Thorsten Kukuk         http://www.suse.de/~kukuk/      kukuk@...
SUSE LINUX Products GmbH       Maxfeldstr. 5       D-90409 Nuernberg
--------------------------------------------------------------------    
Key fingerprint = 8C6B FD92 EE0F 42ED F91A  6A73 6D1A 7F05 2E59 24BB

Hi,

On Thu, Aug 24, 2006 at 06:14:47PM +0200, Thorsten Kukuk wrote:
> On Thu, Aug 24, Tomas Mraz wrote:
> > On Thu, 2006-08-24 at 16:43 +0200, Thorsten Kukuk wrote:
> > >=20
> > > here is a proposed patch by me:
> >=20
> > The patch is OK, however there is a minor problem with creating the file
> > - it can be created with different permissions than which would be in
> > the default system installation.=20
>=20
> Yes, on SUSE Linux that's not a problem, but I have no idea how
> to solve it for other distributions.

We could add a configure option and pass 3rd argument to open(2) call.


--=20
ldv

On Thu, Aug 24, Dmitry V. Levin wrote:

> Hi,
> 
> On Thu, Aug 24, 2006 at 06:14:47PM +0200, Thorsten Kukuk wrote:
> > On Thu, Aug 24, Tomas Mraz wrote:
> > > On Thu, 2006-08-24 at 16:43 +0200, Thorsten Kukuk wrote:
> > > > 
> > > > here is a proposed patch by me:
> > > 
> > > The patch is OK, however there is a minor problem with creating the file
> > > - it can be created with different permissions than which would be in
> > > the default system installation. 
> > 
> > Yes, on SUSE Linux that's not a problem, but I have no idea how
> > to solve it for other distributions.
> 
> We could add a configure option and pass 3rd argument to open(2) call.

Isn't this overkill? Which permissions do distributors normally use?
To get all tools working, you need 644. Does somebody use something
different?
More of a problem could only be the ownership, which user and group
owns the file.
On the other side, rpm -Va will tell the amdin that the permissions of 
this file are not as they should be.

  Thorsten

-- 
Thorsten Kukuk         http://www.suse.de/~kukuk/      kukuk@...
SUSE LINUX Products GmbH       Maxfeldstr. 5       D-90409 Nuernberg
--------------------------------------------------------------------    
Key fingerprint = 8C6B FD92 EE0F 42ED F91A  6A73 6D1A 7F05 2E59 24BB

On Thu, Aug 24, 2006 at 09:38:40PM +0200, Thorsten Kukuk wrote:
[...]
> Isn't this overkill?

Maybe.

> Which permissions do distributors normally use?
> To get all tools working, you need 644. Does somebody use something
> different?

Not me; in ALT and Owl we use 0644.
However, since O_CREAT honors umask, I suppose that there is a small chance
to create lastlog file more writable than 0644.

> More of a problem could only be the ownership, which user and group
> owns the file.

Most likely that root:root is normally used.


--=20
ldv

On Thu, Aug 24, 2006 at 11:52:15PM +0400, Dmitry V. Levin wrote:
> On Thu, Aug 24, 2006 at 09:38:40PM +0200, Thorsten Kukuk wrote:
[...]
> > Which permissions do distributors normally use?
> > To get all tools working, you need 644. Does somebody use something
> > different?
>=20
> Not me; in ALT and Owl we use 0644.
> However, since O_CREAT honors umask, I suppose that there is a small chan=
ce
> to create lastlog file more writable than 0644.

Somehow I missed that your patch actually does 3-arg open(2) call.

That is, to avoid world-writable log files, I suggest to always use 3-arg
form of open(2) in each case where O_CREAT is set, with 0644 mode.
There is only one place where this rule is not honoured, pam_exec.
OK to fix?


--=20
ldv

On Tue, Aug 29, Dmitry V. Levin wrote:

> On Thu, Aug 24, 2006 at 11:52:15PM +0400, Dmitry V. Levin wrote:
> > On Thu, Aug 24, 2006 at 09:38:40PM +0200, Thorsten Kukuk wrote:
> [...]
> > > Which permissions do distributors normally use?
> > > To get all tools working, you need 644. Does somebody use something
> > > different?
> > 
> > Not me; in ALT and Owl we use 0644.
> > However, since O_CREAT honors umask, I suppose that there is a small chance
> > to create lastlog file more writable than 0644.
> 
> Somehow I missed that your patch actually does 3-arg open(2) call.

I forgot to update the patch before I send it to the list...
 
> That is, to avoid world-writable log files, I suggest to always use 3-arg
> form of open(2) in each case where O_CREAT is set, with 0644 mode.
> There is only one place where this rule is not honoured, pam_exec.
> OK to fix?

Yes, that's a clear bug in pam_exec.

 Thanks,
   Thorsten


-- 
Thorsten Kukuk         http://www.suse.de/~kukuk/      kukuk@...
SUSE LINUX Products GmbH       Maxfeldstr. 5       D-90409 Nuernberg
--------------------------------------------------------------------    
Key fingerprint = 8C6B FD92 EE0F 42ED F91A  6A73 6D1A 7F05 2E59 24BB

On Wed, 2006-08-23 at 21:19 +0200, Thorsten Kukuk wrote:
> Hi,
> 
> I found a fatal problem with pam_cracklib today, as normal user I'm
> able to crash pam_cracklib by aborting the input of the new password
> a second time. The problem is that pam_cracklib sets the retval to
> an error condition, but we never check that afterwards.
> 
> I suggest the following patch to fix that (we do the same already for the
> other prompts):
> 
> --- modules/pam_cracklib/pam_cracklib.c	22 May 2006 17:27:54 -0000	1.17
> +++ modules/pam_cracklib/pam_cracklib.c	23 Aug 2006 17:09:33 -0000
> @@ -668,6 +668,12 @@
>                  continue;
>              }
>  
> +	    if (retval != PAM_SUCCESS) {
> +	      if (ctrl && PAM_DEBUG_ARG)
> +                pam_syslog(pamh,LOG_DEBUG,"unable to obtain retyped password");
> +	      continue;
> +	    }
> +
>              /* Hopefully now token1 and token2 the same password ... */
>              if (strcmp(token1,token2) != 0) {
>                  /* tell the user */

Good catch, please commit.
-- 
Tomas Mraz
No matter how far down the wrong road you've gone, turn back.
                                              Turkish proverb

On Thu, 2006-08-10 at 16:35 +0200, Thorsten Kukuk wrote:
> 
> Hi,
> 
> since we have this bootstrap bug, I would like to release 0.99.6.1
> with that fix now. Any objections?

OK from me.
-- 
Tomas Mraz
No matter how far down the wrong road you've gone, turn back.
                                              Turkish proverb

On Tue, 2006-08-01 at 10:55 +0200, Thorsten Kukuk wrote:
> On Mon, Jul 31, Tomas Mraz wrote:
> 
> > This doesn't handle the situation when the action is BAD and the module
> > is the only one in the stack. Attaching a different patch.
> 
> Ok, I commited compileable version of your patch now.
                ^^^^^^^^^^ Oops ;-)

Thanks
-- 
Tomas Mraz
No matter how far down the wrong road you've gone, turn back.
                                              Turkish proverb