Hi,
On Fri, Sep 06, 2019 at 11:25:02AM +0200, Thorsten Kukuk wrote:
[...]
--- a/configure.ac
+++ b/configure.ac
@@ -504,6 +504,23 @@ if test ! -z "$LIBSELINUX" ; then
LIBS=$BACKUP_LIBS
fi
+AC_ARG_ENABLE([econf],
+ AS_HELP_STRING([--disable-econf], [do not use libeconf]),
+ [WITH_ECONF=$enableval], WITH_ECONF=yes)
+if test "$WITH_ECONF" = "yes" ; then
+ PKG_CHECK_MODULES([ECONF], [libeconf], [],
+
[AC_CHECK_LIB([econf],[econf_readDirs],[ECONF_LIBS="-leconf"],[ECONF_LIBS=""])])
+ if test -n "$ECONF_LIBS" ; then
+ ECONF_CFLAGS="-DUSE_ECONF=1 $ECONF_CFLAGS"
+ fi
+fi
+AC_SUBST([ECONF_CFLAGS])
+AC_SUBST([ECONF_LIBS])
+AC_ARG_ENABLE([vendordir],
+ AS_HELP_STRING([--enable-vendordir=DIR], [Direcotry for distribution provided
configuration files]),,[])
+AC_SUBST([VENDORDIR], [$enable_vendordir])
+AM_CONDITIONAL([HAVE_VENDORDIR], [test "x$enable_vendordir" != x])
+
dnl Checks for header files.
AC_HEADER_DIRENT
AC_HEADER_STDC
I'm not sure I understand the intent here.
Does it make sense to enable econf (by default) when vendordir is not enabled?
Does it make sens to enable vendordir when econf is not available?
It seems to me that these two have to be either both enabled or both
disabled, please correct me if I got it wrong.
--- a/doc/man/pam.8.xml
+++ b/doc/man/pam.8.xml
@@ -53,11 +53,12 @@
<para>
Vendor-supplied PAM configuration files might be installed in
- the system directory <filename>/usr/lib/pam.d/</filename> instead
+ the system directory <filename>/usr/lib/pam.d/</filename> or
If no machine configuration file is found, the vendor-supplied file
is used. All files in <filename>/etc/pam.d/</filename> override
- files with the same name in <filename>/usr/lib/pam.d/</filename>.
+ files with the same name in other directories.
</para>
<para>From the point of view of the system administrator, for whom this
@@ -157,6 +158,18 @@ closing hook for modules to affect the services available to a
user.</para>
</para>
</listitem>
</varlistentry>
+ <varlistentry>
+ <term><filename>%vendordir%/pam.d</filename></term>
This line adds spaces before tabs, please correct this.
--- a/libpam/Makefile.am
+++ b/libpam/Makefile.am
@@ -3,10 +3,15 @@
#
AM_CFLAGS = -DDEFAULT_MODULE_PATH=\"$(SECUREDIR)/\" -DLIBPAM_COMPILE \
- -I$(srcdir)/include $(LIBPRELUDE_CFLAGS) -DPAM_VERSION=\"$(VERSION)\"
+ -I$(srcdir)/include $(LIBPRELUDE_CFLAGS) \
+ -DPAM_VERSION=\"$(VERSION)\" $(ECONF_CFLAGS) \
I suppose ECONF_CFLAGS will never define PAM_VERSION, but let's place
$(ECONF_CFLAGS) right after $(LIBPRELUDE_CFLAGS) anyway.
+ -DSYSCONFDIR=\"$(sysconfdir)\"
if HAVE_LIBSELINUX
AM_CFLAGS += -D"WITH_SELINUX"
endif
+if HAVE_VENDORDIR
+ AM_CFLAGS += -DVENDORDIR=\"@VENDORDIR@\"
+endif
Could you use $(VENDORDIR) instead of @VENDORDIR@ here, please?
CLEANFILES = *~
@@ -21,7 +26,7 @@ noinst_HEADERS = pam_prelude.h pam_private.h pam_tokens.h \
pam_modutil_private.h
libpam_la_LDFLAGS = -no-undefined -version-info 84:2:84
-libpam_la_LIBADD = @LIBAUDIT@ $(LIBPRELUDE_LIBS) @LIBDL@
+libpam_la_LIBADD = @LIBAUDIT@ $(LIBPRELUDE_LIBS) @LIBDL@ @ECONF_LIBS@
Could you use $(ECONF_LIBS) instead of @ECONF_LIBS@ here, please?
Also, let it be placed right after $(LIBPRELUDE_LIBS).
--- a/libpam/pam_handlers.c
+++ b/libpam/pam_handlers.c
@@ -282,7 +282,6 @@ _pam_open_config_file(pam_handle_t *pamh
{
char *p;
FILE *f;
- int err = 0;
/* Absolute path */
if (service[0] == '/') {
@@ -329,6 +328,23 @@ _pam_open_config_file(pam_handle_t *pamh
*file = f;
return PAM_SUCCESS;
}
+
+#ifdef PAM_CONFIG_DIST2_D
+ /* System Configuration /<vendordir>/pam.d/ */
+ _pam_drop(p);
+ if (asprintf (&p, PAM_CONFIG_DIST2_DF, service) < 0) {
+ pam_syslog(pamh, LOG_CRIT, "asprintf failed");
+ return PAM_BUF_ERR;
+ }
+ D(("opening %s", p));
+ f = fopen(p, "r");
+ if (f != NULL) {
+ *path = p;
+ *file = f;
+ return PAM_SUCCESS;
+ }
+#endif
+
_pam_drop(p);
return PAM_ABORT;
This creates a third copy of the same code.
Could you turn it into a loop, please?
--- a/libpam/pam_modutil_searchkey.c
+++ b/libpam/pam_modutil_searchkey.c
@@ -13,9 +13,39 @@
#include <string.h>
#include <stdlib.h>
#include <ctype.h>
+#ifdef USE_ECONF
+#include <libeconf.h>
+#endif
#define BUF_SIZE 8192
+#ifdef USE_ECONF
+#define LOGIN_DEFS "/etc/login.defs"
+
+#ifndef VENDORDIR
+#define VENDORDIR NULL
+#endif
+
+static char *
+econf_search_key (const char *name, const char *suffix, const char *key)
+{
+ econf_file *key_file = NULL;
+ char *val;
+
+ if (econf_readDirs (&key_file, VENDORDIR, SYSCONFDIR, name, suffix,
+ " \t", "#"))
+ return NULL;
+
+ if (econf_getStringValue (key_file, NULL, key, &val))
+ return NULL;
+
+ econf_free (key_file);
I'm not familiar with econf API, but this looks like a key_file leak.
--- a/modules/pam_securetty/Makefile.am
+++ b/modules/pam_securetty/Makefile.am
@@ -20,6 +20,9 @@ AM_LDFLAGS = -no-undefined -avoid-version -module
if HAVE_VERSIONING
AM_LDFLAGS += -Wl,--version-script=$(srcdir)/../modules.map
endif
+if HAVE_VENDORDIR
+ AM_CFLAGS += -DVENDORDIR=\"@VENDORDIR@\"
+endif
Could you use $(VENDORDIR) instead of @VENDORDIR@ here, ...
securelib_LTLIBRARIES = pam_securetty.la
pam_securetty_la_LIBADD = $(top_builddir)/libpam/libpam.la
@@ -27,5 +30,10 @@ pam_securetty_la_LIBADD = $(top_builddir)/libpam/libpam.la
if ENABLE_REGENERATE_MAN
noinst_DATA = README
README: pam_securetty.8.xml
+if HAVE_VENDORDIR
+XSLTPROC_CUSTOM = --stringparam vendordir $(VENDORDIR)
+else
+XSLTPROC_CUSTOM = --stringparam vendordir "<vendordir>"
+endif
... like you already did here, please?
--- a/modules/pam_securetty/pam_securetty.c
+++ b/modules/pam_securetty/pam_securetty.c
@@ -1,6 +1,9 @@
/* pam_securetty module */
#define SECURETTY_FILE "/etc/securetty"
+#ifdef VENDORDIR
+#define SECURETTY2_FILE VENDORDIR"/securetty"
+#endif
#define TTY_PREFIX "/dev/"
#define CMDLINE_FILE "/proc/cmdline"
#define CONSOLEACTIVE_FILE "/sys/class/tty/console/active"
@@ -25,6 +28,7 @@
#include <string.h>
#include <ctype.h>
#include <limits.h>
+#include <errno.h>
/*
* here, we make a definition for the externally accessible function
@@ -70,6 +74,7 @@ securetty_perform_check (pam_handle_t *pamh, int ctrl,
const char *function_name)
{
int retval = PAM_AUTH_ERR;
+ const char *securettyfile;
const char *username;
const char *uttyname;
const void *void_uttyname;
@@ -111,10 +116,27 @@ securetty_perform_check (pam_handle_t *pamh, int ctrl,
}
if (stat(SECURETTY_FILE, &ttyfileinfo)) {
+#ifdef VENDORDIR
+ if (errno == ENOENT) {
+ if (stat(SECURETTY2_FILE, &ttyfileinfo)) {
+ pam_syslog(pamh, LOG_NOTICE,
+ "Couldn't open %s: %m", SECURETTY2_FILE);
+ return PAM_SUCCESS; /* for compatibility with old securetty handling,
+ this needs to succeed. But we still log the
+ error. */
I wonder which of two files should be mentioned in the log message here.
--
ldv