ncm-metaconfig: ssh daemon configuration support #1377

gombasg · 2019-04-11T14:26:32Z

This allows deprecating ncm-ssh. I'd appreciate if someone cross-checked the schema with the sshd_options man page.

ned21 · 2019-07-10T17:05:53Z

jrha · 2019-07-11T08:54:36Z

Looks nice, ncm-ssh does run-time validation of the generated configuration, is this now considered unnecessary?

gombasg · 2019-07-12T07:12:52Z

@jrha: It would still be nice to have the run-time check, because the templates cannot verify if the generated configuration is compatible with the version of sshd which happens to be installed. I just don't know how that could be implemented with metaconfig. Maybe @stdweird has ideas :-)

stdweird · 2019-07-12T07:58:40Z

ncm-metaconfig/src/main/metaconfig/ssh/tests/profiles/server_allopts.pan

+
+prefix "/software/components/metaconfig/services/{/etc/ssh/sshd_config}/contents";
+
+"main/AcceptEnv" = list("LC_CTYPE", "LANG", "TERM");


use relative main prefix (or add main to the absolute prefix path)

nvm, this is a test

stdweird · 2019-07-12T07:59:09Z

ncm-metaconfig/src/main/metaconfig/ssh/tests/profiles/server_allopts.pan

@@ -0,0 +1,96 @@
+object template server_allopts;
+
+include 'metaconfig/ssh/server_config';


can you add a comment on what ssh release this is based? or which OS?

nvm, didn't see that this was a test.

stdweird · 2019-07-12T08:03:29Z

ncm-metaconfig/src/main/metaconfig/ssh/server_attrs.tt

