8000 Possible issues in `keyutils` · Issue #695 · AerynOS/recipes · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content
Possible issues in keyutils #695
Open
@QORTEC

Description

@QORTEC

Description:

I've detailed every use of /etc in keyutils to track down probable necessary changes/updates.

  • I've noted an inconsistency in key.dns_resolver.c and it's documentation.
  • I've also noted a possible issue with the currently used patch.

Each instance of /etc in keyutils:

grep -R /etc:
man/keyutils.7:                 .I /etc/request\-key.conf
man/keyutils.7:                 .I /etc/request\-key.d/
man/request-key.conf.5:         /etc/request\-key.d/*.conf
man/request-key.conf.5:         /etc/request\-key.conf
man/request-key.conf.5:         A basic file will be installed in the /etc. This will contain two debugging
man/request-key.conf.5:         /etc/request\-key.conf
man/request-key.conf.5:         /etc/request\-key.d/*.conf
man/key.dns_resolver.conf.5:    /etc/key.dns_resolver.conf
man/key.dns_resolver.conf.5:    /etc/key.dns_resolver.conf
man/key.dns_resolver.8:         configuration file is in /etc, but this can be overridden with the \fB-c\fR
man/request-key.8:              .IR /etc .
man/request-key.8:              /etc/request\-key.d/*.conf
man/request-key.8:              /etc/request\-key.conf
request-key.c:                  scan_conf_dir(params, "/etc/request-key.d");
request-key.c:                  scan_conf_file(params, AT_FDCWD, "/etc/request-key.conf");
keyutils.spec:                  - Allow "keyctl add/padd/etc." to take hex-encoded data.
key.dns_resolver.c:             * lines added to your /etc/request-key.conf file:
key.dns_resolver.c:             static const char *config_file = "/etc/keyutils/key.dns_resolver.conf";
Makefile:                       ETCDIR         := /etc
dns.afsdb.c:                    * lines added to your /etc/request-key.conf file:

Relevant sections from the manpages:

man keyutils:
UTILITIES
The upcalling mechanism is usually routed via the request-key(8) program.  What this does with any particular key
is configurable in:

        /etc/request-key.conf
        /etc/request-key.d/

See the request-key.conf(5) and the request-key(8) manual pages for more information.
man request-key.conf:
DESCRIPTION
request-key looks for the best match, reading all the following files:

            /etc/request-key.d/*.conf
            /etc/request-key.conf

...

EXAMPLE
A basic file will be installed in the /etc. 

...

FILES
    /etc/request-key.conf
    /etc/request-key.d/*.conf
man request-key:
DESCRIPTION
-l     Use  configuration  from the current directory.  The program will use request-key.d/* and request-key.conf
        from the current directory rather than from /etc.

...

FILES
/etc/request-key.d/*.conf Individual configuration files.

/etc/request-key.conf Fallback configuration file.
man key.dns_resolver.conf:
DESCRIPTION
This  file is used by the key.dns_resolver(5) program to set parameters.  Unless otherwise overridden with the -c
flag, the program reads:

        /etc/key.dns_resolver.conf

...

FILES
/etc/key.dns_resolver.conf
man key.dns_resolver:
DESCRIPTION
There program has internal parameters that can be changed with a configuration file (see key.dns_resolver.conf(5)
for more information).  The default configuration file is in /etc, but this can be overridden with the -c flag.

Relevant sections from the sourcecode:

request-key.c: (issue)
if (!xlocaldirs) {
    scan_conf_dir(params, "/etc/request-key.d");
    scan_conf_file(params, AT_FDCWD, "/etc/request-key.conf");
} else {
    scan_conf_dir(params, "request-key.d");
    scan_conf_file(params, AT_FDCWD, "request-key.conf");
}

This section of code scans the config files; a sane sane location to make our stateless patch.

-		scan_conf_file(params, AT_FDCWD, "/etc/request-key.conf");
+		if (access("/etc/request-key.conf", F_OK)) {
+			scan_conf_file(params, AT_FDCWD, "/etc/request-key.conf");
+		} else {
+			scan_conf_file(params, AT_FDCWD, "/usr/share/defaults/keyutils/request-key.conf");
+		}
  • Use /etc/request-key.conf if present, else use /usr/share/defaults/keyutils/request-key.conf:

The current patch differs from the suggested modification; instead it modifies scan_conf_file.

    conf = fdopen(fd, "r");
-	if (!conf)
-		error("Cannot open %s: %m\n", conffile);
+	if (!conf) {
+		snprintf(conffile, sizeof(conffile) - 1, "/usr/share/defaults/etc/keyutils/request-key.conf");
+		conf = fopen(conffile, "r");
+		if (!conf)
+			error("Cannot open %s: %m\n", conffile);
+	}

If I understand this code correctly:

  • The patch loads /usr/share/defaults/etc/keyutils/request-key.conf if request-key.conf cannot be opened.
  • The patch does not respect the -l flag (Use configuration from the current directory).
    • It does not care if the default path is called (/etc) or a user explicit path (current directory) is called.
    • If an explicit path was called, the end user would likely expect the command to fail if the config file could not be opened for any reason (not continue with the default config).

When initially packaging keyutils on serpent-os with this patch, I recall running into a minor compiler warning which triggered my interest.

key.dns_resolver.c: (issue)
/*
* To use this program, you must tell /sbin/request-key how to invoke it.  You
* need to have the keyutils package installed and something like the following
* lines added to your /etc/request-key.conf file:
*/

static const char *config_file = "/etc/keyutils/key.dns_resolver.conf";

Note:

  • /etc/keyutils is not used anywhere else in the code base
  • All the documentation (man) state the path is /etc/key.dns_resolver.conf.

