nixos/ssh: use upstream's default cryptographic algorithms #316934

tomfitzhenry · 2024-06-03T14:34:31Z

Motivation

Hardening SSH algorithms, which typically means dropping all-but-the-strongest is of questionable value, given SSH's downgrade protection[0]. We pay in compatibility, and maintenance.

Further, as noted in
https://github.com/NixOS/nixpkgs/pull/172393/files#r871727289 , both the guidelines that we follow have not been updated in years.

The costs of having/maintaining these defaults:

The burden of having a larger module that deviates from upstream. We've slowly been reducing the upstream diff, to reduce maintenance burden.
Difficult for users to opt-out of these defaults. For example, when using a "no OpenSSL" build of OpenSSH, having these defaults means having to manually overriding NixOS's defaults. Upstream's defaults, meanwhile, gracefully only use available algorithms, if OpenSSL is not linked.
- For users seeking to reduce attack surfaces that are fortunate enough to only have modern clients, they could choose to use pkgs.opensshPackages.openssh.override { linkOpenssl = false; }, which only supports chacha20-poly1305 and curve25519-sha256. This has the added benefit of removing openssl from the memory of sshd.
nixos/sshd: Remove algorithms that do MAC-then-encrypt #231165 unexpectedly broke many clients.
The time in discussing/reviewing these defaults.
Anecdotally, a friend trying NixOS for the first time with a ssh_config supporting only ecdh-* key exchanges was unable to SSH in after install.

There's a certain level of enjoyment that comes from researching and selecting a favourite suite of ciphers, but as a distro, it's not our core competancy, and best left for upstream who are active in advances/attacks/compatibility.

https://eprint.iacr.org/2016/072.pdf

Description of changes

Things done

Add a 👍 reaction to pull requests you find important.

tomfitzhenry · 2024-06-03T14:42:28Z

Seeking wide review on this. A note in the manual will need to be added too.

aszlig · 2024-06-03T14:50:11Z

I agree that we shouldn't deviate too much from upstream, but we could alternatively move those defaults to something like profiles/hardened.nix or even a dedicated module for people to opt-in.

SuperSandro2000

We should document in the options that null means upstreams defaults. Also we should add a release note entry.

Did you do a comparison which algorithms are now enabled again? Is there maybe some upstream guidance we could use instead like those algorithms are soon to be disabled by default and if you can, you should already disable them?

SuperSandro2000

I think the last point from the review thread is, to move the old defaults to the hardened profile.

nixos/doc/manual/release-notes/rl-2411.section.md

nixos/modules/services/networking/ssh/sshd.nix

SuperSandro2000 · 2024-06-05T10:55:51Z

nixos/modules/services/networking/ssh/sshd.nix

-                "[email protected]"
-                "curve25519-sha256"
-                "[email protected]"
-                "diffie-hellman-group-exchange-sha256"


The new ones are

[email protected], curve25519-sha256,[email protected], ecdh-sha2-nistp256,ecdh-sha2-nistp384,ecdh-sha2-nistp521, diffie-hellman-group-exchange-sha256, diffie-hellman-group16-sha512,diffie-hellman-group18-sha512, diffie-hellman-group14-sha256

https://github.com/openssh/openssh-portable/blob/V_9_7_P1/sshd_config.5#L1053C1-L1058C30

SuperSandro2000 · 2024-06-05T10:56:16Z

nixos/modules/services/networking/ssh/sshd.nix

-                "[email protected]"
-                "[email protected]"
-                "[email protected]"


[email protected],[email protected], [email protected],[email protected], [email protected], [email protected],[email protected], hmac-sha2-256,hmac-sha2-512,hmac-sha1

https://github.com/openssh/openssh-portable/blob/V_9_7_P1/sshd_config.5#L1206-L1210

SuperSandro2000 · 2024-06-05T10:56:38Z

nixos/modules/services/networking/ssh/sshd.nix

