nova should accept pre-pended comments in public keys

[Chris Hines]

Prior to commit 3f3f9bf22efd2fb209d2a2fe0246f4857cd2d21a
nova/crypto.py generate_fingerprint used ssh-keygen -q -l -f <pubkey_file> to generate finger prints.
ssh-keygen -qlf is quite happy to process public key matter of the form

cert-authority ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQCfHlWGrnpirvqvUTySnoQK6ze5oIXz7cYIT+XCBeBCahlK05O38g0erBGrNWFozZwbIXnysVCibaUJqtH0JrYqmcr2NnYA0PoiTeranvaJI7pQsga1gBxfK/D4UItw5yI6V7w9efMT0zpIP8WEubQz6GFtkyiNVgFCHj3+VhLs3RslvYzb35SFcLXEDsGVQM5NdWBUgRaNRqpTPvuMcxTyPvy32wW72kwaYRQioDJFcE2WJ240M2oSsx+dhTWvI8sW1sEUI1qIDfyBPsOgsLofuSpt4ZNgJqBUTp/hW85wVpNzud6A4YJWHpZXSDMtUMYE9QL+x2fw/b26yck9ZPE/ hines@tun

The issue is the string cert-authority at the beginning of the public key matter. This form can appear in authorized_keys to enable multiple users on a project to have individual keys certified by a central certifying authority providing access to a single administrative account. The use of ssh certificates is documented here:

https://www.digitalocean.com/community/tutorials/how-to-create-an-ssh-ca-to-validate-hosts-and-clients-with-ubuntu

Steps to reproduce:

1) Place the string """
cert-authority ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQCfHlWGrnpirvqvUTySnoQK6ze5oIXz7cYIT+XCBeBCahlK05O38g0erBGrNWFozZwbIXnysVCibaUJqtH0JrYqmcr2NnYA0PoiTeranvaJI7pQsga1gBxfK/D4UItw5yI6V7w9efMT0zpIP8WEubQz6GFtkyiNVgFCHj3+VhLs3RslvYzb35SFcLXEDsGVQM5NdWBUgRaNRqpTPvuMcxTyPvy32wW72kwaYRQioDJFcE2WJ240M2oSsx+dhTWvI8sW1sEUI1qIDfyBPsOgsLofuSpt4ZNgJqBUTp/hW85wVpNzud6A4YJWHpZXSDMtUMYE9QL+x2fw/b26yck9ZPE/ hines@tun
"""
in a file
2) run nova keypair-add --pub-key <filename> <keypair name>

Expected result:
They nova keypair-list should now list the key

Actual result:
ERROR (BadRequest): Keypair data is invalid: failed to generate fingerprint (HTTP 400)

Environment:
Openstack liberty release (bug is not present on kilo)

Logs:
Sorry, not available (I'm only a user not an admin)

Suggest fix: either:
1) revert generate_fingerprint to using exec ssh-keygen
2) generate_fingerprint should strip the string cert-authority from the begining of the public key matter (if present) before attempting to generate the fingerprint.

[Augustina Ragwitz (auggy)]

Here is a link to the change mentioned in the bug report - http://git.openstack.org/cgit/openstack/nova/commit/?id=3f3f9bf22efd2fb209d2a2fe0246f4857cd2d21a

This change implemented key generation using paramiko. The cert-authority issue has been reported to paramiko - https://github.com/paramiko/paramiko/issues/771

I think this shows a gap in our current test coverage. We should add a test for the "cert-authority" case as described above. I am confirming this bug to add this test coverage.

[Chris Hines (chris-hines)]

Hi Guys,
thanks for taking an interest in this bug (I was afraid it wouldn't get any interest since very few people use this very useful feature of OpenSSHd). Acouple of things I should point out:

1) Augustina identified the commit that broke things but missed a subsequent commit to the code:

http://git.openstack.org/cgit/openstack/nova/commit/?id=452fe92787ff871417846748fc13e2a6a2899325

which moves the upstream dependency from paramiko to cryptography

2) The issue of using a cert-authority in ssh_known_hosts to authenticate hosts to users is related (and indeed they should both be fixed by the upstream dependency) but I think the issue of using a cert-authority to authenticate users (via either ~/.ssh/authorized_keys of /etc/ssh/sshd_config TrustedUserCA option) is more important.

3) Another related case of the use of SSH_FORCE_COMMAND in the authorized_keys file. I'm not sure if anyone has ever tried this (indeed it probably makes more sense as cloud-init user data) but it might be useful to be able to set a pub key with a restricted shell in nova. For example this (take from /root/.ssh/authorized_keys)

no-port-forwarding,no-agent-forwarding,no-X11-forwarding,command="echo 'Please login as the user \"debian\" rather than the user \"root\".';echo;sleep 10" ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQDgG1+9o2Bryv/1hPfRzzrZp7GmBx4NenAxNoG5koZGV/+1Y/UyeYKp8Ho7DKp2xHQbvJTLd2PcEo+mRgknnaSX/sLoxN099hMHM2T6K9shkGRLP8m+RGSTa5kWjwaDJ5rWwtH06Oj7e84sJh1Wv2IAPCmpK0PqkNcv2GFsALhVgcIW27zBb7o2Yg5yUahnpuwBcojocG172wGSsYQw7sAXPbS+W2ohtBwPeI9aL0ET3W5isKR6zcTuLmulvgMvD9qVKJGaAQMfZrlIKyDpHNUcWCOi9xk4oLBT3nAwMfzvSxMU73nNEX29L0M1eTB6ab4Lnn54rlHPOsO538TsPjt5 hines@tun

