Bug#349367: libpam-encfs: wrong behavior if a user don't have an encrypted home, directory + some remarks (patch included)

2006-01-27 Thread Benoit Hamet
Hi Ruben,

Ruben Porras wrote:
>>Anyway, here's the steps to reproduce :
> 
> [...]
> 
>>So here is a possible security hole : if you want that somebody without
>>an encrypted home dir, shouldn't login, then he can ... aïe.
> 
> 
> No, that was not the intended behaviour, everybody should be able to
> login.

Ok, nice.

> 
>>I wrote the patch attached to partialy correct this behavior : at least
>>evrybody with or without an encrypted home dir will be able to log in.
> 
> 
> I think this is fixed in the new version uploaded yesterday to unstable.
> Can you check that?
>  
Looks like I agree with you :).


> 
> 
>>I take the time to correct some warnings (not all, but most of them)
>> too.
>>
>>Ok, back to the default conf file : why the maintainer let it's user
>>name in the file ??? (and not commented).
> 
> 
> why not? Anyway, the example has now a generic "user" and commented.

Just imagine a devil admin having a user with the same login as yourself
and using libpam-encfs ... people will don't understand why they cannot
login (due to the default behavior of encfs, which create the encryption
if no .encfs5 is found (but that's another problem)).

> 
> 
>>I suggest too, to put somewhere that the password for the encfs
>>NEED to be the SAME as the unix login ... that's a serious weakness in
>>my mind ... if somebody was able to crack the login password, it could
>>read the encrypted data's ... So in the same time, I suggest to put a
>>warning, to explain that you need to put the .encfs5 on another device
>>like an USBkey + automount it for reading this file (a link from
>>/home/.enc/encrypt/.encfs5 to /var/autofs/removable/uba/encfs5, could be
>>a start for a better security) NOTE : this can't work with actual code.
> 
> 
> This is not addressed in the last upload, neither the other fixes unless
> upstream has fixed them (which did with a couple of them). So, if you
> agree with me that the main issue is solved, we can downgrade this bug.

Yes, no problem with that.
Just to let you know, you will need to "fix" encfs to get this working
(at least the encfs in testing) since a lstat is done and not a stat (I
guess that's not a mistake, but I'm still trying to understand why using
lstat instead of stat ...). So perhaps some discussions with the encfs
maintainer will be necessary.

Regards, and thanks for your quick answer.

Benoît.



signature.asc
Description: OpenPGP digital signature


Bug#349367: libpam-encfs: wrong behavior if a user don't have an encrypted home, directory + some remarks (patch included)

2006-01-26 Thread Ruben Porras
> Anyway, here's the steps to reproduce :
[...]
> So here is a possible security hole : if you want that somebody without
> an encrypted home dir, shouldn't login, then he can ... aïe.

No, that was not the intended behaviour, everybody should be able to
login.

> I wrote the patch attached to partialy correct this behavior : at least
> evrybody with or without an encrypted home dir will be able to log in.

I think this is fixed in the new version uploaded yesterday to unstable.
Can you check that?
 

> I take the time to correct some warnings (not all, but most of them)
>  too.
> 
> Ok, back to the default conf file : why the maintainer let it's user
> name in the file ??? (and not commented).

why not? Anyway, the example has now a generic "user" and commented.

> I suggest too, to put somewhere that the password for the encfs
> NEED to be the SAME as the unix login ... that's a serious weakness in
> my mind ... if somebody was able to crack the login password, it could
> read the encrypted data's ... So in the same time, I suggest to put a
> warning, to explain that you need to put the .encfs5 on another device
> like an USBkey + automount it for reading this file (a link from
> /home/.enc/encrypt/.encfs5 to /var/autofs/removable/uba/encfs5, could be
> a start for a better security) NOTE : this can't work with actual code.

This is not addressed in the last upload, neither the other fixes unless
upstream has fixed them (which did with a couple of them). So, if you
agree with me that the main issue is solved, we can downgrade this bug.




Bug#349367: libpam-encfs: wrong behavior if a user don't have an encrypted home, directory + some remarks (patch included)

2006-01-22 Thread Benoit Hamet
Package: libpam-encfs
Version: 0.1.2-4
Severity: important
Tags: patch


Hi all,

First, I wan't to say that I hesitate to put it as grave, since I think
this can be a kind of security hole. But since
1) that's not a default configuration
2) not so much people use it :) (I guess, or I'm a martian using encfs
the way I use it :)
3) I perhaps doesn't understand how it should work ...
4) It depends on what you want as default behavior.
I only put it as important.

Anyway, here's the steps to reproduce :
Create a "encrypt" user, I mean with a home directory using the encfs
utilities.
Create a "normal" user.
Config pam as specified in the README.Debian :

