Mailing List Archive: [PATCH] introduce vendordir for easier config file update

[PATCH] introduce vendordir for easier config file update

Jan 29, 2021, 6:18 AM

Post #1 of 6 (609 views)

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)

Re: [PATCH] introduce vendordir for easier config file update [ In reply to ]

kukuk at suse

Feb 3, 2021, 4:54 AM

Post #2 of 6 (606 views)

Permalink

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)

Re: [PATCH] introduce vendordir for easier config file update [ In reply to ]

philipp at marek

Feb 3, 2021, 6:54 AM

Post #3 of 6 (606 views)

Permalink

>> 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.
> does nobody have an opinion about this?

Well, with your solution: if the vendor file gets some new security
settings,
the admin file won't get them, and so the total security might go down.
(Example: "Protocol 2")

I'm left with the conclusion that a REAL solution to all the problems
here
means to have a turing-complete config language - or to have very few
shared settings and to split on the remote host or local user with
an "Include" statement using %u, %i, and similar.

_______________________________________________
openssh-unix-dev mailing list
openssh-unix-dev@mindrot.org
https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev

Re: [PATCH] introduce vendordir for easier config file update [ In reply to ]

kukuk at suse

Feb 3, 2021, 6:59 AM

Post #4 of 6 (606 views)

Permalink

On Wed, Feb 03, Philipp Marek wrote:

> >> 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.
> > does nobody have an opinion about this?
>
> Well, with your solution: if the vendor file gets some new security
> settings,
> the admin file won't get them, and so the total security might go down.
> (Example: "Protocol 2")

If the admin creates an own copy, he has to maintain it like he has
today. If the admin makes changes today, he also don't get the new
security settings.

So in worst case, the situation is as of today, you are right. But not
in general.

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)
_______________________________________________
openssh-unix-dev mailing list
openssh-unix-dev@mindrot.org
https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev

Re: [PATCH] introduce vendordir for easier config file update [ In reply to ]

jjelen at redhat

Feb 3, 2021, 9:09 AM

Post #5 of 6 (606 views)

Permalink

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.

_______________________________________________
openssh-unix-dev mailing list
openssh-unix-dev@mindrot.org
https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev

Re: [PATCH] introduce vendordir for easier config file update [ In reply to ]

dtucker at dtucker

Feb 4, 2021, 6:38 PM

Post #6 of 6 (606 views)

Permalink

On Thu, 4 Feb 2021 at 04:12, Jakub Jelen <jjelen@redhat.com> wrote:
[...]
> 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.

I agree it makes reasoning about how the config file parsing works,
and it's already not easy.

> The discussion about SendEnv is was here recently in the following bug:

Going back to SendEnv specifically, another way to resolve the example
cited here would be to add another token similar to "none" that
prevents any additional items being added to the list (for want of a
better term, let's call it "no-more" for now). That would allow you
to do what you want within the existing Include directive, ie

/etc/ssh/sshd_config:
Include /etc/ssh/sshd_config.d/*
SendEnv foo

/etc/ssh/sshd_config.d/user_config:
SendEnv none
SendEnv no-more

("final" would be nice given we already use it in ssh_config, but we
probably would want the token to not be a possibly valid environment
variable name, or at least one unlikely to be settable from a shell.
Maybe "$final"? Better suggestions welcome.)

--
Darren Tucker (dtucker at dtucker.net)
GPG key 11EAA6FA / A86E 3E07 5B19 5880 E860 37F4 9357 ECEF 11EA A6FA (new)
Good judgement comes with experience. Unfortunately, the experience
usually comes from bad judgement.
_______________________________________________
openssh-unix-dev mailing list
openssh-unix-dev@mindrot.org
https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev

Mailing List Archive

Attached Files:

Attached Files: