Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow setting MACs in SSHD config #91

Merged
merged 1 commit into from
Jan 5, 2024
Merged

Allow setting MACs in SSHD config #91

merged 1 commit into from
Jan 5, 2024

Conversation

mlacko64
Copy link
Contributor

Issue number:

#90

Description of changes:

This change adds option to customize MACs for SSH , for example, to disable SHA1 MACs which are reported as deprecated by vulnerability scanner. README.md updated.

Testing done:

Created custom container and tested in my lab cluster, works as expected.

SSHD config contains line:
MACs hmac-sha2-256,hmac-sha2-512,hmac-md5,hmac-md5-96,umac-64@openssh.com,umac-128@openssh.com,hmac-sha2-256-etm@openssh.com,hmac-sha2-512-etm@openssh.com,hmac-md5-etm@openssh.com,hmac-md5-96-etm@openssh.com,umac-64-etm@openssh.com,umac-128-etm@openssh.com

SSHD server offers just MACs selected by me:

debug2: MACs ctos: hmac-sha2-256,hmac-sha2-512,hmac-md5,hmac-md5-96,umac-64@openssh.com,umac-128@openssh.com,hmac-sha2-256-etm@openssh.com,hmac-sha2-512-etm@openssh.com,hmac-md5-etm@openssh.com,hmac-md5-96-etm@openssh.com,umac-64-etm@openssh.com,umac-128-etm@openssh.com
debug2: MACs stoc: hmac-sha2-256,hmac-sha2-512,hmac-md5,hmac-md5-96,umac-64@openssh.com,umac-128@openssh.com,hmac-sha2-256-etm@openssh.com,hmac-sha2-512-etm@openssh.com,hmac-md5-etm@openssh.com,hmac-md5-96-etm@openssh.com,umac-64-etm@openssh.com,umac-128-etm@openssh.com

Terms of contribution:

By submitting this pull request, I agree that this contribution is dual-licensed under the terms of both the Apache License, version 2.0, and the MIT license.

README.md Outdated
Comment on lines 96 to 97
"hmac-md5",
"hmac-md5-96",

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe drop weak md5 hmacs from the example?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call out. And you know, rather than give an explicit list example maybe show how one might use the setting to tweak the default? Is that intended to be supported here @mlacko64 ?

ie: if we consider https://man.openbsd.org/sshd_config#MACs

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.

I think we'd be able to express surgical "edits" to the defaults - maybe even this would be a model default setting? Something like this:

{
  "ssh": {
    "macs": [
      "-hmac-sha1"
    ]
  }
}

This way, any Bottlerocket settings (ie: other than build time defaults of OpenSSH itself) would be able to augment rather than exhaustively set the list. Settings could even be provided in the model to default remove insecure elements if/when the need arises.

In fact, after chatting with @jpculp I think this might apply to a few of the other "list" settings in ssh.

@jpculp jpculp self-requested a review December 18, 2023 22:57
Copy link
Contributor

@webern webern left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for this.

I'm not a subject matter expert, but this makes sense to me. We allow the configuration of Ciphers and KexAlgorithms, so it feels like MACs would be intended to be configurable as well.

Can you squash these into one commit? 4b2afc6 9b9c2b3

@mlacko64
Copy link
Contributor Author

Thanks for comments, I have:

  • removed "hmac-md5" and "hmac-md5-96" from example
  • updated readme.md with info that ciphers, key exchange algorithms and MACs can be also just added or removed from list
  • squashed all commits into one

@jpculp
Copy link
Member

jpculp commented Jan 4, 2024

@mlacko64, thanks for doing this! I think we can probably go ahead and merge this into an upcoming container release, but would you be willing to prune the commit message beforehand? The top line is probably enough to suffice.

Added MACs into SSHD configuration
@mlacko64
Copy link
Contributor Author

mlacko64 commented Jan 5, 2024

@jpculp sure, I have pruned commit message as you suggested

@jpculp jpculp merged commit 3508499 into bottlerocket-os:develop Jan 5, 2024
@mlacko64 mlacko64 deleted the macs branch January 8, 2024 08:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants