2020-07-10 16:38:12

by Alice Mitchell

[permalink] [raw]
Subject: [PATCH 0/4] nfs-utils: nfs.conf features to enable use of machine-id as nfs4_unique_id

This patch set introduces some additional features to the nfs.conf tool
chain that allows automatic use of /etc/machine-id or other unique
values for setups that otherwise do not have a unique hostname or disk
image and would thus otherwise generate non-unique EXCHANGE_ID and
SETCLIENTID messages.

Signed-off-by: Alice Mitchell <[email protected]>


2020-07-10 16:40:36

by Alice Mitchell

[permalink] [raw]
Subject: [PATCH 1/4] nfs-utils: Factor out common structure cleanup calls


support/nfs/conffile.c | 84 +++++++++++++++++++++---------------------
1 file changed, 41 insertions(+), 43 deletions(-)

diff --git a/support/nfs/conffile.c b/support/nfs/conffile.c
index 3d13610e..aea35917 100644
--- a/support/nfs/conffile.c
+++ b/support/nfs/conffile.c
@@ -128,6 +128,39 @@ conf_hash(const char *s)
return hash;
}

+/*
+ * free all the component parts of a conf_binding struct
+ */
+static void free_confbind(struct conf_binding *cb)
+{
+ if (!cb)
+ return;
+ if (cb->section)
+ free(cb->section);
+ if (cb->arg)
+ free(cb->arg);
+ if (cb->tag)
+ free(cb->tag);
+ if (cb->value)
+ free(cb->value);
+ free(cb);
+}
+
+static void free_conftrans(struct conf_trans *ct)
+{
+ if (!ct)
+ return;
+ if (ct->section)
+ free(ct->section);
+ if (ct->arg)
+ free(ct->arg);
+ if (ct->tag)
+ free(ct->tag);
+ if (ct->value)
+ free(ct->value);
+ free(ct);
+}
+
/*
* Insert a tag-value combination from LINE (the equal sign is at POS)
*/
@@ -143,11 +176,7 @@ conf_remove_now(const char *section, const char *tag)
&& strcasecmp(cb->tag, tag) == 0) {
LIST_REMOVE(cb, link);
xlog(LOG_INFO,"[%s]:%s->%s removed", section, tag, cb->value);
- free(cb->section);
- free(cb->arg);
- free(cb->tag);
- free(cb->value);
- free(cb);
+ free_confbind(cb);
return 0;
}
}
@@ -167,11 +196,7 @@ conf_remove_section_now(const char *section)
unseen = 0;
LIST_REMOVE(cb, link);
xlog(LOG_INFO, "[%s]:%s->%s removed", section, cb->tag, cb->value);
- free(cb->section);
- free(cb->arg);
- free(cb->tag);
- free(cb->value);
- free(cb);
+ free_confbind(cb);
}
}
return unseen;
@@ -567,11 +592,7 @@ static void conf_free_bindings(void)
for (; cb; cb = next) {
next = LIST_NEXT(cb, link);
LIST_REMOVE(cb, link);
- free(cb->section);
- free(cb->arg);
- free(cb->tag);
- free(cb->value);
- free(cb);
+ free_confbind(cb);
}
LIST_INIT(&conf_bindings[i]);
}
@@ -635,11 +656,7 @@ conf_cleanup(void)
for (node = TAILQ_FIRST(&conf_trans_queue); node; node = next) {
next = TAILQ_NEXT(node, link);
TAILQ_REMOVE (&conf_trans_queue, node, link);
- if (node->section) free(node->section);
- if (node->arg) free(node->arg);
- if (node->tag) free(node->tag);
- if (node->value) free(node->value);
- free (node);
+ free_conftrans(node);
}
TAILQ_INIT(&conf_trans_queue);
}
@@ -1005,14 +1022,7 @@ conf_set(int transaction, const char *section, const char *arg,
return 0;

fail:
- if (node->tag)
- free(node->tag);
- if (node->arg)
- free(node->arg);
- if (node->section)
- free(node->section);
- if (node)
- free(node);
+ free_conftrans(node);
return 1;
}

@@ -1038,10 +1048,7 @@ conf_remove(int transaction, const char *section, const char *tag)
return 0;

fail:
- if (node && node->section)
- free (node->section);
- if (node)
- free (node);
+ free_conftrans(node);
return 1;
}

@@ -1062,8 +1069,7 @@ conf_remove_section(int transaction, const char *section)
return 0;

fail:
- if (node)
- free(node);
+ free_conftrans(node);
return 1;
}

@@ -1094,15 +1100,7 @@ conf_end(int transaction, int commit)
}
}
TAILQ_REMOVE (&conf_trans_queue, node, link);
- if (node->section)
- free(node->section);
- if (node->arg)
- free(node->arg);
- if (node->tag)
- free(node->tag);
- if (node->value)
- free(node->value);
- free (node);
+ free_conftrans(node);
}
}
return 0;
--
2.18.1


2020-07-10 16:42:54

by Alice Mitchell

[permalink] [raw]
Subject: [PATCH 3/4] nfs-utils: Add support for further ${variable} expansions in nfs.conf

This adds support for substituting in the systems machine_id as well as random generated uuid or hostname,
and caches the results
---
support/nfs/conffile.c | 268 +++++++++++++++++++++++++++++++++++++++--
1 file changed, 257 insertions(+), 11 deletions(-)

diff --git a/support/nfs/conffile.c b/support/nfs/conffile.c
index cbeef10d..58c03911 100644
--- a/support/nfs/conffile.c
+++ b/support/nfs/conffile.c
@@ -40,6 +40,7 @@
#include <sys/stat.h>
#include <netinet/in.h>
#include <arpa/inet.h>
+#include <linux/if_alg.h>
#include <ctype.h>
#include <fcntl.h>
#include <stdio.h>
@@ -110,12 +111,66 @@ struct conf_binding {
char *tag;
char *value;
int is_default;
+ char *cache;
};

LIST_HEAD (conf_bindings, conf_binding) conf_bindings[256];

+typedef char * (*expand_fn_t)(void);
+struct expansion_types {
+ const char *name;
+ expand_fn_t func;
+};
+
+typedef struct {
+ uint8_t bytes[16];
+} id128_t;
+
+/*
+ * Application ID for use with generating a machine-id string
+ */
+static id128_t nfs_appid = {.bytes = {0xff,0x3b,0xf0,0x0f,0x34,0xa6,0x43,0xc5, \
+ 0x93,0xdd,0x16,0xdc,0x7c,0xeb,0x88,0xc8}};
+
const char *modified_by = NULL;

