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.

Sign up for GitHub

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

Jump to bottom

sru: 19.4.33 manual 4 test for multiple ssh keys #92

Merged
merged 2 commits into from Jan 28, 2020
Merged

sru: 19.4.33 manual 4 test for multiple ssh keys #92

merged 2 commits into from Jan 28, 2020

Conversation

blackboxsw
Copy link
Collaborator

No description provided.

OddBloke
OddBloke requested changes Jan 27, 2020
View reviewed changes
bugs/sru-19.4.33-manual-4.txt Outdated
@@ -0,0 +1,306 @@
=== Begin SRU Template ===
[Impact]
Images which provide multiple ssh authorized key files for users via
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Images which provide multiple ssh authorized key files for users via
Images which configure multiple SSH authorized key files by

bugs/sru-19.4.33-manual-4.txt Outdated
configuration resulting in no remote ssh connectivity due to the absence of
any .ssh/authorized_keys file.

In this scenario, old/broken cloud-init would emit
Copy link
Member

Choose a reason for hiding this comment

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

"subdirectory" is in this sentence twice, which suggests to me it's mid-edit. Can we clarify this wording, please?

bugs/sru-19.4.33-manual-4.txt Outdated
EOF

cat > setup_ssh_multi_keys.sh <<EOF
sed -i 's/#AuthorizedKeysFile.*/AuthorizedKeysFile %h\/\.ssh\/authorized_keys \/default_keys/' /etc/ssh/sshd_config
Copy link
Member

Choose a reason for hiding this comment

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

I find it much easier to read path substitutions if you use something other than / for the sed delimiter:

Suggested change
sed -i 's/#AuthorizedKeysFile.*/AuthorizedKeysFile %h\/\.ssh\/authorized_keys \/default_keys/' /etc/ssh/sshd_config
sed -i 's,#AuthorizedKeysFile.*,AuthorizedKeysFile %h/.ssh/authorized_keys /default_keys,' /etc/ssh/sshd_config

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Nice, Thanks TIL I could replace that delimiter

bugs/sru-19.4.33-manual-4.txt


[Regression Potential]
If ssh key publishing fails on single AuthorizedKeyFiles setup, vms would be
Copy link
Member

Choose a reason for hiding this comment

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

I don't really know what this means.

bugs/sru-19.4.33-manual-4.txt Outdated
lxc exec test-$series -- find /home/ubuntu/.ssh
echo '--- Expect only chad.smith and localkey in .ssh/authorized_keys ---'
lxc exec test-$series -- wc -l /home/ubuntu/.ssh/authorized_keys
lxc exec test-$series -- grep -v chad.smith /home/ubuntu/.ssh/authorized_keys
Copy link
Member

Choose a reason for hiding this comment

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

Filtering out one of the lines makes this a lot harder to follow:

Suggested change
lxc exec test-$series -- grep -v chad.smith /home/ubuntu/.ssh/authorized_keys
lxc exec test-$series -- cat /home/ubuntu/.ssh/authorized_keys

bugs/sru-19.4.33-manual-4.txt Outdated
lxc exec test-$series -- find /home/ubuntu/.ssh
echo '--- Expect chad.smith, localkey and defaultkey in authorized_keys ---'
lxc exec test-$series -- wc -l /home/ubuntu/.ssh/authorized_keys
lxc exec test-$series -- grep -v chad.smith /home/ubuntu/.ssh/authorized_keys
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
lxc exec test-$series -- grep -v chad.smith /home/ubuntu/.ssh/authorized_keys
lxc exec test-$series -- cat /home/ubuntu/.ssh/authorized_keys

bugs/sru-19.4.33-manual-4.txt Show resolved Hide resolved
bugs/sru-19.4.33-manual-4.txt Outdated
=== Begin SRU Template ===
[Impact]
Images which provide multiple ssh authorized key files for users via
setting AuthorizedKeysFiles = <file1> <file2> would result in an invalid ssh
Copy link
Member

Choose a reason for hiding this comment

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

It's not an invalid SSH configuration; the fact that it's valid and we don't handle it is the problem we're fixing.

bugs/sru-19.4.33-manual-4.txt Outdated
/etc/ssh/sshd_config AuthorizedKeysFiles value.

[Test Case]
Create an lxc instance with AuthorizedKeyFiles set to mulitple files.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Create an lxc instance with AuthorizedKeyFiles set to mulitple files.
Create an LXD instance with `AuthorizedKeyFile` set to multiple files.

bugs/sru-19.4.33-manual-4.txt Outdated
lxc exec test-$series -- grep -v chad.smith /home/ubuntu/.ssh/authorized_keys
done


Copy link
Member

Choose a reason for hiding this comment

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

Lots of newlines.

