[SCM] pam wrapper repository - branch master updated

Michael Adam obnox at samba.org
Mon Jan 18 10:10:16 UTC 2016


The branch, master has been updated
       via  31d374c Bump version to 1.0.1
       via  3b1d585 pwrap: Fix a possible timing issue in p_copy()
       via  c91b203 pwrap: Improve p_rmdirs() do avoid timing issues
       via  d3cfb7a pwrap: Remove superfloues lstat()
       via  490ae6a pwrap: Make sure we have a terminating null byte
       via  e2628f0 pam_matrix: Set a secure umask before calling mkstemp()
       via  0b43f0c pwrap: Return EPROTONOSUPPORT in audit_open()
       via  92c01c8 cmake: Link python module against the python library
       via  55ff828 cmake: Do not require a C++ compiler
      from  746496c Initial release of pam_wrapper 1.0.0

https://git.samba.org/?p=pam_wrapper.git;a=shortlog;h=master


- Log -----------------------------------------------------------------
commit 31d374cda4e3ce1f57d290d46ef3a97d6ed0b076
Author: Andreas Schneider <asn at samba.org>
Date:   Mon Jan 18 09:18:56 2016 +0100

    Bump version to 1.0.1
    
    Signed-off-by: Andreas Schneider <asn at samba.org>
    Reviewed-by: Michael Adam <obnox at samba.org>

commit 3b1d585468c7fbd6402f75e27096a19f3620b940
Author: Andreas Schneider <asn at samba.org>
Date:   Fri Jan 15 15:20:24 2016 +0100

    pwrap: Fix a possible timing issue in p_copy()
    
    Do not rely on stat before open - it is racy.
    Open directly and treat failure appropriately.
    
    CID: 47518
    
    Signed-off-by: Andreas Schneider <asn at samba.org>
    Reviewed-by: Michael Adam <obnox at samba.org>

commit c91b2033bebe69ff5bab67c8db960ac5ec021268
Author: Jakub Hrozek <jakub.hrozek at posteo.se>
Date:   Fri Jan 15 12:55:51 2016 +0100

    pwrap: Improve p_rmdirs() do avoid timing issues
    
    When calling stat and rmdir, we could run into timing issues that the
    stat information is incorrect till we are actually running the rmdir()
    command. So we open the directory and have the handle open to avoid
    that we work on outdated information. It is unlikely but Coverity
    complains and we thought better fix it.
    
    CID: 47519
    
    Signed-off-by: Jakub Hrozek <jakub.hrozek at posteo.se>
    Reviewed-by: Andreas Schneider <asn at samba.org>
    Reviewed-by: Michael Adam <obnox at samba.org>

commit d3cfb7af2a8f5ce844c46ce8c5fade8df1e2e429
Author: Andreas Schneider <asn at samba.org>
Date:   Fri Jan 15 11:11:50 2016 +0100

    pwrap: Remove superfloues lstat()
    
    There is no need to check if the file exists, just try to open it.
    
    CID: 47520
    
    Signed-off-by: Andreas Schneider <asn at samba.org>
    Reviewed-by: Michael Adam <obnox at samba.org>

commit 490ae6a25ddf21963095d6d3c66a086fbc6147f9
Author: Andreas Schneider <asn at samba.org>
Date:   Fri Jan 15 11:44:14 2016 +0100

    pwrap: Make sure we have a terminating null byte
    
    This is just to silence Coverity.
    
    CID: 47508
    
    Signed-off-by: Andreas Schneider <asn at samba.org>
    Reviewed-by: Michael Adam <obnox at samba.org>

commit e2628f07ce78abfbe5e29b4d3b6db707c2677063
Author: Andreas Schneider <asn at samba.org>
Date:   Fri Jan 15 11:38:00 2016 +0100

    pam_matrix: Set a secure umask before calling mkstemp()
    
    CID: 47516
    
    Signed-off-by: Andreas Schneider <asn at samba.org>
    Reviewed-by: Michael Adam <obnox at samba.org>

commit 0b43f0cd311d352806f5cd4f867a4a8efb29546e
Author: Andreas Schneider <asn at samba.org>
Date:   Thu Jan 14 17:04:33 2016 +0100

    pwrap: Return EPROTONOSUPPORT in audit_open()
    
    I don't know why but returning EINVAL doesn't work. It treats it as
    success and tries to write to it.
    
    Signed-off-by: Andreas Schneider <asn at samba.org>
    Reviewed-by: Michael Adam <obnox at samba.org>