+static __inline__ char
+hexchar(int x) {
+ static const char table[16] = "0123456789abcdef";
+ return table[x & 15];
+}
+
+static __inline__ int
+unhexchar(char h)
+{
+ if (h >= '0' && h <= '9')
+ return h - '0';
+ if (h >= 'a' && h <= 'f')
+ return h - 'a' + 10;
+ if (h >= 'A' && h <= 'F')
+ return h - 'A' + 10;
+ return -1;
+}
+
+static char *
+tohexstr(const unsigned char *data, int len)
+{
+ int i;
+ char *result = NULL;
+
+ result = calloc(1, (len*2)+1);
+ if (!result) {
+ xlog(L_ERROR, "malloc error formatting string");
+ return NULL;
+ }
+
+ for (i = 0; i < len; i++) {
+ result[i*2] = hexchar(data[i] >> 4);
+ result[i*2+1] = hexchar(data[i] & 0x0F);
+ }
+ return result;
+}
+
static __inline__ uint8_t
conf_hash(const char *s)
{
@@ -128,6 +183,201 @@ conf_hash(const char *s)
return hash;
}

+static int
+id128_from_string(const char s[], id128_t *ret)
+{
+ id128_t t;
+ unsigned int n, i;
+ for (n=0, i=0; n<16; ) {
+ int a, b;
+ a = unhexchar(s[i++]);
+ if (a < 0)
+ return 1;
+ b = unhexchar(s[i++]);
+ if (b < 0)
+ return 1;
+
+ t.bytes[n++] = (a << 4) | b;
+ }
+ if (s[i] != 0)
+ return 1;
+ if (ret)
+ *ret = t;
+ return 0;
+}
+
+/*
+ * cryptographic hash (sha256) data into a hex encoded string
+ */
+static char *
+strhash(unsigned char *key, size_t keylen, unsigned char *data, size_t dlen)
+{
+ union {
+ struct sockaddr sa;
+ struct sockaddr_alg alg;
+ } sa;
+ int sock = -1;
+ int hfd = -1;
+ uint8_t digest[129];
+ int n;
+ char *result = NULL;
+
+ memset(&sa, 0, sizeof(sa));
+ sa.alg.salg_family = AF_ALG;
+ strcpy((char *)sa.alg.salg_type, "hash");
+ strcpy((char *)sa.alg.salg_name, "hmac(sha256)");
+
+ sock = socket(AF_ALG, SOCK_SEQPACKET|SOCK_CLOEXEC, 0);
+ if (sock < 0) {
+ xlog(L_ERROR, "error creating socket");
+ goto cleanup;
+ }
+
+ if (bind(sock, (struct sockaddr *)&sa.sa, sizeof(sa)) < 0) {
+ xlog(L_ERROR, "error opening khash interface");
+ goto cleanup;
+ }
+
+ if (key && keylen > 0) {
+ if (setsockopt(sock, SOL_ALG, ALG_SET_KEY, key, keylen) < 0) {
+ xlog(L_ERROR, "Error setting key: %s", strerror(errno));
+ goto cleanup;
+ }
+ }
+
+ hfd = accept4(sock, NULL, 0, SOCK_CLOEXEC);
+ if (hfd < 0) {
+ xlog(L_ERROR, "Error initiating khash: %s", strerror(errno));
+ goto cleanup;
+ }
+
+ n = send(hfd, data, dlen, 0);
+ if (n < 0) {
+ xlog(L_ERROR, "Error updating khash: %s", strerror(errno));
+ goto cleanup;
+ }
+
+ n = recv(hfd, digest, sizeof(digest), 0);
+ if (n < 0) {
+ xlog(L_ERROR, "Error fetching khash: %s", strerror(errno));
+ goto cleanup;
+ }
+
+ result = tohexstr(digest, n);
+cleanup:
+ if (sock != -1)
+ close(sock);
+ if (hfd != -1)
+ close(hfd);
+ if (hfd != -1)
+ close(hfd);
+
+ return result;
+}
+
+/*
+ * Read one line of content from a file
+ */
+static char *
+read_oneline(const char *filename)
+{
+ char *content = conf_readfile(filename);
+ char *end;
+
+ if (content == NULL)
+ return NULL;
+
+ /* trim to only the first line */
+ end = strchr(content, '\n');
+ if (end != NULL)
+ *end = '\0';
+ end = strchr(content, '\r');
+ if (end != NULL)
+ *end = '\0';
+
+ return content;
+}
+
+static char *
+expand_machine_id(void)
+{
+ char *key = read_oneline("/etc/machine-id");
+ id128_t mid;
+ char * result = NULL;
+ size_t idlen = 0;
+
+ if (key == NULL)
+ return NULL;
+
+ idlen = strlen(key);
+ if (!id128_from_string(key, &mid)) {
+ result = strhash(mid.bytes, sizeof(mid), nfs_appid.bytes, sizeof(nfs_appid));
+ if (result && strlen(result) > idlen)
+ result[idlen]=0;
+ }
+ free(key);
+ return result;
+}
+
+static char *
+expand_random_uuid(void)
+{
+ return read_oneline("/proc/sys/kernel/random/uuid");
+}
+
+static char *
+expand_hostname(void)
+{
+ int maxlen = HOST_NAME_MAX + 1;
+ char * hostname = calloc(1, maxlen);
+
+ if (!hostname)
+ return NULL;
+ if ((gethostname(hostname, maxlen)) == -1) {
+ free(hostname);
+ return NULL;
+ }
+ return hostname;
+}
+
+static struct expansion_types var_expansions[] = {
+ { "machine_id", expand_machine_id },
+ { "machine-id", expand_machine_id },
+ { "random-uuid", expand_random_uuid },
+ { "hostname", expand_hostname },
+};
+
+/* Deal with more complex variable substitutions */
+static char *
+expand_variable(const char *name)
+{
+ size_t len;
+
+ if (name == NULL || name[0] != '$')
+ return NULL;
+
+ len = strlen(name);
+ if (name[1] == '{' && name[len-1] == '}') {
+ char *varname = strndupa(&name[2], len-3);
+
+ for (size_t i=0; i<sizeof(var_expansions); i++) {
+ if (!strcasecmp(varname, var_expansions[i].name)) {
+ return var_expansions[i].func();
+ }
+ }
+ xlog_warn("get_conf: Unknown variable ${%s}", varname);
+ } else {
+ /* expand $name from [environment] section,
+ * or from environment
+ */
+ char *env = getenv(&name[1]);
+ if (env == NULL || *env == 0)
+ env = conf_get_section("environment", NULL, &name[1]);
+ return env;
+ }
+ return NULL;
+}
+
/*
* free all the component parts of a conf_binding struct
*/
@@ -143,6 +393,8 @@ static void free_confbind(struct conf_binding *cb)
free(cb->tag);
if (cb->value)
free(cb->value);
+ if (cb->cache)
+ free(cb->cache);
free(cb);
}

