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 <a...@samba.org> Date: Mon Jan 18 09:18:56 2016 +0100 Bump version to 1.0.1 Signed-off-by: Andreas Schneider <a...@samba.org> Reviewed-by: Michael Adam <ob...@samba.org> commit 3b1d585468c7fbd6402f75e27096a19f3620b940 Author: Andreas Schneider <a...@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 <a...@samba.org> Reviewed-by: Michael Adam <ob...@samba.org> commit c91b2033bebe69ff5bab67c8db960ac5ec021268 Author: Jakub Hrozek <jakub.hro...@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.hro...@posteo.se> Reviewed-by: Andreas Schneider <a...@samba.org> Reviewed-by: Michael Adam <ob...@samba.org> commit d3cfb7af2a8f5ce844c46ce8c5fade8df1e2e429 Author: Andreas Schneider <a...@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 <a...@samba.org> Reviewed-by: Michael Adam <ob...@samba.org> commit 490ae6a25ddf21963095d6d3c66a086fbc6147f9 Author: Andreas Schneider <a...@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 <a...@samba.org> Reviewed-by: Michael Adam <ob...@samba.org> commit e2628f07ce78abfbe5e29b4d3b6db707c2677063 Author: Andreas Schneider <a...@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 <a...@samba.org> Reviewed-by: Michael Adam <ob...@samba.org> commit 0b43f0cd311d352806f5cd4f867a4a8efb29546e Author: Andreas Schneider <a...@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 <a...@samba.org> Reviewed-by: Michael Adam <ob...@samba.org> commit 92c01c80103034d4a3cdd92aa4d00c34d687300d Author: Andreas Schneider <a...@samba.org> Date: Thu Jan 14 17:04:07 2016 +0100 cmake: Link python module against the python library Signed-off-by: Andreas Schneider <a...@samba.org> Reviewed-by: Michael Adam <ob...@samba.org> commit 55ff828e58ff9a29badb19f6d5b68e7fe7a990f8 Author: Andreas Schneider <a...@samba.org> Date: Thu Jan 14 13:46:01 2016 +0100 cmake: Do not require a C++ compiler Signed-off-by: Andreas Schneider <a...@samba.org> Reviewed-by: Michael Adam <ob...@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