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

Change default authorizedkeys file #586

Merged
merged 6 commits into from Oct 20, 2020

Conversation

otubo
Copy link
Contributor

@otubo otubo commented Sep 24, 2020

Proposed Commit Message

The following commit merged all ssh keys into a default user file
~/.ssh/authorized_keys in sshd_config had multiple files configured for
AuthorizedKeysFile:

commit f1094b1
Author: Eduardo Otubo otubo@redhat.com
Date: Thu Dec 5 17:37:35 2019 +0100

Multiple file fix for AuthorizedKeysFile config (#60)

This commit ignored the case when sshd_config would have a single file for
AuthorizedKeysFile, but a non default configuration, for example
~/.ssh/authorized_keys_foobar. In this case cloud-init would grab all keys
from this file and write a new one, the default ~/.ssh/authorized_keys
causing the bug.

rhbz: #1862967

Signed-off-by: Eduardo Otubo otubo@redhat.com

Additional Context

None

Test Steps

  1. Prepare a VM with cloud-init enabled
  2. Modify /etc/ssh/sshd_config to remove default .ssh/authorized_keys and change to another file e.g.:
# cat /etc/ssh/sshd_config | grep AuthorizedKeysFile
AuthorizedKeysFile .ssh/authorized_keys2
  1. Remove /var/lib/cloud/instance/sem/config_ssh
  2. systemctl restart cloud-init ; systemctl restart sshd
  3. Try to ssh login through the cloud-init created user

Actual results:
Cannot login: Permission denied (publickey,gssapi-keyex,gssapi-with-mic)
The public key is written into .ssh/authorized_keys but not .ssh/authorized_keys2

Expected results:
Can login successfully. The public key is written into .ssh/authorized_keys2

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

The following commit merged all ssh keys into a default user file
"~/.ssh/authorized_keys" in sshd_config had multiple files configured for
AuthorizedKeysFile:

commit f1094b1
Author: Eduardo Otubo <otubo@redhat.com>
Date:   Thu Dec 5 17:37:35 2019 +0100

    Multiple file fix for AuthorizedKeysFile config (canonical#60)

This commit ignored the case when sshd_config would have a single file for
AuthorizedKeysFile, but a non default configuration, for example
"~/.ssh/authorized_keys_foobar". In this case cloud-init would grab all keys
from this file and write a new one, the default "~/.ssh/authorized_keys"
causing the bug.

rhbz: #1862967

Signed-off-by: Eduardo Otubo <otubo@redhat.com>
@otubo
Copy link
Contributor Author

otubo commented Sep 24, 2020

The commit that adapts the test cases also fixes the second test case test_multiple_authorizedkeys_file_order2 which was testing exact the same scenario as the first one.

@@ -593,7 +593,7 @@ def test_multiple_authorizedkeys_file_order1(self, m_getpwnam):
fpw.pw_name, sshd_config)
content = ssh_util.update_authorized_keys(auth_key_entries, [])

self.assertEqual("%s/.ssh/authorized_keys" % fpw.pw_dir, auth_key_fn)
self.assertEqual(authorized_keys, auth_key_fn)
self.assertTrue(VALID_CONTENT['rsa'] in content)
self.assertTrue(VALID_CONTENT['dsa'] in content)
Copy link
Collaborator

Choose a reason for hiding this comment

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

unrelatedly, it's probably high time to retire DSA

Copy link
Collaborator

@OddBloke OddBloke left a comment

Choose a reason for hiding this comment

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

This LGTM, and I've tested it locally. Thanks!

@OddBloke OddBloke merged commit b0e7381 into canonical:master Oct 20, 2020
TheRealFalcon added a commit to TheRealFalcon/cloud-init that referenced this pull request Dec 2, 2020
If a non-default AuthorizedKeysFile is specified in
/etc/ssh/sshd_config,  ensure we can still ssh as expected
TheRealFalcon added a commit to TheRealFalcon/cloud-init that referenced this pull request Dec 2, 2020
If a non-default AuthorizedKeysFile is specified in
/etc/ssh/sshd_config,  ensure we can still ssh as expected
TheRealFalcon added a commit to TheRealFalcon/cloud-init that referenced this pull request Dec 2, 2020
If a non-default AuthorizedKeysFile is specified in
/etc/ssh/sshd_config,  ensure we can still ssh as expected
@TheRealFalcon TheRealFalcon mentioned this pull request Dec 2, 2020
3 tasks
OddBloke pushed a commit that referenced this pull request Dec 3, 2020
If a non-default AuthorizedKeysFile is specified in
/etc/ssh/sshd_config, ensure we can still ssh as expected
blackboxsw added a commit to blackboxsw/cloud-init that referenced this pull request Jan 14, 2021
…cal#586)

Revert PR canonical#586 related to where cloud-init writes authorized keys.

Avoid storing all the ssh keys for all users into the default
ssh config AuthorizedKeysFiles.

LP: #1839061
OddBloke added a commit that referenced this pull request Jan 19, 2021
OddBloke added a commit to OddBloke/cloud-init that referenced this pull request Jan 19, 2021
OddBloke added a commit that referenced this pull request Jan 19, 2021
andrewbogott pushed a commit to andrewbogott/cloud-init that referenced this pull request Jan 21, 2021
TheRealFalcon added a commit to TheRealFalcon/cloud-init that referenced this pull request Jan 22, 2021
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.

None yet

3 participants