+ 'RekeyLimit' ] -%]
+[% commalist = ['Ciphers', 'HostKeyAlgorithms', 'HostbasedAcceptedKeyTypes', 'KexAlgorithms', 'MACs', 'PubkeyAcceptedKeyTypes' ] -%]
+[% multilinelist = ['HostKey', 'ListenAddress', 'Port' ] -%]
+[% booleans = ['AllowAgentForwarding',


these are not needed if they are true pan booleans. you can test those explicitly in TT using CCM.is_boolean() (but not in a case block

it's value.is_boolean; see default case below

stdweird · 2019-07-12T08:08:14Z

ncm-metaconfig/src/main/metaconfig/ssh/server_attrs.tt

+SetEnv [% item.key %]="[% item.value %]"
+[% END -%]
+[% CASE -%]
+[% pair.key %] [% pair.value %]


or do the boolean check here, and maybe add a the most common type pf list and have that here as default list format (saves you another static list)

[% pair.key %] [% -%] [-% IF pair.value.is_boolean -%] [%- pair.value ? 'Yes' : 'No' -%] [%- ELSIF CCM.is_list(pair.value) -%] [%- pair.value.join(' ') -%] [%- ELSE -%] [%- pair.value -%] [-% END -%]

stdweird · 2019-07-12T08:08:59Z

ncm-metaconfig/src/main/metaconfig/ssh/server_attrs.tt

@@ -0,0 +1,44 @@
+[% spacelist = ['AcceptEnv', 'AllowGroups', 'AllowUsers', 'AuthenticationMethods', 'AuthorizedKeysFile', 'AuthorizedPrincipalsFile',


this seems the longest list; use this as the default list fallback

stdweird · 2019-07-12T08:34:02Z

@gombasg i have ideas 😄
i have an objection against implementing another "run arbitrary something as root" from the profiles, but hardcoding a whole bunch of test commands and being able to call them is fine for me.
and here it would be easy because the test does not require a file (or "the" final file).
anything that can be verified from stdin is easy to do.

verifying from file might be easy to do, if we patch/hack the atomic filewriter so we can test on the temp file. but it needs a patch to the atomicwriter.

jrha · 2019-07-15T10:02:09Z

@gombasg if it was desirable then the cleanest (IMHO) way to implement it would be to make the existing component use textrender with these TT files.

jrha · 2019-12-12T18:29:27Z

I'm bumping this - I want to look at migrating ncm-ssh to use these TT files with textrender so we can keep the validation logic.

gombasg · 2020-01-31T12:10:29Z

@jrha The question is, where to put the schema? I don't want to duplicate the definitions which are common between the client and the server.

gombasg · 2020-02-13T16:38:37Z

There is another issue with ncm-ssh. It is currently using CAF::FileEditor, and not CAF::FileWriter, which is a pain if one has to switch between different OpenSSH version supporting a different set of parameters, with the ability to back the change out if needed. However, making the switch in the ncm-ssh component would break deployments which do not define all SSH daemon options in profiles, but depend on system default remaining undisturbed. Using a different component means the template changes need updating anyway, so the missing options could be added.

Add support for prefixing various key types with "+" or "-" to indicate relative changes to the built-in defaults.

The main driver was getting support for "Match ..." blocks, which would have been more dificult to add to ncm-ssh.

Use value.is_boolean instead.

Re-work some regular expressions to make lines shorter. Add GSSAPIKexAlgorithms and a missing CASignatureAlgorithms setting.

ned21 · 2020-03-05T20:20:41Z

There is another issue with ncm-ssh. It is currently using CAF::FileEditor, and not CAF::FileWriter, which is a pain if one has to switch between different OpenSSH version supporting a different set of parameters, with the ability to back the change out if needed. However, making the switch in the ncm-ssh component would break deployments which do not define all SSH daemon options in profiles, but depend on system default remaining undisturbed. Using a different component means the template changes need updating anyway, so the missing options could be added.

How about adding a "full control" option to ncm-ssh? That would allow us to keep the config validation check prior to restarting sshd? Or have I not understood the constraint with Atomic FileWriter?

gombasg · 2020-03-06T16:24:41Z

A "full control" option need a new implementation of the Configure method. The schema used by ncm-ssh is incompatible with the metaconfig-based schema, so either the TT files will have to be re-written from scratch, or the entire schema need to be switched based on the value of "full control". So essentially, we'd end up with two independent, incompatible components disguised as one. I'm not sure keeping the validation would worth it. If validation is really needed, then we should convince @stdweird to add a generic validation step to metaconfig. Yes, that would in fact mean running arbitrary commands as root, because we do not always run the OS-supplied sshd, so the full path of the daemon would need to be specified in the templates, it cannot be hard-coded in the component.

stdweird · 2020-06-05T17:01:12Z

@gombasg @ned21 @jrha @jouvin lets settle this: feel free to vote and/or campaign for

a: run arbitrary commands via ncm-metaconfig
b: make static list of allowed commands (or list of regexes or somesuch) for things that make sense (like the ssh config self test or others)
c: do nothing, if you want to run arbitrary code, make a service/unit and call it as part of the daemons (should fix the dependecy order to run ncm-systemd before ncm-metaconfig)

whatever is decided, i can then also implement it to run validation

gombasg · 2020-06-07T21:33:34Z

Ask the question differently: do we want ncm-metaconfig to ever became a suitable replacement of ncm-filecopy or not? If you want to make ncm-metaconfig to replace ncm-filecopy, then you must allow running arbitrary commands, because not everything is a systemd service. E.g., if you want to disable the user list on the GDM login screen, then the command which needs to be run is “dconf update”. Now listing all such commands in a distinct dictionary, outside of the configuration file contents, would make sense, as it would make extracting/auditing all such potential commands easier. Individual config files would then just reference a key inside that dictionary instead of spelling out the whole command. So if one uses structure templates to define the contents of a config file, then such a structure template would still be unable to cause arbitrary commands to run – it could only trigger commands which are defined elsewhere. From: Stijn De Weirdt <notifications@github.com> Sent: 05 June 2020 19:02 To: quattor/configuration-modules-core <configuration-modules-core@noreply.github.com> Cc: Gombas, Gabor (Enterprise Infrastructure) <Gabor.Gombas@morganstanley.com>; Mention <mention@noreply.github.com> Subject: Re: [quattor/configuration-modules-core] ncm-metaconfig: ssh daemon configuration support (#1377) @gombasg<https://github.com/gombasg> @ned21<https://github.com/ned21> @jrha<https://github.com/jrha> @jouvin<https://github.com/jouvin> lets settle this: feel free to vote and/or campaign for a: run arbitrary commands via ncm-metaconfig b: make static list of allowed commands (or list of regexes or somesuch) for things that make sense (like the ssh config self test or others) c: do nothing, if you want to run arbitrary code, make a service/unit and call it as part of the daemons (should fix the dependecy order to run ncm-systemd before ncm-metaconfig) whatever is decided, i can then also implement it to run validation — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub<#1377 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/ABNYSMBPQ73NJZMJHJN43PLRVEQG3ANCNFSM4HFG7M3A>.

…

________________________________ NOTICE: Morgan Stanley is not acting as a municipal advisor and the opinions or views contained herein are not intended to be, and do not constitute, advice within the meaning of Section 975 of the Dodd-Frank Wall Street Reform and Consumer Protection Act. If you have received this communication in error, please destroy all electronic and paper copies and notify the sender immediately. Mistransmission is not intended to waive confidentiality or privilege. Morgan Stanley reserves the right, to the extent required and/or permitted under applicable law, to monitor electronic communications, including telephone calls with Morgan Stanley personnel. This message is subject to the Morgan Stanley General Disclaimers available at the following link: http://www.morganstanley.com/disclaimers. If you cannot access the links, please notify us by reply message and we will send the contents to you. By communicating with Morgan Stanley you acknowledge that you have read, understand and consent, (where applicable), to the foregoing and the Morgan Stanley General Disclaimers. You may have certain rights regarding the information that Morgan Stanley collects about you. Please see our Privacy Pledge https://www.morganstanley.com/privacy-pledge for more information about your rights.

stdweird · 2020-06-08T06:59:37Z

@gombasg thanks for voting! ;) i get that, i also struggle with that, as it's hard to try to limit the power of running arbitrary commands in a practical way.

i can add a pre, verifiy and post command option. verify will receive the file content on stdin (like ncm-ssh checks the validity).

in this case, i would then set a default value of the verify command to list("/usr/sbin/sshd", '-t', '-f', '/dev/stdin').
i would stick to the list format (no subshell), but not sure this is enough? should it be a list of lists to support running multiple commands? or just a string?

gombasg · 2020-06-08T08:27:52Z

I’d not complicate it too much. A string should be fine. I did not write any substantial Perl code in a long time, but AFAIK some of the usual helper modules is smart enough to avoid going through the shell if the command line does not contain any shell metacharacters. From: Stijn De Weirdt <notifications@github.com> Sent: 08 June 2020 09:00 To: quattor/configuration-modules-core <configuration-modules-core@noreply.github.com> Cc: Gombas, Gabor (Enterprise Infrastructure) <Gabor.Gombas@morganstanley.com>; Mention <mention@noreply.github.com> Subject: Re: [quattor/configuration-modules-core] ncm-metaconfig: ssh daemon configuration support (#1377) @gombasg<https://github.com/gombasg> thanks for voting! ;) i get that, i also struggle with that, as it's hard to try to limit the power of running arbitrary commands in a practical way. i can add a pre, verifiy and post command option. verify will receive the file content on stdin (like ncm-ssh checks the validity). in this case, i would then set a default value of the verify command to list("/usr/sbin/sshd", '-t', '-f', '/dev/stdin'). i would stick to the list format (no subshell), but not sure this is enough? should it be a list of lists to support running multiple commands? or just a string? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub<#1377 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/ABNYSMHWGHVCLYU4BGT5XETRVSD6PANCNFSM4HFG7M3A>.

…

________________________________ NOTICE: Morgan Stanley is not acting as a municipal advisor and the opinions or views contained herein are not intended to be, and do not constitute, advice within the meaning of Section 975 of the Dodd-Frank Wall Street Reform and Consumer Protection Act. If you have received this communication in error, please destroy all electronic and paper copies and notify the sender immediately. Mistransmission is not intended to waive confidentiality or privilege. Morgan Stanley reserves the right, to the extent required and/or permitted under applicable law, to monitor electronic communications, including telephone calls with Morgan Stanley personnel. This message is subject to the Morgan Stanley General Disclaimers available at the following link: http://www.morganstanley.com/disclaimers. If you cannot access the links, please notify us by reply message and we will send the contents to you. By communicating with Morgan Stanley you acknowledge that you have read, understand and consent, (where applicable), to the foregoing and the Morgan Stanley General Disclaimers. You may have certain rights regarding the information that Morgan Stanley collects about you. Please see our Privacy Pledge https://www.morganstanley.com/privacy-pledge for more information about your rights.

stdweird · 2020-06-10T15:29:07Z

ncm-metaconfig/src/main/metaconfig/ssh/pan/schema.pan

+};
+
+type sshd_config_match = {
+ "matches" : string[]


@gombasg why did you choose this so freeform? why not a list of dicts wit allowed keys and arbitrary values?

gombasg · 2020-06-10T16:05:24Z

Because implementing a “union” data type in Pan is more complex than I’d wanted to be bothered with 😊 If you want to model the real grammar, then somehow you’d need to allow “matches” to be either the “All” string literal, or a list of (choice, list) items. So ideally, something like “matches” : resource with ((is_string(SELF) && SELF == “All”) || is_list(SELF) && (… validate all items are 2-element lists…)) … would be nice, but the “resource” type does not allow assigning a literal. Btw. if the above “string or list” construct would be possible somehow, that would also help with the “cannot reset the value of ExecStart” issue with ncm-systemd 😊 From: Stijn De Weirdt <notifications@github.com> Sent: 10 June 2020 17:29 To: quattor/configuration-modules-core <configuration-modules-core@noreply.github.com> Cc: Gombas, Gabor (Enterprise Infrastructure) <Gabor.Gombas@morganstanley.com>; Mention <mention@noreply.github.com> Subject: Re: [quattor/configuration-modules-core] ncm-metaconfig: ssh daemon configuration support (#1377) @stdweird commented on this pull request.

________________________________ In ncm-metaconfig/src/main/metaconfig/ssh/pan/schema.pan<#1377 (comment)>:

+ 'RekeyLimit' ? string[] with length(SELF) == 1 || length(SELF) == 2

+ 'RSAAuthentication' ? boolean + 'RhostsRSAAuthentication' ? boolean + 'RevokedKeys' ? string + 'RDomain' ? string + 'SetEnv' ? string{} + 'StreamLocalBindMask' ? string with match (SELF, "^[0-7]{3,5}$") + 'StreamLocalBindUnlink' ? boolean + 'TrustedUserCAKeys' ? string + 'X11DisplayOffset' ? long(0..) + 'X11Forwarding' ? boolean + 'X11UseLocalHost' ? boolean +}; + +type sshd_config_match = { + "matches" : string[] @gombasg<https://github.com/gombasg> why did you choose this so freeform? why not a list of dicts wit allowed keys and arbitrary values? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub<#1377 (review)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/ABNYSMDTVTWAUNKZCFQGKY3RV6RFHANCNFSM4HFG7M3A>.

…

________________________________ NOTICE: Morgan Stanley is not acting as a municipal advisor and the opinions or views contained herein are not intended to be, and do not constitute, advice within the meaning of Section 975 of the Dodd-Frank Wall Street Reform and Consumer Protection Act. If you have received this communication in error, please destroy all electronic and paper copies and notify the sender immediately. Mistransmission is not intended to waive confidentiality or privilege. Morgan Stanley reserves the right, to the extent required and/or permitted under applicable law, to monitor electronic communications, including telephone calls with Morgan Stanley personnel. This message is subject to the Morgan Stanley General Disclaimers available at the following link: http://www.morganstanley.com/disclaimers. If you cannot access the links, please notify us by reply message and we will send the contents to you. By communicating with Morgan Stanley you acknowledge that you have read, understand and consent, (where applicable), to the foregoing and the Morgan Stanley General Disclaimers. You may have certain rights regarding the information that Morgan Stanley collects about you. Please see our Privacy Pledge https://www.morganstanley.com/privacy-pledge for more information about your rights.

stdweird · 2020-06-10T17:15:53Z

@gombasg i don't see why the matches itself need to be a list. all criteria have to match, so order seems irrelavnt. and if the 'All' key is used, we can handle in TT to only print 'All' without anything else.

wrt the mixed type type, if you really want that, you can

[stdweird@localhost tmp]$ cat t.pan 
object template t;

type x = element with is_string(SELF) || is_list(SELF);

bind "/a" = x{};

"/a/b" = "string";
"/a/c" = list(0,1);

[stdweird@localhost tmp]$ cat t.xml 
<?xml version="1.0" encoding="UTF-8"?><nlist format="pan" name="profile">
    <nlist name="a">
        <string name="b">string</string>
        <list name="c">
            <long>0</long>
            <long>1</long>
        </list>
    </nlist>
</nlist>

stdweird · 2020-06-10T19:42:12Z

@gombasg commands implemented in #1451
i'm going to take over this PR

stdweird · 2020-06-10T21:45:42Z

@gombasg i've opened #1452

ned21 · 2020-06-11T17:03:45Z

Superseded by #1452

gombasg requested a review from jrha April 11, 2019 14:26

jrha added this to the 19.6 milestone Apr 12, 2019

stdweird reviewed Jul 12, 2019

View reviewed changes

jrha modified the milestones: 19.12, 20.2 Dec 12, 2019

gombasg added 6 commits March 3, 2020 14:29

ncm-metaconfig/ssh: Minor reformatting

273d4ac

ncm-metaconfig/ssh: Add more client options

5303dda

ncm-metaconfig/ssh: Allow using key sets relative to the defaults

3114ea6

Add support for prefixing various key types with "+" or "-" to indicate relative changes to the built-in defaults.

ncm-metaconfig/ssh: Implement SSH daemon configuration

026aab3

The main driver was getting support for "Match ..." blocks, which would have been more dificult to add to ncm-ssh.

ncm-metaconfig: ssh: Drop boolean lists

29f80ab

Use value.is_boolean instead.

ncm-metaconfig: ssh: schema updates

7391554

Re-work some regular expressions to make lines shorter. Add GSSAPIKexAlgorithms and a missing CASignatureAlgorithms setting.

gombasg force-pushed the ssh_metaconfig branch from b792116 to 7391554 Compare March 5, 2020 13:47

stdweird reviewed Jun 10, 2020

View reviewed changes

This was referenced Jun 10, 2020

ncm-metaconfig: support running arbitrary commands in various steps #1451

Merged

ncm-metaconfig: ssh daemon configuration support #1452

Merged

ned21 closed this Jun 11, 2020

jrha removed this from the 20.12 milestone Oct 8, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ncm-metaconfig: ssh daemon configuration support #1377

ncm-metaconfig: ssh daemon configuration support #1377

gombasg commented Apr 11, 2019

ned21 commented Jul 10, 2019

jrha commented Jul 11, 2019

gombasg commented Jul 12, 2019

stdweird Jul 12, 2019

stdweird Jul 12, 2019

stdweird Jul 12, 2019

stdweird Jul 12, 2019

stdweird Jul 12, 2019

stdweird Jul 12, 2019

stdweird Jul 12, 2019

stdweird Jul 12, 2019

stdweird Jul 12, 2019

stdweird commented Jul 12, 2019

jrha commented Jul 15, 2019

jrha commented Dec 12, 2019

gombasg commented Jan 31, 2020

gombasg commented Feb 13, 2020 •

edited by ned21

Loading

ned21 commented Mar 5, 2020

gombasg commented Mar 6, 2020

stdweird commented Jun 5, 2020

gombasg commented Jun 7, 2020 via email

stdweird commented Jun 8, 2020

gombasg commented Jun 8, 2020 via email

stdweird Jun 10, 2020

gombasg commented Jun 10, 2020 via email

stdweird commented Jun 10, 2020

stdweird commented Jun 10, 2020

stdweird commented Jun 10, 2020

ned21 commented Jun 11, 2020


		prefix "/software/components/metaconfig/services/{/etc/ssh/sshd_config}/contents";

		"main/AcceptEnv" = list("LC_CTYPE", "LANG", "TERM");

		@@ -0,0 +1,96 @@
		object template server_allopts;

		include 'metaconfig/ssh/server_config';

		@@ -0,0 +1,44 @@
		[% spacelist = ['AcceptEnv', 'AllowGroups', 'AllowUsers', 'AuthenticationMethods', 'AuthorizedKeysFile', 'AuthorizedPrincipalsFile',

ncm-metaconfig: ssh daemon configuration support #1377

ncm-metaconfig: ssh daemon configuration support #1377

Conversation

gombasg commented Apr 11, 2019

ned21 commented Jul 10, 2019

jrha commented Jul 11, 2019

gombasg commented Jul 12, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stdweird commented Jul 12, 2019

jrha commented Jul 15, 2019

jrha commented Dec 12, 2019

gombasg commented Jan 31, 2020

gombasg commented Feb 13, 2020 • edited by ned21 Loading

ned21 commented Mar 5, 2020

gombasg commented Mar 6, 2020

stdweird commented Jun 5, 2020

gombasg commented Jun 7, 2020 via email

stdweird commented Jun 8, 2020

gombasg commented Jun 8, 2020 via email

Choose a reason for hiding this comment

gombasg commented Jun 10, 2020 via email

stdweird commented Jun 10, 2020

stdweird commented Jun 10, 2020

stdweird commented Jun 10, 2020

ned21 commented Jun 11, 2020

gombasg commented Feb 13, 2020 •

edited by ned21

Loading