commit 92c01c80103034d4a3cdd92aa4d00c34d687300d
Author: Andreas Schneider <asn at samba.org>
Date:   Thu Jan 14 17:04:07 2016 +0100

    cmake: Link python module against the python library
    
    Signed-off-by: Andreas Schneider <asn at samba.org>
    Reviewed-by: Michael Adam <obnox at samba.org>

commit 55ff828e58ff9a29badb19f6d5b68e7fe7a990f8
Author: Andreas Schneider <asn at samba.org>
Date:   Thu Jan 14 13:46:01 2016 +0100

    cmake: Do not require a C++ compiler
    
    Signed-off-by: Andreas Schneider <asn at samba.org>
    Reviewed-by: Michael Adam <obnox at samba.org>

-----------------------------------------------------------------------

Summary of changes:
 CMakeLists.txt             |   4 +-
 ChangeLog                  |   5 ++
 src/modules/CMakeLists.txt |   2 +-
 src/modules/pam_matrix.c   |   2 +-
 src/pam_wrapper.c          | 208 ++++++++++++++++++++++-----------------------
 src/python/CMakeLists.txt  |   4 +-
 6 files changed, 111 insertions(+), 114 deletions(-)


Changeset truncated at 500 lines:

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 8709a14..1674909 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -8,7 +8,7 @@ set(APPLICATION_NAME ${PROJECT_NAME})
 
 set(APPLICATION_VERSION_MAJOR "1")
 set(APPLICATION_VERSION_MINOR "0")
-set(APPLICATION_VERSION_PATCH "0")
+set(APPLICATION_VERSION_PATCH "1")
 
 set(APPLICATION_VERSION "${APPLICATION_VERSION_MAJOR}.${APPLICATION_VERSION_MINOR}.${APPLICATION_VERSION_PATCH}")
 