-                "[email protected]"
-                "[email protected]"
-                "[email protected]"
-                "aes256-ctr"
-                "aes192-ctr"
-                "aes128-ctr"


https://github.com/openssh/openssh-portable/blob/V_9_7_P1/sshd_config.5#L578-L580

[email protected], aes128-ctr,aes192-ctr,aes256-ctr, [email protected],[email protected]

SuperSandro2000

To test this out I just picked this PR into my fork and wanted to deploy it but it failed because I use openssh in the initrd

I think we need to add some conditions to that config file

https://github.com/NixOS/nixpkgs/blob/master/nixos/modules/system/boot/initrd-ssh.nix#L153-L155

tomfitzhenry · 2024-06-05T12:01:27Z

I agree that we shouldn't deviate too much from upstream, but we could alternatively move those defaults to something like profiles/hardened.nix or even a dedicated module for people to opt-in.

In this PR I've moved NixOS's current defaults to the hardened profile, but I'm not 100% on whether it's the best place, on the basis that:

maintaining hardened.nix has maintenance costs (so all we've really done is distributed those costs, not reduced)
keeping the overridden defaults in hardened.nix does still offer improved ergonomics for those that want to opt-out of those overrides, but not for hardened.nix users (who have to do the un-ergonomic "Ciphers = lib.mkForce [ ... ]".

Alternatives are to have an option enableMozillaRecommendations (or whoever we're trusting, and I'd like us to select one guide, if any).

SuperSandro2000 · 2024-06-05T12:01:30Z

I fixed evaluation with SuperSandro2000@02c08fc but I didn't test it yet e2e

tomfitzhenry · 2024-06-05T12:02:10Z

To test this out I just picked this PR into my fork and wanted to deploy it but it failed because I use openssh in the initrd

I think we need to add some conditions to that config file

https://github.com/NixOS/nixpkgs/blob/master/nixos/modules/system/boot/initrd-ssh.nix#L153-L155

Great find, that's worrying!

SuperSandro2000 · 2024-06-05T12:02:34Z

keeping the overridden defaults in hardened.nix does still offer improved ergonomics for those that want to opt-out of those overrides, but not for hardened.nix users (who have to do the un-ergonomic "Ciphers = lib.mkForce [ ... ]".

We could fix that by using lib.mkDefault

I don#t think we need an extra option for that

@aszlig opinions?

aszlig · 2024-06-05T12:26:11Z

Alternatives are to have an option enableMozillaRecommendations (or whoever we're trusting, and I'd like us to select one guide, if any).

That would be similar to eg. services.nginx.recommendedTlsSettings, which might probably be even preferable to the hardened.nix module, since if we want this in the hardened module it could still be included there via services.openssh.enableMozillaRecommendations (if the maintainers (ping @joachifm and @emilazy) of hardened.nix feel inclined to do).

We could fix that by using lib.mkDefault

With the enableMozillaRecommendations option it would also address that issue since overriding is a single mkForce away.

tomfitzhenry · 2024-06-05T12:29:43Z

We should document in the options that null means upstreams defaults. Also we should add a release note entry.

Done.

Did you do a comparison which algorithms are now enabled again?

I see you've commented with the new selections, thanks for that!

Is there maybe some upstream guidance we could use instead like those algorithms are soon to be disabled by default and if you can, you should already disable them?

https://www.openssh.com/releasenotes.html includes "Future deprecation notices". The only current such notice is the future removal of DSA support, which we recently expedited: #300716 👍

tomfitzhenry · 2024-06-05T13:06:08Z

With the enableMozillaRecommendations option it would also address that issue since overriding is a single mkForce away.

I've updated the PR to introduce this option.

tomfitzhenry · 2024-06-05T13:10:16Z

nixos/modules/services/networking/ssh/sshd.nix

+      enableMozillaRecommendations = mkEnableOption "" // {
+        descrption = ''
+          Whether to enable Mozilla's recommended cryptographic algorithms, per
+          https://infosec.mozilla.org/guidelines/openssh#modern-openssh-67.


Note that enableMozillaRecommendations uses the exact algorithms as listed on its page, which is slightly different to the set of algorithms that we previously had.

https://github.com/mozilla/infosec.mozilla.org/blob/master/docs/guidelines/openssh.md is the right place to engage/query with Mozilla's recommendations.

mweinelt · 2024-06-05T13:10:40Z

Mozilla has fired their security team who was in charge of config recommendations back in 2020.

tomfitzhenry · 2024-06-05T13:13:32Z

Mozilla has fired their security team who was in charge of config recommendations back in 2020.

That's sad to hear. :/

I'm happy with upstream's recommendations, but it sounds like some would prefer the recommendations of others (which is fine/great, and a strength of FOSS; different people have different views). To those people, is there any published guide that you prefer? Unless we can point to one, I'd be inclined to leave that option out for now, and it can be added if/when a suitable alternative is found.

If it's not published/maintained, then it might be better placed in one's own config.

For those seeking an sshd with reduced attack surface, and who have the luxury of only supporting modern clients (as I do, in some environments), consider services.openssh.package = pkgs.opensshPackages.openssh.override { linkOpenssl = false; }. This sshd has no OpenSSL in its memory, and only (?) supports AES, chacha20-poly1305, curve25519-sha256 and chacha20-poly1305 and curve25519-sha256.

tomfitzhenry · 2024-06-05T13:57:42Z

Mozilla has fired their security team who was in charge of config recommendations back in 2020.

I've now removed the enableMozillaRecommendation option. If this guide isn't kept uptodate, it shouldn't be offered.

Hardening SSH algorithms, which typically means dropping all-but-the-strongest is of questionable value, given SSH's downgrade protection[0]. We pay in compatibility, and maintenance. Further, as noted in https://github.com/NixOS/nixpkgs/pull/172393/files#r871727289 , both the guidelines that we follow have not been updated in years. The costs of having/maintaining these defaults: * The burden of having a larger module that deviates from upstream. We've slowly been reducing the upstream diff, to reduce maintenance burden. * Difficult for users to opt-out of these defaults. For example, when using a "no OpenSSL" build of OpenSSH, having these defaults means having to manually overriding NixOS's defaults. Upstream's defaults, meanwhile, gracefully only use available algorithms, if OpenSSL is not linked. * For users seeking to reduce attack surfaces that are fortunate enough to only have modern clients, they could choose to use `pkgs.opensshPackages.openssh.override { linkOpenssl = false; }`, which only supports chacha20-poly1305 and curve25519-sha256. * NixOS#231165 unexpectedly broke some clients. * The time in discussing/reviewing these defaults. * Anecdotally, a friend trying NixOS for the first time with a ssh_config supporting only ecdh-* key exchanges was unable to SSH in after install. There's a certain level of enjoyment that comes from researching and selecting a favourite suite of ciphers, but as a distro, it's not our core competancy, and best left for upstream who are active in advances/attacks/compatibility. 0. https://eprint.iacr.org/2016/072.pdf

tomfitzhenry · 2024-06-05T14:07:50Z

I fixed evaluation with SuperSandro2000@02c08fc but I didn't test it yet e2e

I reproduced the issue with nixosTests.initrd-network-ssh, and confirmed the fix fixes it. Thanks!

@ofborg test initrd-network-ssh

tomfitzhenry · 2024-06-05T14:10:17Z

@ofborg test openssh

tomfitzhenry · 2024-06-05T14:12:02Z

I'm proposing this PR as it stands (and happy to hear more comments).

If/when people interested in other advice can find a published guide that they want to follow, that can be added as an option (named according to who is giving the advice). hardened.nix maintainers can choose whether to follow that guide too.

LeSuisse · 2024-06-05T18:57:54Z

Hello,

The given arguments seem fair to me however I'm a bit worried of re-introducing weak cryptography primitives by default. This seems like a regression and I would not be surprised if we end up getting issues from people getting suddenly a worse reports from <INSERT_YOUR_FAVORITE_COMMERCIAL_AUTOMATIC_PENTEST_TOOL> (or things like ssh-audit as shown in this PR).

I'm mostly worried about 2 things:

re-introducing things like hmac-sha1 or [email protected] to the list of MACs
(to a lesser extend) adding additional kex algos based on DH

both are arguably not something that is future proof.

We might have a middle ground solution that makes things easier for users wanting to harden their systems while not making it worse for users relying on the default values: sshd allows you to add or remove items from its default values using the + or - characters at the beginning of the list.
For example, right now at the top of your PR I can have services.openssh.settings.Macs = ["-hmac-sha1" "[email protected]" "[email protected]"]; and hmac-sha1 / [email protected] / [email protected] will be removed from the available MACs (I agree it feels a bit like a hack 😄 and maybe we should expose an nicer way to control that?).

This approach would allow us to exclude the crypto primitives with the lowest security margins and pick up the new ones proposed by openssh (I'm assuming they will be sane...) when we upgrade and also not cause issues with the linkOpenssl = false; use case.

tomfitzhenry · 2024-06-06T01:25:23Z

The given arguments seem fair to me however I'm a bit worried of re-introducing weak cryptography primitives by default. This seems like a regression and I would not be surprised if we end up getting issues from people getting suddenly a worse reports from <INSERT_YOUR_FAVORITE_COMMERCIAL_AUTOMATIC_PENTEST_TOOL> (or things like ssh-audit as shown in this PR).

I agree there's a risk here, and we should enable users who have compliance/personal-driven reasons to easily manage that.

In some cases I think it's reasonable to expect the user to take on ownership of this config, but it'd be nice if we could offer some abstractions to help manage that.

We might have a middle ground solution that makes things easier for users wanting to harden their systems while not making it worse for users relying on the default values: sshd allows you to add or remove items from its default values using the + or - characters at the beginning of the list.
For example, right now at the top of your PR I can have services.openssh.settings.Macs = ["-hmac-sha1" "[email protected]" "[email protected]"]; and hmac-sha1 / [email protected] / [email protected] will be removed from the available MACs (I agree it feels a bit like a hack 😄 and maybe we should expose an nicer way to control that?).

I like this idea. It's a bit risky as-is, since "-" doesn't compose well with nix's lists. To explain what I mean, first let's look at sshd_config:

If the specified list begins
with a ‘+’ character, then the specified algorithms will
be appended to the default set instead of replacing them.
If the specified list begins with a ‘-’ character, then
the specified algorithms (including wildcards) will be
removed from the default set instead of replacing them.
If the specified list begins with a ‘^’ character, then
the specified algorithms will be placed at the head of
the default set.

This means if we have

{
  // nixos/modules/services/networking/ssh/sshd.nix
  // Potentially guarded by an option, but whether it is isn't important for this example.
  services.openssh.settings.Macs = lib.mkFirst ["-hmac-sha1"]; # remove MAC
}
{
  // my-personal-sshd.nix config
  services.openssh.settings.Macs = ["[email protected]"]; # add my favourite MAC
}

then these will accumulate to:

Macs -hmac-sha1,[email protected]

... and now your favourite MAC is unexpectedly disabled (since a "-" at the beginning will exclude all algorithms listed)!

But as you say, maybe there's a way to more nicely expose that?

Maybe expose a union-like option, with four alternatives:

default (which doesn't set "MACs")
additions (which uses the "+" syntax)
deletions (which uses the "-" syntax)
list (which uses types.list semantics)

We'd have an assertion that only one alternative can be used at a time.

Do we have any precedent for that sort of thing? Does it work? Is it too complex/overengineering? RFC42 and #51630 discuss this in a wider sense.

I'll put this PR into draft while we try to answer that.

tomfitzhenry · 2024-06-06T04:13:42Z

Maybe expose a union-like option, with four alternatives:
[...]
Do we have any precedent for that sort of thing? Does it work? Is it too complex/overengineering? RFC42 and #51630 discuss this in a wider sense.

I had a think through this some more.

I think introducing a union-like option (default, additions, deletions, list) would take us further from a declarative model, and be pretty complex.

I get the impression that most are happy with the status quo of overriding upstream's defaults, and many prefer it. Thus it's not worth complicating things. For those that want more control (or to use upstream's defaults), they can set to null, or use lib.mkForce.

Next steps:

I close this PR.
We continue to maintain a NixOS-recommended suite, which has essentially worked for us.
When I get more time, maybe I'll re-open nixos/ssh: introduce enableRecommendedAlgorithms #316896 to guard that with an option, at least.
I raise a PR to apply SuperSandro2000@02c08fc . nixos/initrd-ssh: set KexAlgorithms/Ciphers/MACs only if non-null #318185
I add a NixOS test for the = null case. tests/openssh: use upstream's algorithms in "no openssl" example #317745

Please comment and/or re-open if you think otherwise! :)

For posterity/fun, some resources I found re the difference of opinions re algorithm selection (which I don't hold strong opinions on):

NIST P curves: https://safecurves.cr.yp.to/ (response 1 2 3)
MACs: 1, 2

@SuperSandro2000

Prior to this commit, if services.openssh.settings.Macs is null, then initrd-ssh.nix would fail to build. Same for KexAlgorithms and Ciphers. Noticed by @SuperSandro2000: NixOS#316934 (comment)

github-actions bot added 6.topic: nixos 8.has: module (update) labels Jun 3, 2024

tomfitzhenry requested review from LeSuisse, aszlig and teto June 3, 2024 14:34

tomfitzhenry force-pushed the drop-openssh-algorithms-overrides branch from e5a9f80 to 928ad76 Compare June 3, 2024 14:36

tomfitzhenry mentioned this pull request Jun 3, 2024

nixos/ssh: introduce enableRecommendedAlgorithms #316896

Closed

13 tasks

tomfitzhenry requested a review from SuperSandro2000 June 3, 2024 14:41

aszlig requested a review from a team June 3, 2024 14:43

SuperSandro2000 reviewed Jun 3, 2024

View reviewed changes

ofborg bot added 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10 labels Jun 3, 2024

teto removed their request for review June 3, 2024 17:13

tomfitzhenry force-pushed the drop-openssh-algorithms-overrides branch from 928ad76 to a0e2211 Compare June 5, 2024 09:54

github-actions bot added 8.has: documentation 8.has: changelog labels Jun 5, 2024

SuperSandro2000 reviewed Jun 5, 2024

View reviewed changes

tomfitzhenry force-pushed the drop-openssh-algorithms-overrides branch 3 times, most recently from 1c18bbe to c13c3b0 Compare June 5, 2024 11:53

tomfitzhenry requested a review from joachifm as a code owner June 5, 2024 11:53

SuperSandro2000 reviewed Jun 5, 2024

View reviewed changes

tomfitzhenry force-pushed the drop-openssh-algorithms-overrides branch from c13c3b0 to 76531de Compare June 5, 2024 13:05

tomfitzhenry commented Jun 5, 2024

View reviewed changes

tomfitzhenry force-pushed the drop-openssh-algorithms-overrides branch from 76531de to 2907a5f Compare June 5, 2024 13:51

tomfitzhenry force-pushed the drop-openssh-algorithms-overrides branch from 2907a5f to d091822 Compare June 5, 2024 14:06

tomfitzhenry requested a review from dasJ as a code owner June 5, 2024 14:06

tomfitzhenry requested review from SuperSandro2000 and mweinelt June 5, 2024 14:10

tomfitzhenry marked this pull request as draft June 6, 2024 01:28

tomfitzhenry closed this Jun 6, 2024

tomfitzhenry mentioned this pull request Jun 8, 2024

nixos/initrd-ssh: set KexAlgorithms/Ciphers/MACs only if non-null #318185

Merged

13 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

nixos/ssh: use upstream's default cryptographic algorithms #316934

nixos/ssh: use upstream's default cryptographic algorithms #316934

tomfitzhenry commented Jun 3, 2024 •

edited

Loading

tomfitzhenry commented Jun 3, 2024

aszlig commented Jun 3, 2024

SuperSandro2000 left a comment

SuperSandro2000 left a comment

SuperSandro2000 Jun 5, 2024

SuperSandro2000 Jun 5, 2024

SuperSandro2000 Jun 5, 2024

SuperSandro2000 left a comment

tomfitzhenry commented Jun 5, 2024

SuperSandro2000 commented Jun 5, 2024

tomfitzhenry commented Jun 5, 2024

SuperSandro2000 commented Jun 5, 2024

aszlig commented Jun 5, 2024

tomfitzhenry commented Jun 5, 2024

tomfitzhenry commented Jun 5, 2024

tomfitzhenry Jun 5, 2024

mweinelt commented Jun 5, 2024

tomfitzhenry commented Jun 5, 2024 •

edited

Loading

tomfitzhenry commented Jun 5, 2024

tomfitzhenry commented Jun 5, 2024

tomfitzhenry commented Jun 5, 2024

tomfitzhenry commented Jun 5, 2024 •

edited

Loading

LeSuisse commented Jun 5, 2024

tomfitzhenry commented Jun 6, 2024 •

edited

Loading

tomfitzhenry commented Jun 6, 2024 •

edited

Loading

nixos/ssh: use upstream's default cryptographic algorithms #316934

nixos/ssh: use upstream's default cryptographic algorithms #316934

Conversation

tomfitzhenry commented Jun 3, 2024 • edited Loading

Motivation

Description of changes

Things done

tomfitzhenry commented Jun 3, 2024

aszlig commented Jun 3, 2024

SuperSandro2000 left a comment

Choose a reason for hiding this comment

SuperSandro2000 left a comment

Choose a reason for hiding this comment

SuperSandro2000 Jun 5, 2024

Choose a reason for hiding this comment

SuperSandro2000 Jun 5, 2024

Choose a reason for hiding this comment

SuperSandro2000 Jun 5, 2024

Choose a reason for hiding this comment

SuperSandro2000 left a comment

Choose a reason for hiding this comment

tomfitzhenry commented Jun 5, 2024

SuperSandro2000 commented Jun 5, 2024

tomfitzhenry commented Jun 5, 2024

SuperSandro2000 commented Jun 5, 2024

aszlig commented Jun 5, 2024

tomfitzhenry commented Jun 5, 2024

tomfitzhenry commented Jun 5, 2024

tomfitzhenry Jun 5, 2024

Choose a reason for hiding this comment

mweinelt commented Jun 5, 2024

tomfitzhenry commented Jun 5, 2024 • edited Loading

tomfitzhenry commented Jun 5, 2024

tomfitzhenry commented Jun 5, 2024

tomfitzhenry commented Jun 5, 2024

tomfitzhenry commented Jun 5, 2024 • edited Loading

LeSuisse commented Jun 5, 2024

tomfitzhenry commented Jun 6, 2024 • edited Loading

tomfitzhenry commented Jun 6, 2024 • edited Loading

tomfitzhenry commented Jun 3, 2024 •

edited

Loading

tomfitzhenry commented Jun 5, 2024 •

edited

Loading

tomfitzhenry commented Jun 5, 2024 •

edited

Loading

tomfitzhenry commented Jun 6, 2024 •

edited

Loading

tomfitzhenry commented Jun 6, 2024 •

edited

Loading