@blackboxsw blackboxsw force-pushed the sru/19.4.33-manual-4-multi-sshauth branch from 229ddd3 to 0646620 Compare January 28, 2020 00:07
@blackboxsw blackboxsw force-pushed the sru/19.4.33-manual-4-multi-sshauth branch from 0646620 to 72e285f Compare January 28, 2020 03:10
OddBloke
OddBloke reviewed Jan 28, 2020
View reviewed changes
bugs/sru-19.4.33-manual-4.txt Outdated
.ssh/authorized_keys

[Test Case]
Create an lxc instance with AuthorizedKeyFiles configured for mulitple files:
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Create an lxc instance with AuthorizedKeyFiles configured for mulitple files:
Create an lxc instance with AuthorizedKeyFiles configured for multiple files:

OddBloke
OddBloke reviewed Jan 28, 2020
View reviewed changes
bugs/sru-19.4.33-manual-4.txt
+ echo --- Expect chad.smith, localkey and defaultkey in authorized_keys ---
--- Expect chad.smith, localkey and defaultkey in authorized_keys ---
+ lxc exec test-xenial -- cat /home/ubuntu/.ssh/authorized_keys
ssh-rsa localkey
Copy link
Member

Choose a reason for hiding this comment

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

This looks like a failure to me. We're missing the LP keys and the defaultkey.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Strange, I think there was a timing issue in the manual test, if I perform the following now on the same lxc that I haven't touched since the test was run, it contains all expected keys:

lxc exec test-xenial cat /home/ubuntu/.ssh/authorized_keys
ssh-rsa localkey
ssh-rsa defaultkey
ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQDSL7uWGj8cgWyIOaspgKdVy0cKJ+UTjfv7jBOjG2H/GN8bJVXy72XAvnhM0dUM+CCs8FOf0YlPX+Frvz2hKInrmRhZVwRSL129PasD12MlI3l44u6IwS1o/W86Q+tkQYEljtqDOo0a+cOsaZkvUNzUyEXUwz/lmYa6G4hMKZH4NBj7nbAAF96wsMCoyNwbWryBnDYUr6wMbjRR1J9Pw7Xh7WRC73wy4Va2YuOgbD3V/5ZrFPLbWZW/7TFXVrql04QVbyei4aiFR5n//GvoqwQDNe58LmbzX/xvxyKJYdny2zXmdAhMxbrpFQsfpkJ9E/H5w0yOdSvnWbUoG5xNGoOB csmith@fringe # ssh-import-id lp:chad.smith

ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAACAQDvl3VfPjVXsXBsm6r2J+UneIMr4ZOJhQlXuBWTwzexbd/XugB3k5EXA18yyqjEVT+bApVwlxATY66drVUPBuZ2JMU1HuLOKhG6toZd7j042oV5b2TEvg0es9qxs9mtGzvMPf3mB3tBVY/ESall023M+J5JjGGSO4J3zM/9c+P3Hs7xyCjAoySZDN2VZzscPgSGZzck8xtyO39uPfscKXi9LJkkhDDG6SVWie5OeM8TxyH2W2eNDKeXid/qgdIxqRLSYiNnWpt9htI0SzahnFYtsw9VLkij+0cM29lBIGUr5AehN2Y6jetxODR3pZt4YqOiyC6D5NaEsVGKOb0zjIBBCso6mIseejlOwocSYUH21YnLDS2Mu31bHRmPjpRvMVTOFtnS2OkfOxYTyMNFZ5PH/a0/t3DGxZZqz74F+APxG1X0vsgSFA9yYzbBaY3fr3vNAEYsRMTeBIjF6Gx6QmX3/kw5KBid4t8qQCV4Z1l8UmWZu4qFYxV/Z0IYPZazgYy/1W0qfRm5AdvpDdH9XArIokwqe1E2Djp5/xWp4Z9dAINmfJvNZxiDJk7gQz+Hdka/1U/f3wQSds9OAjF+a94Lj+F9CmMrhpVEZG5OL8ysK4iwSOsDhW7iLeZw5AO7cVhDUWj53/p2FP4+zxin/tYkDhNTJF0Nhc2uLMLxRCOGrQ== csmith@uptown # ssh-import-id lp:chad.smith

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

sleep 10 FTW ( I think we may have a timing issue with cloud-init status --wait on lxc that we'll need to sort at some point).... like --wait doesn't block if the results.json or status.json is completely empty possibly.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I added a card to look into the lxc reboot race https://trello.com/c/Epf5kwpH/50-test-cloud-init-status-wait-across-on-lxc-reboot-for-race

OddBloke
OddBloke requested changes Jan 28, 2020
View reviewed changes
Copy link
Member

@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.

Looks to me like we're seeing a verification failure on xenial; am I misreading something?

OddBloke
OddBloke approved these changes Jan 28, 2020
View reviewed changes
Copy link
Member

@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.

Thanks!

@OddBloke OddBloke merged commit 6ba19bf into cloud-init:master Jan 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@OddBloke OddBloke OddBloke approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@blackboxsw @OddBloke