Resolution:

  1. Correct the config_file path to match the documentation
  2. Correct the manpages to reflect the new path
dns.afsdb.c:
/*
 * To use this program, you must tell /sbin/request-key how to invoke it.  You
 * need to have the keyutils package installed and something like the following
 * lines added to your /etc/request-key.conf file:
*/

Patch:
commit 925b2d44f193ad50f9bda5898f358d507b3a9353
Author: Benjamin Lowell <lowell.bv@gmail.com>
Date:   Wed Jul 24 23:37:21 2024 -0400

    Introduce statelessness by prioritizing `/etc/request-key.conf` if present; otherwise, fallback to `/usr/share/defaults/keyutils/request-key.conf`.
    Fix path for `key.dns_resolver.conf`; According `man key.dns_resolver.conf` the configuration file should be found in `/etc` (not `/etc/keyutils`).
    Updated mapages to include `/usr/share/defaults/keyutils/request-key.conf`.

diff --git a/Makefile b/Makefile
index d8d4ee3..9a1a923 100644
--- a/Makefile
+++ b/Makefile
@@ -7,9 +7,9 @@ SPECFILE	:= keyutils.spec
 NO_GLIBC_KEYERR	:= 0
 NO_ARLIB	:= 0
 NO_SOLIB	:= 0
-ETCDIR		:= /etc
 BINDIR		:= /bin
 SBINDIR		:= /sbin
+DEFAULTSDIR	:= /usr/share/defaults/keyutils/
 SHAREDIR	:= /usr/share/keyutils
 MANDIR		:= /usr/share/man
 MAN1		:= $(MANDIR)/man1
@@ -217,9 +217,7 @@ endif
 	$(INSTALL) -D request-key $(DESTDIR)$(SBINDIR)/request-key
 	$(INSTALL) -D request-key-debug.sh $(DESTDIR)$(SHAREDIR)/request-key-debug.sh
 	$(INSTALL) -D key.dns_resolver $(DESTDIR)$(SBINDIR)/key.dns_resolver
-	$(INSTALL) -D -m 0644 request-key.conf $(DESTDIR)$(ETCDIR)/request-key.conf
-	mkdir -p $(DESTDIR)$(ETCDIR)/request-key.d
-	mkdir -p $(DESTDIR)$(ETCDIR)/keyutils
+	$(INSTALL) -D -m 0644 request-key.conf $(DESTDIR)$(DEFAULTSDIR)/request-key.conf
 	mkdir -p $(DESTDIR)$(MAN1)
 	$(INSTALL) -m 0644 $(wildcard man/*.1) $(DESTDIR)$(MAN1)
 	mkdir -p $(DESTDIR)$(MAN3)
diff --git a/key.dns_resolver.c b/key.dns_resolver.c
index 7a7ec42..154ac61 100644
--- a/key.dns_resolver.c
+++ b/key.dns_resolver.c
@@ -46,7 +46,7 @@ static const char key_type[] = "dns_resolver";
 static const char a_query_type[] = "a";
 static const char aaaa_query_type[] = "aaaa";
 static const char afsdb_query_type[] = "afsdb";
-static const char *config_file = "/etc/keyutils/key.dns_resolver.conf";
+static const char *config_file = "/etc/key.dns_resolver.conf";
 static bool config_specified = false;
 key_serial_t key;
 static int verbose;
diff --git a/man/request-key.8 b/man/request-key.8
index 50a7506..fc36cf3 100644
--- a/man/request-key.8
+++ b/man/request-key.8
@@ -53,6 +53,11 @@ Individual configuration files.
 /etc/request\-key.conf
 .ul 0
 Fallback configuration file.
+.P
+.ul
+/usr/share/defaults/keyutils/request\-key.conf
+.ul 0
+System provided fallback configuration file.
 .SH SEE ALSO
 .ad l
 .nh
diff --git a/man/request-key.conf.5 b/man/request-key.conf.5
index 276c771..159d12d 100644
--- a/man/request-key.conf.5
+++ b/man/request-key.conf.5
@@ -20,6 +20,8 @@ request\-key looks for the best match, reading all the following files:
 	/etc/request\-key.d/*.conf
 .br
 	/etc/request\-key.conf
+.br
+	/usr/share/defaults/keyutils/request\-key.conf
 .P
 If it doesn't find a match, it will return an error
 and the kernel will automatically negate the key.
@@ -141,5 +143,9 @@ the payload.
 .ul
 /etc/request\-key.d/*.conf
 .ul 0
+.br
+.ul
+/usr/share/defaults/keyutils/request\-key.conf
+.ul 0
 .SH SEE ALSO
 \fBkeyctl\fR(1), \fBrequest\-key.conf\fR(5)
diff --git a/request-key.c b/request-key.c
index bf47c0a..e3c50a3 100644
--- a/request-key.c
+++ b/request-key.c
@@ -279,7 +279,11 @@ static void lookup_action(struct parameters *params)
 {
 	if (!xlocaldirs) {
 		scan_conf_dir(params, "/etc/request-key.d");
-		scan_conf_file(params, AT_FDCWD, "/etc/request-key.conf");
+		if (access("/etc/request-key.conf", F_OK)) {
+			scan_conf_file(params, AT_FDCWD, "/etc/request-key.conf");
+		} else {
+			scan_conf_file(params, AT_FDCWD, "/usr/share/defaults/keyutils/request-key.conf");
+		}
 	} else {
 		scan_conf_dir(params, "request-key.d");
 		scan_conf_file(params, AT_FDCWD, "request-key.conf");

Notes:

  • I am confident in my patch as the changes are minimal, simple and straight forward.
  • I would describe my c coding skills as at a "beginner level", so feel free to scrutinize my alterations.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions

      0