-- 8< -- /etc/pam.d/common-auth -- 8< --
authsufficient  pam_encfs.so
authrequiredpam_unix.so nullok_secure use_first_pass
-- 8< --

use the default configuration file /etc/pam_encfs.conf (more about this
file later)
Login as the normal user : it's failing complaining about not able to
read /home/.enc/encrypt/.encfs5 ... then the login is waiting for
creating the new encrypted dir ... so you need to Ctrl+C to try a new
time ...
Login as the encrypt user : ok your logged !
In another console, try to loggin as the normal user : it works !
So here is a possible security hole : if you want that somebody without
an encrypted home dir, shouldn't login, then he can ... aïe.

I wrote the patch attached to partialy correct this behavior : at least
evrybody with or without an encrypted home dir will be able to log in. I
take the
time to correct some warnings (not all, but most of them) too.

Ok, back to the default conf file : why the maintainer let it's user
name in the file ??? (and not commented).

I suggest too, to put somewhere that the password for the encfs
NEED to be the SAME as the unix login ... that's a serious weakness in
my mind ... if somebody was able to crack the login password, it could
read the encrypted data's ... So in the same time, I suggest to put a
warning, to explain that you need to put the .encfs5 on another device
like an USBkey + automount it for reading this file (a link from
/home/.enc/encrypt/.encfs5 to /var/autofs/removable/uba/encfs5, could be
a start for a better security) NOTE : this can't work with actual code.

Hope this help,
I'm ok to discuss issue.

Regards,
Benoît.

-- System Information:
Debian Release: testing/unstable
  APT prefers testing
  APT policy: (500, 'testing')
Architecture: i386 (i686)
Shell:  /bin/sh linked to /bin/bash
Kernel: Linux 2.6.12dsdt
Locale: [EMAIL PROTECTED], [EMAIL PROTECTED] (charmap=UTF-8)

Versions of packages libpam-encfs depends on:
ii  encfs 1.2.4.1-2  encrypted virtual filesystem
ii  libc6 2.3.5-8GNU C Library: Shared
libraries an
ii  libpam0g  0.79-3 Pluggable Authentication
Modules l

libpam-encfs recommends no packages.

-- no debconf information
--- libpam-encfs-0.1.2-orig/pam_encfs.c 2006-01-22 15:14:36.0 +0100
+++ libpam-encfs-0.1.2/pam_encfs.c  2006-01-22 15:46:45.0 +0100
@@ -69,6 +69,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -173,7 +174,7 @@
 }
 
 
-int checkmnt(char *targetpath) {
+int checkmnt(const char *targetpath) {
   FILE *f = setmntent("/etc/mtab", "r");
   struct mntent *m;
   
@@ -193,7 +194,7 @@
   char *str;
   do {
 str =  strchr(line,',');
-if (str > NULL) {
+if (str != NULL) {
   *str = ' ';
 }
   } while (str != NULL);
@@ -217,7 +218,7 @@
   char line[BUFSIZE];
   char username[USERNAME_MAX];
   int parsed;
-  char *tmp;
+  const char *tmp;
   
   // Return 1 = error, 2 = silent error (ie already mounted)
   
@@ -262,6 +263,9 @@
   // Todo check if this dir exists and give better error msg
 }
   }
+  else
+  //If we are not the right user, just read next line 
+  continue;
   
   if (strcmp("-",targetpath) == 0) {
 // We do not have targetpath, construct one.
@@ -355,7 +359,10 @@
   int inpipe[2],outpipe[2];
   
   rval = pam_get_user(pamh, &user, NULL);
-  if ((rval != PAM_SUCCESS) || (!user)) {
+  if (rval != PAM_SUCCESS)
+  return rval;
+
+  if (!user) {
 _pam_log ( LOG_ERR, "can't get username: %s", pam_strerror ( pamh, rval ) 
);
 return PAM_AUTH_ERR;
   }
@@ -363,7 +370,7 @@
   rval = pam_get_item(pamh, PAM_AUTHTOK, (const void **)(void *)&passwd);
   if (rval != PAM_SUCCESS) {
 _pam_log(LOG_ERR, "Could not retrieve user's password");
-return PAM_AUTH_ERR;
+return rval;
   }
   
   if (!passwd) {
@@ -372,7 +379,9 @@
   return rval;
 }
 rval = pam_get_item(pamh, PAM_AUTHTOK, (const void **)(void *)&passwd);
-if (rval != PAM_SUCCESS || passwd == NULL) {
+if (rval != PAM_SUCCESS)
+return rval;
+if(passwd == NULL) {
   _pam_log(LOG_ERR, "Could not retrieve user's password");
   return PAM_AUTH_ERR;
 }
@@ -552,7 +561,7 @@
   int retval;
   pid_t pid;
   const char *targetpath;
-