Skip to content

Fix permission of SSH host keys#1971

Merged
TheRealFalcon merged 5 commits into
canonical:mainfrom
Mazorius:main
Jan 25, 2023
Merged

Fix permission of SSH host keys#1971
TheRealFalcon merged 5 commits into
canonical:mainfrom
Mazorius:main

Conversation

@Mazorius
Copy link
Copy Markdown
Contributor

@Mazorius Mazorius commented Jan 19, 2023

private keys should have 0600: https://www.tenable.com/audits/items/CIS_Google_Container_Optimized_OS_v1.0.0_L1_Server.audit:6225b8224fbd4f360ebdc72c56f3eae9
public keys should have 0644: https://www.tenable.com/audits/items/CIS_Google_Container_Optimized_OS_v1.0.0_L1_Server.audit:7f016cd406100a1ee2ad94834111f005

Proposed Commit Message

fix: permissions for SSH host keys

If the host-keys are automatically generated the private key has 0640 which should be 0600 so no group or other can see the private key. The public key is correctly set to 0644.

If the host-keys are provided the script generated the private key with 0600 which is indeed correct. But the public key is also 0600 which should be instead 0644.

With this change the private key is always 0600 and the public key is always 0644.

Additional Context

During the execution of Terraform the keys permission were not set correctly.
The application could not generate the public fingerprint as it could not read the public host key.

After changing the private key to 0600 and the public to 0644 everything works as expected.

Test Steps

Install a tool like GitLab via Omnibus that require to generate the public host key fingerprint.
Open this page after setup: https://gitlab.example.com/help/configuration .

Which the correct permission the page should be open. With the wrong permission the page should throw a 500 HTTP Code.

Checklist:

  • My code follows the process laid out in the documentation
  • I have updated or added any unit tests accordingly
  • I have updated or added any documentation accordingly

@Mazorius
Copy link
Copy Markdown
Contributor Author

I not really get the issue in the test. Anyone an idea what the issue is? Cannot see something checks the permission.

Copy link
Copy Markdown
Collaborator

@igalic igalic 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 PR!

you'll need to sign the CLA, as per the Contribution guidelines

Comment thread cloudinit/config/cc_ssh.py Outdated
Comment thread cloudinit/config/cc_ssh.py
@Mazorius
Copy link
Copy Markdown
Contributor Author

thank you for this PR!

you'll need to sign the CLA, as per the Contribution guidelines

Ok. Need to do that tomorrow. The contribution link inside the readme.md is a deadlink.

@holmanb holmanb mentioned this pull request Jan 20, 2023
@holmanb
Copy link
Copy Markdown
Member

holmanb commented Jan 20, 2023

contribution link inside the readme.md is a deadlink.

I merged a commit that broke this link recently. It should point to https://cloudinit.readthedocs.io/en/latest/development/contributing.html.

@Mazorius
Copy link
Copy Markdown
Contributor Author

@igalic I cannot as I have not contact:

image

@igalic
Copy link
Copy Markdown
Collaborator

igalic commented Jan 20, 2023

from https://cloudinit.readthedocs.io/en/latest/index.html

When signing it:

  • ensure that you fill in the GitHub username field,
  • when prompted for ‘Project contact’ or ‘Canonical Project Manager’, enter ‘James Falcon’.

@Mazorius
Copy link
Copy Markdown
Contributor Author

Thanks @igalic
image

change to octal and use 420 for public
- 384 => 0o600
- 420 => 0o644
- 384 => 0o600
@igalic
Copy link
Copy Markdown
Collaborator

igalic commented Jan 20, 2023

you also need to add your name to this file tools/.lp-to-git-user

@Mazorius
Copy link
Copy Markdown
Contributor Author

I checked a bit in the network and find that the cert.pub should also has 644: https://access.redhat.com/documentation/en-us/red_hat_enterprise_linux/6/html/deployment_guide/sec-creating_ssh_ca_certificate_signing-keys#doc-wrapper

Therefore I changed this also to 644.

@aciba90 aciba90 self-assigned this Jan 23, 2023
Copy link
Copy Markdown
Contributor

@aciba90 aciba90 left a comment

Choose a reason for hiding this comment

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

Thanks, @Mazorius, for the improvements.

I am only concerned about the inline comment related to the change in group permissions on systems with ssh_keys group. Otherwise, looks good to me.

  • CLA signed

Comment thread cloudinit/config/cc_ssh.py Outdated
Copy link
Copy Markdown
Contributor

@aciba90 aciba90 left a comment

Choose a reason for hiding this comment

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

Marked as request changes until the ssh_keys block is reverted.

Comment thread cloudinit/config/cc_ssh.py Outdated
Copy link
Copy Markdown
Contributor

@aciba90 aciba90 left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks, @Mazorius for improving cloud-init!

Being this change a security related one, I will let it unmerged for a while to let others have a look.

Copy link
Copy Markdown
Contributor

@TheRealFalcon TheRealFalcon left a comment

Choose a reason for hiding this comment

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

LGTM!

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.

6 participants