Thorsten Kukuk
2021-Jan-29 14:18 UTC
[PATCH] introduce vendordir for easier config file update
Hi, Distributors have one common problem: configuration files and updates. If a configuration file is modified by an user and the distributor mades changes to it, the package manager needs to decide which version of the configuration file should be used: the one of the admin or the one from the distributor. Independent of the decission, in worst case the service is broken until the admin merges the changes manually. Which is not that problem with a single system, but could be a lot of work for big clusters. There is now the include statement, which solves already many cases as the admin could put his changes in an extra file, but there are still some bigger issues. As an example for sshd_config: most Linux distributions added meanwhile an include statement to read at first files from /etc/ssh/sshd_config.d/* This works fine for directives like 'PermitRootLogin', where the first entry found wins. But you can have multiple AcceptEnv directives. And there is no way for an admin to change the distributor default without editing the config file itself, which again leads to update problems in the future. With ssh_config it's even more complicated: You can have multiple SendEnv directives, and you can change them later. This leads now to the situation, that you need two include directives: one on the beginning of the configuration file, which sets variables which could only be set once, and at the end, to remove and modify SendEnv. I don't know currently if there are more directives you cannot modify, so that the admin still has to modify the original file. I made a relativ small patch, which tries to follow the "systemd" behavior, which is meanwhile used by many more projects: - There is a distributor/vendor default configuration file in /usr/share/ssh - The admin can create his own configuration file in /etc/ssh - There is still the possibility to use the include statement to only override single directives. So if there is no admin provided configuration file, the vendor file from /usr/share/ssh is used. If there is an admin provided configuration file in /etc/ssh, this one will be used by default. Includes are only used from the configuration file which is really read. And if a distribution does not like this, it can still only ship the configuration files in /etc/ssh and there is no change in behavior. Attached is a patch which I'm using currently. I would like to see if upstream openssh would support this. Thorsten -- Thorsten Kukuk, Distinguished Engineer, Senior Architect SLES & MicroOS SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nuernberg, Germany Managing Director: Felix Imendoerffer (HRB 36809, AG N?rnberg) -------------- next part -------------- A non-text attachment was scrubbed... Name: openssh-8.4p1-vendordir.patch Type: text/x-patch Size: 5970 bytes Desc: not available URL: <http://lists.mindrot.org/pipermail/openssh-unix-dev/attachments/20210129/02cf31bf/attachment.bin>
Thorsten Kukuk
2021-Feb-03 12:54 UTC
[PATCH] introduce vendordir for easier config file update
Hi, does nobody have an opinion about this? Thanks, Thorsten On Fri, Jan 29, Thorsten Kukuk wrote:> > Hi, > > Distributors have one common problem: configuration files and updates. > > If a configuration file is modified by an user and the distributor mades > changes to it, the package manager needs to decide which version of the > configuration file should be used: the one of the admin or the one from > the distributor. Independent of the decission, in worst case the service > is broken until the admin merges the changes manually. Which is not that > problem with a single system, but could be a lot of work for big clusters. > > There is now the include statement, which solves already many cases as > the admin could put his changes in an extra file, but there are still > some bigger issues. > > As an example for sshd_config: most Linux distributions added meanwhile an > include statement to read at first files from /etc/ssh/sshd_config.d/* > This works fine for directives like 'PermitRootLogin', where the first entry > found wins. But you can have multiple AcceptEnv directives. And there is no > way for an admin to change the distributor default without editing the > config file itself, which again leads to update problems in the future. > > With ssh_config it's even more complicated: You can have multiple SendEnv > directives, and you can change them later. This leads now to the situation, > that you need two include directives: one on the beginning of the > configuration file, which sets variables which could only be set once, > and at the end, to remove and modify SendEnv. I don't know currently if > there are more directives you cannot modify, so that the admin still has > to modify the original file. > > I made a relativ small patch, which tries to follow the "systemd" behavior, > which is meanwhile used by many more projects: > > - There is a distributor/vendor default configuration file in /usr/share/ssh > - The admin can create his own configuration file in /etc/ssh > - There is still the possibility to use the include statement to only override > single directives. > > So if there is no admin provided configuration file, the vendor file from > /usr/share/ssh is used. If there is an admin provided configuration file > in /etc/ssh, this one will be used by default. > > Includes are only used from the configuration file which is really read. > And if a distribution does not like this, it can still only ship the > configuration files in /etc/ssh and there is no change in behavior. > > Attached is a patch which I'm using currently. I would like to see if > upstream openssh would support this. > > Thorsten > > -- > Thorsten Kukuk, Distinguished Engineer, Senior Architect SLES & MicroOS > SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nuernberg, Germany > Managing Director: Felix Imendoerffer (HRB 36809, AG N?rnberg)> diff -urN openssh-8.4p1/dh.c openssh-8.4p1-vendor/dh.c > --- openssh-8.4p1/dh.c 2020-09-27 09:25:01.000000000 +0200 > +++ openssh-8.4p1-vendor/dh.c 2021-01-29 11:49:40.968418136 +0100 > @@ -151,10 +151,18 @@ > size_t linesize = 0; > int best, bestcount, which, linenum; > struct dhgroup dhg; > + char *dh_moduli_path; > + struct stat st; > > - if ((f = fopen(_PATH_DH_MODULI, "r")) == NULL) { > + if (stat(_PATH_VENDOR_DH_MODULI, &st) == 0 && > + stat(_PATH_DH_MODULI, &st) == -1) { > + dh_moduli_path = _PATH_VENDOR_DH_MODULI; > + } else { > + dh_moduli_path = _PATH_DH_MODULI; > + } > + if ((f = fopen(dh_moduli_path, "r")) == NULL) { > logit("WARNING: could not open %s (%s), using fixed modulus", > - _PATH_DH_MODULI, strerror(errno)); > + dh_moduli_path, strerror(errno)); > return (dh_new_group_fallback(max)); > } > > @@ -185,7 +193,7 @@ > > if (bestcount == 0) { > fclose(f); > - logit("WARNING: no suitable primes in %s", _PATH_DH_MODULI); > + logit("WARNING: no suitable primes in %s", dh_moduli_path); > return (dh_new_group_fallback(max)); > } > which = arc4random_uniform(bestcount); > @@ -210,7 +218,7 @@ > fclose(f); > if (bestcount != which + 1) { > logit("WARNING: selected prime disappeared in %s, giving up", > - _PATH_DH_MODULI); > + dh_moduli_path); > return (dh_new_group_fallback(max)); > } > > diff -urN openssh-8.4p1/pathnames.h openssh-8.4p1-vendor/pathnames.h > --- openssh-8.4p1/pathnames.h 2020-09-27 09:25:01.000000000 +0200 > +++ openssh-8.4p1-vendor/pathnames.h 2021-01-29 11:35:41.655599046 +0100 > @@ -18,6 +18,8 @@ > #define SSHDIR ETCDIR "/ssh" > #endif > > +#define VENDORDIR "/usr/share/ssh" > + > #ifndef _PATH_SSH_PIDDIR > #define _PATH_SSH_PIDDIR "/var/run" > #endif > @@ -35,13 +37,16 @@ > * should be world-readable. > */ > #define _PATH_SERVER_CONFIG_FILE SSHDIR "/sshd_config" > +#define _PATH_SERVER_VENDOR_CONFIG_FILE VENDORDIR "/sshd_config" > #define _PATH_HOST_CONFIG_FILE SSHDIR "/ssh_config" > +#define _PATH_HOST_VENDOR_CONFIG_FILE VENDORDIR "/ssh_config" > #define _PATH_HOST_DSA_KEY_FILE SSHDIR "/ssh_host_dsa_key" > #define _PATH_HOST_ECDSA_KEY_FILE SSHDIR "/ssh_host_ecdsa_key" > #define _PATH_HOST_ED25519_KEY_FILE SSHDIR "/ssh_host_ed25519_key" > #define _PATH_HOST_XMSS_KEY_FILE SSHDIR "/ssh_host_xmss_key" > #define _PATH_HOST_RSA_KEY_FILE SSHDIR "/ssh_host_rsa_key" > #define _PATH_DH_MODULI SSHDIR "/moduli" > +#define _PATH_VENDOR_DH_MODULI VENDORDIR "/moduli" > > #ifndef _PATH_SSH_PROGRAM > #define _PATH_SSH_PROGRAM "/usr/bin/ssh" > diff -urN openssh-8.4p1/ssh.c openssh-8.4p1-vendor/ssh.c > --- openssh-8.4p1/ssh.c 2020-09-27 09:25:01.000000000 +0200 > +++ openssh-8.4p1-vendor/ssh.c 2021-01-27 18:22:52.322271681 +0100 > @@ -593,6 +593,7 @@ > process_config_files(const char *host_name, struct passwd *pw, int final_pass, > int *want_final_pass) > { > + struct stat st; > char buf[PATH_MAX]; > int r; > > @@ -611,10 +612,23 @@ > &options, SSHCONF_CHECKPERM | SSHCONF_USERCONF | > (final_pass ? SSHCONF_FINAL : 0), want_final_pass); > > - /* Read systemwide configuration file after user config. */ > - (void)read_config_file(_PATH_HOST_CONFIG_FILE, pw, > - host, host_name, &options, > - final_pass ? SSHCONF_FINAL : 0, want_final_pass); > + /* If only the vendor configuration file exists, use that. > + * Else use the standard configuration file. > + */ > + if (stat(_PATH_HOST_VENDOR_CONFIG_FILE, &st) == 0 && > + stat(_PATH_HOST_CONFIG_FILE, &st) == -1) { > + /* Read vendor distributed configuration file. */ > + (void)read_config_file(_PATH_HOST_VENDOR_CONFIG_FILE, > + pw, host, host_name, &options, > + final_pass ? SSHCONF_FINAL : 0, > + want_final_pass); > + } else { > + /* Read systemwide configuration file after user config. */ > + (void)read_config_file(_PATH_HOST_CONFIG_FILE, pw, > + host, host_name, &options, > + final_pass ? SSHCONF_FINAL : 0, > + want_final_pass); > + } > } > } > > diff -urN openssh-8.4p1/sshd.c openssh-8.4p1-vendor/sshd.c > --- openssh-8.4p1/sshd.c 2020-09-27 09:25:01.000000000 +0200 > +++ openssh-8.4p1-vendor/sshd.c 2021-01-27 18:25:38.370273280 +0100 > @@ -136,7 +136,7 @@ > ServerOptions options; > > /* Name of the server configuration file. */ > -char *config_file_name = _PATH_SERVER_CONFIG_FILE; > +char *config_file_name = NULL; > > /* > * Debug mode flag. This can be set on the command line. If debug > @@ -1526,6 +1526,7 @@ > int > main(int ac, char **av) > { > + struct stat st; > struct ssh *ssh = NULL; > extern char *optarg; > extern int optind; > @@ -1737,7 +1738,21 @@ > */ > (void)atomicio(vwrite, startup_pipe, "\0", 1); > } > + } else if (config_file_name == NULL) { > + /* If only the vendor configuration file exists, use that. > + * Else use the standard configuration file. > + */ > + if (stat(_PATH_SERVER_VENDOR_CONFIG_FILE, &st) == 0 && > + stat(_PATH_SERVER_CONFIG_FILE, &st) == -1) { > + /* fill with global distributor settings */ > + config_file_name = _PATH_SERVER_VENDOR_CONFIG_FILE; > + } else { > + /* load global admin settings */ > + config_file_name = _PATH_SERVER_CONFIG_FILE; > + } > + load_server_config(config_file_name, cfg); > } else if (strcasecmp(config_file_name, "none") != 0) > + /* load config specified on commandline */ > load_server_config(config_file_name, cfg); > > parse_server_config(&options, rexeced_flag ? "rexec" : config_file_name, > diff -urN openssh-8.4p1/ssh-keysign.c openssh-8.4p1-vendor/ssh-keysign.c > --- openssh-8.4p1/ssh-keysign.c 2020-09-27 09:25:01.000000000 +0200 > +++ openssh-8.4p1-vendor/ssh-keysign.c 2021-01-15 15:03:13.258901048 +0100 > @@ -207,6 +207,8 @@ > initialize_options(&options); > (void)read_config_file(_PATH_HOST_CONFIG_FILE, pw, "", "", > &options, 0, NULL); > + (void)read_config_file(_PATH_HOST_VENDOR_CONFIG_FILE, pw, "", "", > + &options, 0, NULL); > fill_default_options(&options); > if (options.enable_ssh_keysign != 1) > fatal("ssh-keysign not enabled in %s",-- Thorsten Kukuk, Distinguished Engineer, Senior Architect SLES & MicroOS SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nuernberg, Germany Managing Director: Felix Imendoerffer (HRB 36809, AG N?rnberg) -------------- next part -------------- A non-text attachment was scrubbed... Name: openssh-8.4p1-vendordir.patch Type: text/x-patch Size: 5970 bytes Desc: not available URL: <http://lists.mindrot.org/pipermail/openssh-unix-dev/attachments/20210203/a3485018/attachment-0001.bin>
Jakub Jelen
2021-Feb-03 17:09 UTC
[PATCH] introduce vendordir for easier config file update
On 1/29/21 3:18 PM, Thorsten Kukuk wrote:> > Hi, > > Distributors have one common problem: configuration files and updates. > > If a configuration file is modified by an user and the distributor mades > changes to it, the package manager needs to decide which version of the > configuration file should be used: the one of the admin or the one from > the distributor. Independent of the decission, in worst case the service > is broken until the admin merges the changes manually. Which is not that > problem with a single system, but could be a lot of work for big clusters. > > There is now the include statement, which solves already many cases as > the admin could put his changes in an extra file, but there are still > some bigger issues. > > As an example for sshd_config: most Linux distributions added meanwhile an > include statement to read at first files from /etc/ssh/sshd_config.d/* > This works fine for directives like 'PermitRootLogin', where the first entry > found wins. But you can have multiple AcceptEnv directives. And there is no > way for an admin to change the distributor default without editing the > config file itself, which again leads to update problems in the future. > > With ssh_config it's even more complicated: You can have multiple SendEnv > directives, and you can change them later. This leads now to the situation, > that you need two include directives: one on the beginning of the > configuration file, which sets variables which could only be set once, > and at the end, to remove and modify SendEnv. I don't know currently if > there are more directives you cannot modify, so that the admin still has > to modify the original file. > > I made a relativ small patch, which tries to follow the "systemd" behavior, > which is meanwhile used by many more projects: > > - There is a distributor/vendor default configuration file in /usr/share/ssh > - The admin can create his own configuration file in /etc/ssh > - There is still the possibility to use the include statement to only override > single directives. > > So if there is no admin provided configuration file, the vendor file from > /usr/share/ssh is used. If there is an admin provided configuration file > in /etc/ssh, this one will be used by default. > > Includes are only used from the configuration file which is really read. > And if a distribution does not like this, it can still only ship the > configuration files in /etc/ssh and there is no change in behavior. > > Attached is a patch which I'm using currently. I would like to see if > upstream openssh would support this.We use the simple Include directory as described above, where we ship our defaults. It is up to the admin to decide if they need to override some values to including their configuration file before our file (in lexicographical order) without breaking updates. Even though your change looks like fitting better Linux FHS, it introduces a new complexity and quite huge change after 20+ years of history where the ssh configuration works as it works. The discussion about SendEnv is was here recently in the following bug: https://bugzilla.mindrot.org/show_bug.cgi?id=3247#c2 Regards, -- Jakub Jelen Senior Software Engineer Crypto Team, Security Engineering Red Hat, Inc.