@@ -782,7 +1034,7 @@ char *
conf_get_section(const char *section, const char *arg, const char *tag)
{
struct conf_binding *cb;
-retry:
+
cb = LIST_FIRST (&conf_bindings[conf_hash (section)]);
for (; cb; cb = LIST_NEXT (cb, link)) {
if (strcasecmp(section, cb->section) != 0)
@@ -794,19 +1046,13 @@ retry:
if (strcasecmp(tag, cb->tag) != 0)
continue;
if (cb->value[0] == '$') {
- /* expand $name from [environment] section,
- * or from environment
- */
- char *env = getenv(cb->value+1);
- if (env && *env)
- return env;
- section = "environment";
- tag = cb->value + 1;
- goto retry;
+ if (!cb->cache)
+ cb->cache = expand_variable(cb->value);
+ return cb->cache;
}
return cb->value;
}
- return 0;
+ return NULL;
}

/*
--
2.18.1


2020-07-10 16:43:14

by Alice Mitchell

[permalink] [raw]
Subject: [PATCH 2/4] nfs-utils: Enable the retrieval of raw config settings without expansion

Config entries sometimes contain variable expansions, this adds options
to retrieve the config entry rather than its current expanded value.
---
support/include/conffile.h | 1 +
support/nfs/conffile.c | 23 +++++++++++++++++++++++
tools/nfsconf/nfsconfcli.c | 22 ++++++++++++++++------
3 files changed, 40 insertions(+), 6 deletions(-)

diff --git a/support/include/conffile.h b/support/include/conffile.h
index 7d974fe9..c4a3ca62 100644
--- a/support/include/conffile.h
+++ b/support/include/conffile.h
@@ -61,6 +61,7 @@ extern _Bool conf_get_bool(const char *, const char *, _Bool);
extern char *conf_get_str(const char *, const char *);
extern char *conf_get_str_with_def(const char *, const char *, char *);
extern char *conf_get_section(const char *, const char *, const char *);
+extern char *conf_get_entry(const char *, const char *, const char *);
extern int conf_init_file(const char *);
extern void conf_cleanup(void);
extern int conf_match_num(const char *, const char *, int);
diff --git a/support/nfs/conffile.c b/support/nfs/conffile.c
index aea35917..cbeef10d 100644
--- a/support/nfs/conffile.c
+++ b/support/nfs/conffile.c
@@ -752,6 +752,29 @@ conf_get_str_with_def(const char *section, const char *tag, char *def)
return result;
}

+/*
+ * Retrieve an entry without interpreting its contents
+ */
+char *
+conf_get_entry(const char *section, const char *arg, const char *tag)
+{
+ struct conf_binding *cb;
+
+ cb = LIST_FIRST (&conf_bindings[conf_hash (section)]);
+ for (; cb; cb = LIST_NEXT (cb, link)) {
+ if (strcasecmp(section, cb->section) != 0)
+ continue;
+ if (arg && (cb->arg == NULL || strcasecmp(arg, cb->arg) != 0))
+ continue;
+ if (!arg && cb->arg)
+ continue;
+ if (strcasecmp(tag, cb->tag) != 0)
+ continue;
+ return cb->value;
+ }
+ return 0;
+}
+
/*
* Find a section that may or may not have an argument
*/
diff --git a/tools/nfsconf/nfsconfcli.c b/tools/nfsconf/nfsconfcli.c
index 361d386e..b2ef96d1 100644
--- a/tools/nfsconf/nfsconfcli.c
+++ b/tools/nfsconf/nfsconfcli.c
@@ -11,6 +11,7 @@
typedef enum {
MODE_NONE,
MODE_GET,
+ MODE_ENTRY,
MODE_ISSET,
MODE_DUMP,
MODE_SET,
@@ -30,6 +31,8 @@ static void usage(const char *name)
fprintf(stderr, " Outputs the configuration to the named file\n");
fprintf(stderr, " --get [--arg subsection] {section} {tag}\n");
fprintf(stderr, " Output one specific config value\n");
+ fprintf(stderr, " --entry [--arg subsection] {section} {tag}\n");
+ fprintf(stderr, " Output the uninterpreted config entry\n");
fprintf(stderr, " --isset [--arg subsection] {section} {tag}\n");
fprintf(stderr, " Return code indicates if config value is present\n");
fprintf(stderr, " --set [--arg subsection] {section} {tag} {value}\n");
@@ -55,6 +58,7 @@ int main(int argc, char **argv)
int index = 0;
struct option long_options[] = {
{"get", no_argument, 0, 'g' },
+ {"entry", no_argument, 0, 'e' },
{"set", no_argument, 0, 's' },
{"unset", no_argument, 0, 'u' },
{"arg", required_argument, 0, 'a' },
@@ -66,7 +70,7 @@ int main(int argc, char **argv)
{NULL, 0, 0, 0 }
};

- c = getopt_long(argc, argv, "gsua:id::f:vm:", long_options, &index);
+ c = getopt_long(argc, argv, "gesua:id::f:vm:", long_options, &index);
if (c == -1) break;

switch (c) {
@@ -86,6 +90,9 @@ int main(int argc, char **argv)
case 'g':
mode = MODE_GET;
break;
+ case 'e':
+ mode = MODE_ENTRY;
+ break;
case 's':
mode = MODE_SET;
break;
@@ -167,8 +174,8 @@ int main(int argc, char **argv)
if (dumpfile)
fclose(out);
} else
- /* --iset and --get share a lot of code */
- if (mode == MODE_GET || mode == MODE_ISSET) {
+ /* --isset and --get share a lot of code */
+ if (mode == MODE_GET || mode == MODE_ISSET || mode == MODE_ENTRY) {
char * section = NULL;
char * tag = NULL;
const char * val;
@@ -186,14 +193,17 @@ int main(int argc, char **argv)
tag = argv[optind++];

/* retrieve the specified tags value */
- val = conf_get_section(section, arg, tag);
+ if (mode == MODE_ENTRY)
+ val = conf_get_entry(section, arg, tag);
+ else
+ val = conf_get_section(section, arg, tag);
if (val != NULL) {
/* ret=0, success, mode --get wants to output the value as well */
- if (mode == MODE_GET)
+ if (mode != MODE_ISSET)
printf("%s\n", val);
} else {
/* ret=1, no value found, tell the user if they asked */
- if (mode == MODE_GET && verbose)
+ if (mode != MODE_ISSET && verbose)
fprintf(stderr, "Tag '%s' not found\n", tag);
ret = 1;
}
--
2.18.1


2020-07-10 16:44:39

by Alice Mitchell

[permalink] [raw]
Subject: [PATCH 4/4] nfs-utils: Update nfs4_unique_id module parameter from the nfs.conf value

systemd service to grab the config value and feed it to the kernel module
---
nfs.conf | 1 +
systemd/Makefile.am | 3 +++
systemd/nfs-conf-export.sh | 28 ++++++++++++++++++++++++++++
systemd/nfs-config.service.in | 17 +++++++++++++++++
4 files changed, 49 insertions(+)
create mode 100755 systemd/nfs-conf-export.sh
create mode 100644 systemd/nfs-config.service.in

diff --git a/nfs.conf b/nfs.conf
index 186a5b19..8bb41227 100644
--- a/nfs.conf
+++ b/nfs.conf
@@ -4,6 +4,7 @@
#
[general]
# pipefs-directory=/var/lib/nfs/rpc_pipefs
+# nfs4_unique_id = ${machine-id}
#
[exports]
# rootdir=/export
diff --git a/systemd/Makefile.am b/systemd/Makefile.am
index 75cdd9f5..51acdc3f 100644
--- a/systemd/Makefile.am
+++ b/systemd/Makefile.am
@@ -9,6 +9,7 @@ unit_files = \
nfs-mountd.service \
nfs-server.service \
nfs-utils.service \
+ nfs-config.service \
rpc-statd-notify.service \
rpc-statd.service \
\
@@ -69,4 +70,6 @@ genexec_PROGRAMS = nfs-server-generator rpc-pipefs-generator
install-data-hook: $(unit_files)
mkdir -p $(DESTDIR)/$(unitdir)
cp $(unit_files) $(DESTDIR)/$(unitdir)
+ mkdir -p $(DESTDIR)/$(libexecdir)/nfs-utils
+ install nfs-conf-export.sh $(DESTDIR)/$(libexecdir)/nfs-utils/
endif
diff --git a/systemd/nfs-conf-export.sh b/systemd/nfs-conf-export.sh
new file mode 100755
index 00000000..486e8df9
--- /dev/null
+++ b/systemd/nfs-conf-export.sh
@@ -0,0 +1,28 @@
+#!/bin/bash
+#
+# This script pulls values out of /etc/nfs.conf and configures
+# the appropriate kernel modules which cannot read it directly
+
+NFSMOD=/sys/module/nfs/parameters/nfs4_unique_id
+NFSPROBE=/etc/modprobe.d/nfs.conf
+
+# Now read the values from nfs.conf
+MACHINEID=`nfsconf --get general nfs4_unique_id`
+if [ $? -ne 0 ] || [ "$MACHINEID" == "" ]
+then
+# No config vaue found, assume blank
+MACHINEID=""
+fi
+
+# Kernel module is already loaded, update the live one
+if [ -e $NFSMOD ]; then
+echo -n "$MACHINEID" >> $NFSMOD
+fi
+
+# Rewrite the modprobe file for next reboot
+echo "# This file is overwritten by systemd nfs-config.service" > $NFSPROBE
+echo "# with values taken from /etc/nfs.conf" >> $NFSPROBE
+echo "# Do not hand modify" >> $NFSPROBE
+echo "options nfs nfs4_unique_id=\"$MACHINEID\"" >> $NFSPROBE
+
+echo "Set to: $MACHINEID"
diff --git a/systemd/nfs-config.service.in b/systemd/nfs-config.service.in
new file mode 100644
index 00000000..c5ef1024
--- /dev/null
+++ b/systemd/nfs-config.service.in
@@ -0,0 +1,17 @@
+[Unit]
+Description=Preprocess NFS configuration
+PartOf=nfs-client.target
+After=nfs-client.target
+DefaultDependencies=no
+
+[Service]
+Type=oneshot
+# This service needs to run any time any nfs service
+# is started, so changes to local config files get
+# incorporated. Having "RemainAfterExit=no" (the default)
+# ensures this happens.
+RemainAfterExit=no
+ExecStart=@_libexecdir@/nfs-utils/nfs-conf-export.sh
+
+[Install]
+WantedBy=nfs-client.target
--
2.18.1


2020-07-15 17:44:58

by Steve Dickson

[permalink] [raw]
Subject: Re: [PATCH 4/4] nfs-utils: Update nfs4_unique_id module parameter from the nfs.conf value

Hello,

On 7/10/20 12:44 PM, Alice Mitchell wrote:
> systemd service to grab the config value and feed it to the kernel module
Again, I'm wondering if the systemd/README should be updated to explain
this new script...

steved.

> ---
> nfs.conf | 1 +
> systemd/Makefile.am | 3 +++
> systemd/nfs-conf-export.sh | 28 ++++++++++++++++++++++++++++
> systemd/nfs-config.service.in | 17 +++++++++++++++++
> 4 files changed, 49 insertions(+)
> create mode 100755 systemd/nfs-conf-export.sh
> create mode 100644 systemd/nfs-config.service.in
>
> diff --git a/nfs.conf b/nfs.conf
> index 186a5b19..8bb41227 100644
> --- a/nfs.conf
> +++ b/nfs.conf
> @@ -4,6 +4,7 @@
> #
> [general]
> # pipefs-directory=/var/lib/nfs/rpc_pipefs
> +# nfs4_unique_id = ${machine-id}
> #
> [exports]
> # rootdir=/export
> diff --git a/systemd/Makefile.am b/systemd/Makefile.am
> index 75cdd9f5..51acdc3f 100644
> --- a/systemd/Makefile.am
> +++ b/systemd/Makefile.am
> @@ -9,6 +9,7 @@ unit_files = \
> nfs-mountd.service \
> nfs-server.service \
> nfs-utils.service \
> + nfs-config.service \
> rpc-statd-notify.service \
> rpc-statd.service \
> \
> @@ -69,4 +70,6 @@ genexec_PROGRAMS = nfs-server-generator rpc-pipefs-generator
> install-data-hook: $(unit_files)
> mkdir -p $(DESTDIR)/$(unitdir)
> cp $(unit_files) $(DESTDIR)/$(unitdir)
> + mkdir -p $(DESTDIR)/$(libexecdir)/nfs-utils
> + install nfs-conf-export.sh $(DESTDIR)/$(libexecdir)/nfs-utils/
> endif
> diff --git a/systemd/nfs-conf-export.sh b/systemd/nfs-conf-export.sh
> new file mode 100755
> index 00000000..486e8df9
> --- /dev/null
> +++ b/systemd/nfs-conf-export.sh
> @@ -0,0 +1,28 @@
> +#!/bin/bash
> +#
> +# This script pulls values out of /etc/nfs.conf and configures
> +# the appropriate kernel modules which cannot read it directly
> +
> +NFSMOD=/sys/module/nfs/parameters/nfs4_unique_id
> +NFSPROBE=/etc/modprobe.d/nfs.conf
> +
> +# Now read the values from nfs.conf
> +MACHINEID=`nfsconf --get general nfs4_unique_id`
> +if [ $? -ne 0 ] || [ "$MACHINEID" == "" ]
> +then
> +# No config vaue found, assume blank
> +MACHINEID=""
> +fi
> +
> +# Kernel module is already loaded, update the live one
> +if [ -e $NFSMOD ]; then
> +echo -n "$MACHINEID" >> $NFSMOD
> +fi
> +
> +# Rewrite the modprobe file for next reboot
> +echo "# This file is overwritten by systemd nfs-config.service" > $NFSPROBE
> +echo "# with values taken from /etc/nfs.conf" >> $NFSPROBE
> +echo "# Do not hand modify" >> $NFSPROBE
> +echo "options nfs nfs4_unique_id=\"$MACHINEID\"" >> $NFSPROBE
> +
> +echo "Set to: $MACHINEID"
> diff --git a/systemd/nfs-config.service.in b/systemd/nfs-config.service.in
> new file mode 100644
> index 00000000..c5ef1024
> --- /dev/null
> +++ b/systemd/nfs-config.service.in
> @@ -0,0 +1,17 @@
> +[Unit]
> +Description=Preprocess NFS configuration
> +PartOf=nfs-client.target
> +After=nfs-client.target
> +DefaultDependencies=no
> +
> +[Service]
> +Type=oneshot
> +# This service needs to run any time any nfs service
> +# is started, so changes to local config files get
> +# incorporated. Having "RemainAfterExit=no" (the default)
> +# ensures this happens.
> +RemainAfterExit=no
> +ExecStart=@_libexecdir@/nfs-utils/nfs-conf-export.sh
> +
> +[Install]
> +WantedBy=nfs-client.target
>

2020-07-15 17:45:37

by Steve Dickson

[permalink] [raw]
Subject: Re: [PATCH 3/4] nfs-utils: Add support for further ${variable} expansions in nfs.conf

Hello,

On 7/10/20 12:42 PM, Alice Mitchell wrote:
> This adds support for substituting in the systems machine_id as well as random generated uuid or hostname,
> and caches the results
Just curious... should the nfs.conf man page be updated to explain this new support?

steved.
> ---
> support/nfs/conffile.c | 268 +++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 257 insertions(+), 11 deletions(-)
>
> diff --git a/support/nfs/conffile.c b/support/nfs/conffile.c
> index cbeef10d..58c03911 100644
> --- a/support/nfs/conffile.c
> +++ b/support/nfs/conffile.c
> @@ -40,6 +40,7 @@
> #include <sys/stat.h>
> #include <netinet/in.h>
> #include <arpa/inet.h>
> +#include <linux/if_alg.h>
> #include <ctype.h>
> #include <fcntl.h>
> #include <stdio.h>
> @@ -110,12 +111,66 @@ struct conf_binding {
> char *tag;
> char *value;
> int is_default;
> + char *cache;
> };
>
> LIST_HEAD (conf_bindings, conf_binding) conf_bindings[256];
>
> +typedef char * (*expand_fn_t)(void);
> +struct expansion_types {
> + const char *name;
> + expand_fn_t func;
> +};
> +
> +typedef struct {
> + uint8_t bytes[16];
> +} id128_t;
> +
> +/*
> + * Application ID for use with generating a machine-id string
> + */
> +static id128_t nfs_appid = {.bytes = {0xff,0x3b,0xf0,0x0f,0x34,0xa6,0x43,0xc5, \
> + 0x93,0xdd,0x16,0xdc,0x7c,0xeb,0x88,0xc8}};
> +
> const char *modified_by = NULL;
>
> +static __inline__ char
> +hexchar(int x) {
> + static const char table[16] = "0123456789abcdef";
> + return table[x & 15];
> +}
> +
> +static __inline__ int
> +unhexchar(char h)
> +{
> + if (h >= '0' && h <= '9')
> + return h - '0';
> + if (h >= 'a' && h <= 'f')
> + return h - 'a' + 10;
> + if (h >= 'A' && h <= 'F')
> + return h - 'A' + 10;
> + return -1;
> +}
> +
> +static char *
> +tohexstr(const unsigned char *data, int len)
> +{
> + int i;
> + char *result = NULL;
> +
> + result = calloc(1, (len*2)+1);
> + if (!result) {
> + xlog(L_ERROR, "malloc error formatting string");
> + return NULL;
> + }
> +
> + for (i = 0; i < len; i++) {
> + result[i*2] = hexchar(data[i] >> 4);
> + result[i*2+1] = hexchar(data[i] & 0x0F);
> + }
> + return result;
> +}
> +
> static __inline__ uint8_t
> conf_hash(const char *s)
> {
> @@ -128,6 +183,201 @@ conf_hash(const char *s)
> return hash;
> }
>
> +static int
> +id128_from_string(const char s[], id128_t *ret)
> +{
> + id128_t t;
> + unsigned int n, i;
> + for (n=0, i=0; n<16; ) {
> + int a, b;
> + a = unhexchar(s[i++]);
> + if (a < 0)
> + return 1;
> + b = unhexchar(s[i++]);
> + if (b < 0)
> + return 1;
> +
> + t.bytes[n++] = (a << 4) | b;
> + }
> + if (s[i] != 0)
> + return 1;
> + if (ret)
> + *ret = t;
> + return 0;
> +}
> +
> +/*
> + * cryptographic hash (sha256) data into a hex encoded string
> + */
> +static char *
> +strhash(unsigned char *key, size_t keylen, unsigned char *data, size_t dlen)
> +{
> + union {
> + struct sockaddr sa;
> + struct sockaddr_alg alg;
> + } sa;
> + int sock = -1;
> + int hfd = -1;
> + uint8_t digest[129];
> + int n;
> + char *result = NULL;
> +
> + memset(&sa, 0, sizeof(sa));
> + sa.alg.salg_family = AF_ALG;
> + strcpy((char *)sa.alg.salg_type, "hash");
> + strcpy((char *)sa.alg.salg_name, "hmac(sha256)");
> +
> + sock = socket(AF_ALG, SOCK_SEQPACKET|SOCK_CLOEXEC, 0);
> + if (sock < 0) {
> + xlog(L_ERROR, "error creating socket");
> + goto cleanup;
> + }
> +
> + if (bind(sock, (struct sockaddr *)&sa.sa, sizeof(sa)) < 0) {
> + xlog(L_ERROR, "error opening khash interface");
> + goto cleanup;
> + }
> +
> + if (key && keylen > 0) {
> + if (setsockopt(sock, SOL_ALG, ALG_SET_KEY, key, keylen) < 0) {
> + xlog(L_ERROR, "Error setting key: %s", strerror(errno));
> + goto cleanup;
> + }
> + }
> +
> + hfd = accept4(sock, NULL, 0, SOCK_CLOEXEC);
> + if (hfd < 0) {
> + xlog(L_ERROR, "Error initiating khash: %s", strerror(errno));
> + goto cleanup;
> + }
> +
> + n = send(hfd, data, dlen, 0);
> + if (n < 0) {
> + xlog(L_ERROR, "Error updating khash: %s", strerror(errno));
> + goto cleanup;
> + }
> +
> + n = recv(hfd, digest, sizeof(digest), 0);
> + if (n < 0) {
> + xlog(L_ERROR, "Error fetching khash: %s", strerror(errno));
> + goto cleanup;
> + }
> +
> + result = tohexstr(digest, n);
> +cleanup:
> + if (sock != -1)
> + close(sock);
> + if (hfd != -1)
> + close(hfd);
> + if (hfd != -1)
> + close(hfd);
> +
> + return result;
> +}
> +
> +/*
> + * Read one line of content from a file
> + */
> +static char *
> +read_oneline(const char *filename)
> +{
> + char *content = conf_readfile(filename);
> + char *end;
> +
> + if (content == NULL)
> + return NULL;
> +
> + /* trim to only the first line */
> + end = strchr(content, '\n');
> + if (end != NULL)
> + *end = '\0';
> + end = strchr(content, '\r');
> + if (end != NULL)
> + *end = '\0';
> +
> + return content;
> +}
> +
> +static char *
> +expand_machine_id(void)
> +{
> + char *key = read_oneline("/etc/machine-id");
> + id128_t mid;
> + char * result = NULL;
> + size_t idlen = 0;
> +
> + if (key == NULL)
> + return NULL;
> +
> + idlen = strlen(key);
> + if (!id128_from_string(key, &mid)) {
> + result = strhash(mid.bytes, sizeof(mid), nfs_appid.bytes, sizeof(nfs_appid));
> + if (result && strlen(result) > idlen)
> + result[idlen]=0;
> + }
> + free(key);
> + return result;
> +}
> +
> +static char *
> +expand_random_uuid(void)
> +{
> + return read_oneline("/proc/sys/kernel/random/uuid");
> +}
> +
> +static char *
> +expand_hostname(void)
> +{
> + int maxlen = HOST_NAME_MAX + 1;
> + char * hostname = calloc(1, maxlen);
> +
> + if (!hostname)
> + return NULL;
> + if ((gethostname(hostname, maxlen)) == -1) {
> + free(hostname);
> + return NULL;
> + }
> + return hostname;
> +}
> +
> +static struct expansion_types var_expansions[] = {
> + { "machine_id", expand_machine_id },
> + { "machine-id", expand_machine_id },
> + { "random-uuid", expand_random_uuid },
> + { "hostname", expand_hostname },
> +};
> +
> +/* Deal with more complex variable substitutions */
> +static char *
> +expand_variable(const char *name)
> +{
> + size_t len;
> +
> + if (name == NULL || name[0] != '$')
> + return NULL;
> +
> + len = strlen(name);
> + if (name[1] == '{' && name[len-1] == '}') {
> + char *varname = strndupa(&name[2], len-3);
> +
> + for (size_t i=0; i<sizeof(var_expansions); i++) {
> + if (!strcasecmp(varname, var_expansions[i].name)) {
> + return var_expansions[i].func();
> + }
> + }
> + xlog_warn("get_conf: Unknown variable ${%s}", varname);
> + } else {
> + /* expand $name from [environment] section,
> + * or from environment
> + */
> + char *env = getenv(&name[1]);
> + if (env == NULL || *env == 0)
> + env = conf_get_section("environment", NULL, &name[1]);
> + return env;
> + }
> + return NULL;
> +}
> +
> /*
> * free all the component parts of a conf_binding struct
> */
> @@ -143,6 +393,8 @@ static void free_confbind(struct conf_binding *cb)
> free(cb->tag);
> if (cb->value)
> free(cb->value);
> + if (cb->cache)
> + free(cb->cache);
> free(cb);
> }
>
> @@ -782,7 +1034,7 @@ char *
> conf_get_section(const char *section, const char *arg, const char *tag)
> {
> struct conf_binding *cb;
> -retry:
> +
> cb = LIST_FIRST (&conf_bindings[conf_hash (section)]);
> for (; cb; cb = LIST_NEXT (cb, link)) {
> if (strcasecmp(section, cb->section) != 0)
> @@ -794,19 +1046,13 @@ retry:
> if (strcasecmp(tag, cb->tag) != 0)
> continue;
> if (cb->value[0] == '$') {
> - /* expand $name from [environment] section,
> - * or from environment
> - */
> - char *env = getenv(cb->value+1);
> - if (env && *env)
> - return env;
> - section = "environment";
> - tag = cb->value + 1;
> - goto retry;
> + if (!cb->cache)
> + cb->cache = expand_variable(cb->value);
> + return cb->cache;
> }
> return cb->value;
> }
> - return 0;
> + return NULL;
> }
>
> /*
>

2020-07-15 18:05:46

by Steve Dickson

[permalink] [raw]
Subject: Re: [PATCH 0/4] nfs-utils: nfs.conf features to enable use of machine-id as nfs4_unique_id

Hello,

On 7/10/20 12:37 PM, Alice Mitchell wrote:
> This patch set introduces some additional features to the nfs.conf tool
> chain that allows automatic use of /etc/machine-id or other unique
> values for setups that otherwise do not have a unique hostname or disk
> image and would thus otherwise generate non-unique EXCHANGE_ID and
> SETCLIENTID messages.
>
> Signed-off-by: Alice Mitchell <[email protected]>
>
In the future, could you please use the '-s' git commit flag
which would add the Signed-off-by to every patch. thanks!

steved.

2020-07-16 15:53:32

by Patrick Goetz

[permalink] [raw]
Subject: Re: [PATCH 4/4] nfs-utils: Update nfs4_unique_id module parameter from the nfs.conf value

Speaking of which, it would be great if the distros (or whomever)
stopped setting up the unit files so that rpcbind is a required service.
This is a headache for me, as our security group flags machines running
rpcbind and it's entirely useless if you only use NFSv4.

In fact, isn't it about time to EOL NFSv3? <:)

On 7/15/20 12:44 PM, Steve Dickson wrote:
> Hello,
>
> On 7/10/20 12:44 PM, Alice Mitchell wrote:
>> systemd service to grab the config value and feed it to the kernel module
> Again, I'm wondering if the systemd/README should be updated to explain
> this new script...
>
> steved.
>
>> ---
>> nfs.conf | 1 +
>> systemd/Makefile.am | 3 +++
>> systemd/nfs-conf-export.sh | 28 ++++++++++++++++++++++++++++
>> systemd/nfs-config.service.in | 17 +++++++++++++++++
>> 4 files changed, 49 insertions(+)
>> create mode 100755 systemd/nfs-conf-export.sh
>> create mode 100644 systemd/nfs-config.service.in
>>
>> diff --git a/nfs.conf b/nfs.conf
>> index 186a5b19..8bb41227 100644
>> --- a/nfs.conf
>> +++ b/nfs.conf
>> @@ -4,6 +4,7 @@
>> #
>> [general]
>> # pipefs-directory=/var/lib/nfs/rpc_pipefs
>> +# nfs4_unique_id = ${machine-id}
>> #
>> [exports]
>> # rootdir=/export
>> diff --git a/systemd/Makefile.am b/systemd/Makefile.am
>> index 75cdd9f5..51acdc3f 100644
>> --- a/systemd/Makefile.am
>> +++ b/systemd/Makefile.am
>> @@ -9,6 +9,7 @@ unit_files = \
>> nfs-mountd.service \
>> nfs-server.service \
>> nfs-utils.service \
>> + nfs-config.service \
>> rpc-statd-notify.service \
>> rpc-statd.service \
>> \
>> @@ -69,4 +70,6 @@ genexec_PROGRAMS = nfs-server-generator rpc-pipefs-generator
>> install-data-hook: $(unit_files)
>> mkdir -p $(DESTDIR)/$(unitdir)
>> cp $(unit_files) $(DESTDIR)/$(unitdir)
>> + mkdir -p $(DESTDIR)/$(libexecdir)/nfs-utils
>> + install nfs-conf-export.sh $(DESTDIR)/$(libexecdir)/nfs-utils/
>> endif
>> diff --git a/systemd/nfs-conf-export.sh b/systemd/nfs-conf-export.sh
>> new file mode 100755
>> index 00000000..486e8df9
>> --- /dev/null
>> +++ b/systemd/nfs-conf-export.sh
>> @@ -0,0 +1,28 @@
>> +#!/bin/bash
>> +#
>> +# This script pulls values out of /etc/nfs.conf and configures
>> +# the appropriate kernel modules which cannot read it directly
>> +
>> +NFSMOD=/sys/module/nfs/parameters/nfs4_unique_id
>> +NFSPROBE=/etc/modprobe.d/nfs.conf
>> +
>> +# Now read the values from nfs.conf
>> +MACHINEID=`nfsconf --get general nfs4_unique_id`
>> +if [ $? -ne 0 ] || [ "$MACHINEID" == "" ]
>> +then
>> +# No config vaue found, assume blank
>> +MACHINEID=""
>> +fi
>> +
>> +# Kernel module is already loaded, update the live one
>> +if [ -e $NFSMOD ]; then
>> +echo -n "$MACHINEID" >> $NFSMOD
>> +fi
>> +
>> +# Rewrite the modprobe file for next reboot
>> +echo "# This file is overwritten by systemd nfs-config.service" > $NFSPROBE
>> +echo "# with values taken from /etc/nfs.conf" >> $NFSPROBE
>> +echo "# Do not hand modify" >> $NFSPROBE
>> +echo "options nfs nfs4_unique_id=\"$MACHINEID\"" >> $NFSPROBE
>> +
>> +echo "Set to: $MACHINEID"
>> diff --git a/systemd/nfs-config.service.in b/systemd/nfs-config.service.in
>> new file mode 100644
>> index 00000000..c5ef1024
>> --- /dev/null
>> +++ b/systemd/nfs-config.service.in
>> @@ -0,0 +1,17 @@
>> +[Unit]
>> +Description=Preprocess NFS configuration
>> +PartOf=nfs-client.target
>> +After=nfs-client.target
>> +DefaultDependencies=no
>> +
>> +[Service]
>> +Type=oneshot
>> +# This service needs to run any time any nfs service
>> +# is started, so changes to local config files get
>> +# incorporated. Having "RemainAfterExit=no" (the default)
>> +# ensures this happens.
>> +RemainAfterExit=no
>> +ExecStart=@_libexecdir@/nfs-utils/nfs-conf-export.sh
>> +
>> +[Install]
>> +WantedBy=nfs-client.target
>>
>

2020-07-17 13:41:20

by Steve Dickson

[permalink] [raw]
Subject: Re: [PATCH 4/4] nfs-utils: Update nfs4_unique_id module parameter from the nfs.conf value



On 7/16/20 11:52 AM, Patrick Goetz wrote:
> Speaking of which, it would be great if the distros (or whomever) stopped setting up the unit files so that rpcbind is a required service. This is a headache for me, as our security group flags machines running rpcbind and it's entirely useless if you only use NFSv4.
Why do you see rpcbind as such a security risk?

>
> In fact, isn't it about time to EOL NFSv3?  <:)
You are not the first to suggest this... No so much
of EOLing v3... more of a V4only client.

Personally I don't see EOL-ing v3 anytime soon.

steved.
>
> On 7/15/20 12:44 PM, Steve Dickson wrote:
>> Hello,
>>
>> On 7/10/20 12:44 PM, Alice Mitchell wrote:
>>> systemd service to grab the config value and feed it to the kernel module
>> Again, I'm wondering if the systemd/README should be updated to explain
>> this new script...
>>
>> steved.
>>
>>> ---
>>>   nfs.conf                      |  1 +
>>>   systemd/Makefile.am           |  3 +++
>>>   systemd/nfs-conf-export.sh    | 28 ++++++++++++++++++++++++++++
>>>   systemd/nfs-config.service.in | 17 +++++++++++++++++
>>>   4 files changed, 49 insertions(+)
>>>   create mode 100755 systemd/nfs-conf-export.sh
>>>   create mode 100644 systemd/nfs-config.service.in
>>>
>>> diff --git a/nfs.conf b/nfs.conf
>>> index 186a5b19..8bb41227 100644
>>> --- a/nfs.conf
>>> +++ b/nfs.conf
>>> @@ -4,6 +4,7 @@
>>>   #
>>>   [general]
>>>   # pipefs-directory=/var/lib/nfs/rpc_pipefs
>>> +# nfs4_unique_id = ${machine-id}
>>>   #
>>>   [exports]
>>>   # rootdir=/export
>>> diff --git a/systemd/Makefile.am b/systemd/Makefile.am
>>> index 75cdd9f5..51acdc3f 100644
>>> --- a/systemd/Makefile.am
>>> +++ b/systemd/Makefile.am
>>> @@ -9,6 +9,7 @@ unit_files =  \
>>>       nfs-mountd.service \
>>>       nfs-server.service \
>>>       nfs-utils.service \
>>> +    nfs-config.service \
>>>       rpc-statd-notify.service \
>>>       rpc-statd.service \
>>>       \
>>> @@ -69,4 +70,6 @@ genexec_PROGRAMS = nfs-server-generator rpc-pipefs-generator
>>>   install-data-hook: $(unit_files)
>>>       mkdir -p $(DESTDIR)/$(unitdir)
>>>       cp $(unit_files) $(DESTDIR)/$(unitdir)
>>> +    mkdir -p $(DESTDIR)/$(libexecdir)/nfs-utils
>>> +    install  nfs-conf-export.sh $(DESTDIR)/$(libexecdir)/nfs-utils/
>>>   endif
>>> diff --git a/systemd/nfs-conf-export.sh b/systemd/nfs-conf-export.sh
>>> new file mode 100755
>>> index 00000000..486e8df9
>>> --- /dev/null
>>> +++ b/systemd/nfs-conf-export.sh
>>> @@ -0,0 +1,28 @@
>>> +#!/bin/bash
>>> +#
>>> +# This script pulls values out of /etc/nfs.conf and configures
>>> +# the appropriate kernel modules which cannot read it directly
>>> +
>>> +NFSMOD=/sys/module/nfs/parameters/nfs4_unique_id
>>> +NFSPROBE=/etc/modprobe.d/nfs.conf
>>> +
>>> +# Now read the values from nfs.conf
>>> +MACHINEID=`nfsconf --get general nfs4_unique_id`
>>> +if [ $? -ne 0 ] || [ "$MACHINEID" == "" ]
>>> +then
>>> +# No config vaue found, assume blank
>>> +MACHINEID=""
>>> +fi
>>> +
>>> +# Kernel module is already loaded, update the live one
>>> +if [ -e $NFSMOD ]; then
>>> +echo -n "$MACHINEID" >> $NFSMOD
>>> +fi
>>> +
>>> +# Rewrite the modprobe file for next reboot
>>> +echo "# This file is overwritten by systemd nfs-config.service" > $NFSPROBE
>>> +echo "# with values taken from /etc/nfs.conf" >> $NFSPROBE
>>> +echo "# Do not hand modify" >> $NFSPROBE
>>> +echo "options nfs nfs4_unique_id=\"$MACHINEID\"" >> $NFSPROBE
>>> +
>>> +echo "Set to: $MACHINEID"
>>> diff --git a/systemd/nfs-config.service.in b/systemd/nfs-config.service.in
>>> new file mode 100644
>>> index 00000000..c5ef1024
>>> --- /dev/null
>>> +++ b/systemd/nfs-config.service.in
>>> @@ -0,0 +1,17 @@
>>> +[Unit]
>>> +Description=Preprocess NFS configuration
>>> +PartOf=nfs-client.target
>>> +After=nfs-client.target
>>> +DefaultDependencies=no
>>> +
>>> +[Service]
>>> +Type=oneshot
>>> +# This service needs to run any time any nfs service
>>> +# is started, so changes to local config files get
>>> +# incorporated.  Having "RemainAfterExit=no" (the default)
>>> +# ensures this happens.
>>> +RemainAfterExit=no
>>> +ExecStart=@_libexecdir@/nfs-utils/nfs-conf-export.sh
>>> +
>>> +[Install]
>>> +WantedBy=nfs-client.target
>>>
>>
>