@@ -19,7 +19,7 @@ set(APPLICATION_VERSION "${APPLICATION_VERSION_MAJOR}.${APPLICATION_VERSION_MINO
 #     Increment AGE. Set REVISION to 0
 #   If the source code was changed, but there were no interface changes:
 #     Increment REVISION.
-set(LIBRARY_VERSION "0.0.1")
+set(LIBRARY_VERSION "0.0.2")
 set(LIBRARY_SOVERSION "0")
 
 set(PAMTEST_LIBRARY_VERSION "0.0.1")
diff --git a/ChangeLog b/ChangeLog
index 869aaa3..4d7ea27 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,6 +1,11 @@
 ChangeLog
 ==========
 
+version 1.0.1 (released 2016-01-18)
+  * Fixed issue with audit_open() and sshd
+  * Fixed several issues found by Coverity
+  * Fixed python linking issues
+
 version 1.0.0 (released 2016-01-14)
   * Initial release
     - pam_wrapper
diff --git a/src/modules/CMakeLists.txt b/src/modules/CMakeLists.txt
index 93ce522..8e13a0b 100644
--- a/src/modules/CMakeLists.txt
+++ b/src/modules/CMakeLists.txt
@@ -1,4 +1,4 @@
-project(pam_wrapper-modules)
+project(pam_wrapper-modules C)
 
 set(PAM_MODULES pam_matrix pam_get_items pam_set_items)
 
diff --git a/src/modules/pam_matrix.c b/src/modules/pam_matrix.c
index bf5c60a..c48a916 100644
--- a/src/modules/pam_matrix.c
+++ b/src/modules/pam_matrix.c
@@ -214,7 +214,7 @@ static int pam_matrix_lib_items_put(const char *db,
 	}
 
 	/* We don't support concurrent runs.. */
-	old_mask = umask(0);
+	old_mask = umask(S_IRWXO | S_IRWXG);
 	rv = mkstemp(template);
 	umask(old_mask);
 	if (rv <= 0) {
diff --git a/src/pam_wrapper.c b/src/pam_wrapper.c
index 0c451d1..fddc7fa 100644
--- a/src/pam_wrapper.c
+++ b/src/pam_wrapper.c
@@ -533,32 +533,21 @@ static int p_copy(const char *src, const char *dst, const char *pdir, mode_t mod
 		return -1;
 	}
 
-	if (lstat(src, &sb) < 0) {
-		return -1;
-	}
-
-	if (S_ISDIR(sb.st_mode)) {
-		errno = EISDIR;
+	srcfd = open(src, O_RDONLY, 0);
+	if (srcfd < 0) {
 		return -1;
 	}
 
 	if (mode == 0) {
-		mode = sb.st_mode;
-	}
-
-	if (lstat(dst, &sb) == 0) {
-		if (S_ISDIR(sb.st_mode)) {
-			errno = EISDIR;
+		rc = fstat(srcfd, &sb);
+		if (rc != 0) {
 			return -1;
 		}
+		mode = sb.st_mode;
 	}
 
-	if ((srcfd = open(src, O_RDONLY, 0)) < 0) {
-		rc = -1;
-		goto out;
-	}
-
-	if ((dstfd = open(dst, O_CREAT|O_WRONLY|O_TRUNC, mode)) < 0) {
+	dstfd = open(dst, O_CREAT|O_WRONLY|O_TRUNC, mode);
+	if (dstfd < 0) {
 		rc = -1;
 		goto out;
 	}
@@ -695,58 +684,59 @@ static void pwrap_clean_stale_dirs(const char *dir)
 {
 	size_t len = strlen(dir);
 	char pidfile[len + 5];
-	struct stat sb;
 	ssize_t rc;
+	char buf[8] = {0};
+	long int tmp;
+	pid_t pid;
+	int fd;
 
 	snprintf(pidfile,
 		 sizeof(pidfile),
 		 "%s/pid",
 		 dir);
 
-	rc = lstat(pidfile, &sb);
-	if (rc == 0) {
-		char buf[8] = {0};
-		long int tmp;
-		pid_t pid;
-		int fd;
-
-		/* read the pidfile */
-		fd = open(pidfile, O_RDONLY);
-		if (fd < 0) {
+	/* read the pidfile */
+	fd = open(pidfile, O_RDONLY);
+	if (fd < 0) {
+		if (errno == ENOENT) {
+			PWRAP_LOG(PWRAP_LOG_TRACE,
+				  "pidfile %s missing, nothing to do\n",
+				  pidfile);
+		} else {
 			PWRAP_LOG(PWRAP_LOG_ERROR,
 				  "Failed to open pidfile %s - error: %s",
 				  pidfile, strerror(errno));
-			return;
 		}
+		return;
+	}
 
-		rc = read(fd, buf, sizeof(buf));
-		close(fd);
-		if (rc < 0) {
-			PWRAP_LOG(PWRAP_LOG_ERROR,
-				  "Failed to read pidfile %s - error: %s",
-				  pidfile, strerror(errno));
-			return;
-		}
+	rc = read(fd, buf, sizeof(buf));
+	close(fd);
+	if (rc < 0) {
+		PWRAP_LOG(PWRAP_LOG_ERROR,
+			  "Failed to read pidfile %s - error: %s",
+			  pidfile, strerror(errno));
+		return;
+	}
 
-		buf[sizeof(buf) - 1] = '\0';
+	buf[sizeof(buf) - 1] = '\0';
 
-		tmp = strtol(buf, NULL, 10);
-		if (tmp == 0 || tmp > 0xFFFF || errno == ERANGE) {
-			PWRAP_LOG(PWRAP_LOG_ERROR,
-				  "Failed to parse pid, buf=%s",
-				  buf);
-			return;
-		}
+	tmp = strtol(buf, NULL, 10);
+	if (tmp == 0 || tmp > 0xFFFF || errno == ERANGE) {
+		PWRAP_LOG(PWRAP_LOG_ERROR,
+			  "Failed to parse pid, buf=%s",
+			  buf);
+		return;
+	}
 
-		pid = (pid_t)(tmp & 0xFFFF);
+	pid = (pid_t)(tmp & 0xFFFF);
 
-		rc = kill(pid, 0);
-		if (rc == -1) {
-			PWRAP_LOG(PWRAP_LOG_TRACE,
-				  "Remove stale pam_wrapper dir: %s",
-				  dir);
-			p_rmdirs(dir);
-		}
+	rc = kill(pid, 0);
+	if (rc == -1) {
+		PWRAP_LOG(PWRAP_LOG_TRACE,
+			  "Remove stale pam_wrapper dir: %s",
+			  dir);
+		p_rmdirs(dir);
 	}
 
 	return;
@@ -899,6 +889,7 @@ static void pwrap_init(void)
 		char *dname;
 
 		strncpy(libpam_path_cp, libpam_path, sizeof(libpam_path_cp));
+		libpam_path_cp[sizeof(libpam_path_cp) - 1] = '\0';
 
 		dname = dirname(libpam_path_cp);
 		if (dname == NULL) {
@@ -1523,7 +1514,7 @@ int audit_open(void)
 	 * Tell the application that the kernel doesn't
 	 * have audit compiled in.
 	 */
-	errno = EINVAL;
+	errno = EPROTONOSUPPORT;
 	return -1;
 }
 
@@ -1531,74 +1522,75 @@ int audit_open(void)
  * DESTRUCTOR
  ***************************/
 
-static int p_rmdirs(const char *path)
+static int p_rmdirs_at(const char *path, int parent_fd)
 {
 	DIR *d;
 	struct dirent *dp;
 	struct stat sb;
-	char *fname;
+	int path_fd;
+	int rc;
 
-	if ((d = opendir(path)) != NULL) {
-		while(stat(path, &sb) == 0) {
-			/* if we can remove the directory we're done */
-			if (rmdir(path) == 0) {
-				break;
-			}
-			switch (errno) {
-				case ENOTEMPTY:
-				case EEXIST:
-				case EBADF:
-					break; /* continue */
-				default:
-					closedir(d);
-					return 0;
-			}
+	/* If path is absolute, parent_fd is ignored. */
+	PWRAP_LOG(PWRAP_LOG_TRACE,
+		  "p_rmdirs_at removing %s at %d\n", path, parent_fd);
 
-			while ((dp = readdir(d)) != NULL) {
-				size_t len;
-				/* skip '.' and '..' */
-				if (dp->d_name[0] == '.' &&
-				    (dp->d_name[1] == '\0' ||
-				     (dp->d_name[1] == '.' && dp->d_name[2] == '\0'))) {
-					continue;
-				}
+	path_fd = openat(parent_fd,
+			 path, O_RDONLY | O_DIRECTORY | O_NOFOLLOW);
+	if (path_fd == -1) {
+		return -1;
+	}
 
-				len = strlen(path) + strlen(dp->d_name) + 2;
-				fname = malloc(len);
-				if (fname == NULL) {
-					closedir(d);
-					return -1;
-				}
-				snprintf(fname, len, "%s/%s", path, dp->d_name);
-
-				/* stat the file */
-				if (lstat(fname, &sb) != -1) {
-					if (S_ISDIR(sb.st_mode) && !S_ISLNK(sb.st_mode)) {
-						if (rmdir(fname) < 0) { /* can't be deleted */
-							if (errno == EACCES) {
-								closedir(d);
-								SAFE_FREE(fname);
-								return -1;
-							}
-							p_rmdirs(fname);
-						}
-					} else {
-						unlink(fname);
-					}
-				} /* lstat */
-				SAFE_FREE(fname);
-			} /* readdir */
+	d = fdopendir(path_fd);
+	if (d == NULL) {
+		close(path_fd);
+		return -1;
+	}
 
-			rewinddir(d);
+	while ((dp = readdir(d)) != NULL) {
+		/* skip '.' and '..' */
+		if (dp->d_name[0] == '.' &&
+			(dp->d_name[1] == '\0' ||
+			(dp->d_name[1] == '.' && dp->d_name[2] == '\0'))) {
+			continue;
 		}
-	} else {
+
+		rc = fstatat(path_fd, dp->d_name,
+			     &sb, AT_SYMLINK_NOFOLLOW);
+		if (rc != 0) {
+			continue;
+		}
+
+		if (S_ISDIR(sb.st_mode)) {
+			rc = p_rmdirs_at(dp->d_name, path_fd);
+		} else {
+			rc = unlinkat(path_fd, dp->d_name, 0);
+		}
+		if (rc != 0) {
+			continue;
+		}
+	}
+	closedir(d);
+
+	rc = unlinkat(parent_fd, path, AT_REMOVEDIR);
+	if (rc != 0) {
+		rc = errno;
+		PWRAP_LOG(PWRAP_LOG_TRACE,
+			  "cannot unlink %s error %d\n", path, rc);
 		return -1;
 	}
 
-	closedir(d);
 	return 0;
 }
 
+static int p_rmdirs(const char *path)
+{
+	/*
+	 * If path is absolute, p_rmdirs_at ignores parent_fd.
+	 * If it's relative, start from cwd.
+	 */
+	return p_rmdirs_at(path, AT_FDCWD);
+}
+
 /*
  * This function is called when the library is unloaded and makes sure that
  * resources are freed.
diff --git a/src/python/CMakeLists.txt b/src/python/CMakeLists.txt
index 108daae..cbee2a6 100644
--- a/src/python/CMakeLists.txt
+++ b/src/python/CMakeLists.txt
@@ -1,11 +1,11 @@
-project(pypamtest)
+project(pypamtest C)
 
 include_directories(${CMAKE_BINARY_DIR})
 include_directories(${pam_wrapper-headers_DIR})
 include_directories(${PYTHON_INCLUDE_DIR})
 
 python_add_module(pypamtest pypamtest.c)
-target_link_libraries(pypamtest pamtest)
+target_link_libraries(pypamtest pamtest ${PYTHON_LIBRARY})
 
 install(
     TARGETS


-- 
pam wrapper repository



More information about the samba-cvs mailing list