is acceptable to ssh-keygen -qlf but would fail with either paramiko or cyptography

Please let me know if there is anything I can help with or who I should submit merge requests to. I'm happy to generate test cases and fixes (just a little unsure on the appropriate procedure around here)

Cheers,
--
Chris.

[Augustina Ragwitz (auggy)]

First off, apologies, when I first glanced at your bug I initially thought you were reporting an issue with Nova not accepting valid public key files. Upon further investigation, I attempted to follow your reproduction steps by adding a public key generated by a certificate authority and did not experience the error. When I looked at that public key file, it did not have the prefix "cert-authority". Upon further investigation, it looks like these libraries are working as expected for the key files formatted per RFC 4253: https://tools.ietf.org/html/rfc4253#section-6.6.

So the big question I have for you is what exactly is your workflow for generating the public key that you are trying to add to nova?

[Chris Hines (chris-hines)]

Thanks Augistina,
I believe you are correct, nova does accept keys generated according to rfc4253. At issue is whether nova should accept only keys formated according to rfc4253, or all contents intended for the default users authorized_keys file. (The format of the authorized_keys file isn't covered by the RFC, but by the man page for OpenSSHD https://www.freebsd.org/cgi/man.cgi?sshd(8) under the section AUTHORIZED KEYS FILE FORMAT.)

My point is that keys generated according to RFC4253 are a subset of the content people expect to put in the authorized_keys. Now nova already goes beyond RFC4253 by accept a comment field on the ened of the public key matter (or more specifically both paramiko and python-cryptography go beyond RFC4253).
Its tempting to say that nova shouldn't go beyond RFC4253 because it already support public keys, but I think this is a loss of functionality.

So my workflow was roughtly:

ssh-keygen -f ca
ssh-keygen -f personal
echo "cert-authority " > uploadthis.pub
cat ca.pub >> uploadthis.pub
nova keypair-add -f uploadthis.pub ca_keypair
ssh-keygen -s ca -I Chris_Key -n debian,ubuntu,ec2-user -V +520w personal.pub # this produces personal-cert.pub
eval `ssh-agent` # note that gnome-ssh-keyring doesn't work, Mac OS keyring does, OpenSSH ssh-agent does
ssh-add personal
nova boot .....
ssh ...

I then collect pub keys from my colleagues and repeat the ssh-keygen -s and lock up the file "ca" (the private part) in a safe. My colleagues don't need access to the private part of ca because they have their own cert (in the same way my web server doesn't need access to the private part of the x509 root cas for SSL verification). When ever we on board a new team member, the ca file comes out of the safe. Whenever a team member leaves, we update the certificate revocation list (Actually we haven't had to do this yet, since this is a new workflow). Because everyone had their own personal key we don't need to rekey the instances.

Other workflows that are impacted involve pre-pending differemt material to the front of the public key. For example if you wish to use nova to setup an rsync server that no one can log into, and restrict rsync to a subdirectory you could follow instructions here

https://www.samba.org/ftp/unpacked/rsync/support/rrsync

The full list of options is in the sshd man page

Thanks for your time looking at this (and reading my long comments :-)

Cheers,
--
Chris.

[Augustina Ragwitz (auggy)]

This seems to me to be more of a feature request than an actual bug. If you'd like Nova to support prepended comments in public keys, feel free to propose it per our blueprint process.

https://wiki.openstack.org/wiki/Blueprints

[Sam Morrison (sorrison)]

I think this is a valid bug, something did work, a change happened in nova and now it now longer works. Seems like a regression to me?

[Blair Bethwaite (blair-bethwaite)]

Agreed, this is a regression, please treat as such

[Matt Riedemann (mriedem)]

FYI: https://review.openstack.org/#/c/367395/

[Augustina Ragwitz (auggy)]

Is there a valid use case for supporting this? Should this have worked this way before in the first place?

[Augustina Ragwitz (auggy)]

If we want to include this bug in the paramiko fix Diana Clarke is working on, I'd suggest that we need to include the following:
* Unit test coverage that explicitly tests for the prepended text so the test breaks if we ever change this
* Documentation updates making it explicit that this is something we support and supported workflows for using it
* Validation and test coverage to ensure the prepended text doesn't cause failures (eg, what happens if someone prepends a unicode snowman)
* Potential security impacts of supporting prepended text (or a statement that there are none along with justification)

Given that I'm not clear what the use cases are for supporting this, I'm not sure it's worth the overhead and potential added complexity. That said, we could also not do the above things and just not officially support it, in which case this bug is still invalid.

If we are going to say this is a legit issue and recognize this as a regression, then we need to properly support this feature.

[Sean Dague (sdague)]

Automatically discovered version liberty in description. If this is incorrect, please update the description to include 'nova version: ...'

[Arnaud Morin (arnaud-morin)]

I am affected by this bug also.
I am working on Newton release for now, but this is even affecting the current master.

[Swe W Aung (sirswa)]

Is there any update on this bug?
We have few projects which wants to make use of ssh ca instead of personal ssh keypair.

regards,

[John Garbutt (johngarbutt)]

Seems like no progress in paramiko, but from the scientific-sig discussion this was clearly very useful and a regression.

I guess it should be noted that you can inject multiple keys and different keys via scripts in user-data, but clearly that isn't a great user experience compared to first class support via the regular key system.