Authorized SSH keys for VM's based on cloud image

[skobyda]

Fixes #431

• Move DynamicListForm component to cockpit's pkg/lib

These ssh keys are then passed to cloud-init's user-data config as "ssh_authorized_keys": https://cloudinit.readthedocs.io/en/latest/reference/examples.html#configure-instances-ssh-keys

Machines: Add SSH keys to VM creation dialog

The provided keys are stored in the ~/.ssh/authorized_keys file of the designated non-root user, enabling immediate SSH access to the user account right after creating the VM.

[skobyda]

Updated the component to use TrashIcon

[marusak]

Any update on this?

[skobyda]

Any update on this?

Still on design-review phase

[skobyda]

@garrett do you mind looking at it once you find some time?

[skobyda]

Updated the design a bit. Now we validate the ssh key upon the user input, and once it's a valid key, we confirm its validity by turning a text-input field into a text:

Screencast.from.2023-09-11.14-26-14.webm

[garrett]

I thought it could be like this:

[skobyda]

@garrett updated the design:

Screencast.from.2023-10-02.14-11-55.webm

[jelly]

I tested adding a wrong key and noticed that there is no visual feedback when I enter an invalid ssh key. It would be nice if we had that as we do invoke ssh-keyscan -l.

[skobyda]

I tested adding a wrong key and noticed that there is no visual feedback when I enter an invalid ssh key. It would be nice if we had that as we do invoke ssh-keyscan -l.

I don't feel confident showing such an error. Reason is simple. Your ssh-keygen (which we use to validate public key) might not know all the types of ssh keys used. E.g. on my machine, it can recognize and validate only dsa, ecdsa, ecdsa-sk, ed25519, ed25519-sk, rsa. But cloud-init accepts more types, so sometimes validation might fail even though a public-key is perfectly fine for cloud-init. So in summary, we can validate that the key is correct (which we show by switching to a non-editable mod now), but we cannot validate that the key is incorrect.

[jelly]

Traceback (most recent call last):
  File "/work/bots/make-checkout-workdir/test/machineslib.py", line 475, in tearDown
    super().tearDown()
  File "/work/bots/make-checkout-workdir/test/common/testlib.py", line 1590, in tearDown
    self.check_journal_messages()
  File "/work/bots/make-checkout-workdir/test/common/testlib.py", line 1841, in check_journal_messages
    raise Error(UNEXPECTED_MESSAGE + "journal messages:\n" + first)
testlib.Error: FAIL: Test completed, but found unexpected journal messages:
/dev/stdin is not a public key file.

[martinpitt]

Thanks! The tests need work though, they crash with firefox. Should be reproducible locally with TEST_BROWSER=firefox?

[skobyda]

Blocked by cockpit-project/cockpit#19442

[garrett]

Blocked by cockpit-project/cockpit#19442

13 is carriage return (enter)
10 is linefeed

This is a CRLF issue.

HTML 5 spec says that textareas should send both CR and LF.
https://www.w3.org/TR/2021/SPSD-html52-20210128/sec-forms.html#the-textarea-element

Interpretation between browsers may be different, as there are different ways it is interpreted:

For historical reasons, the element’s value is normalized in three different ways for three different purposes. The raw value is the value as it was originally set. It is not normalized. The API value is the value used in the value IDL attribute. It is normalized so that line breaks use U+000A LINE FEED (LF) characters. Finally, there is the value, as used in form submission and other processing models in this specification. It is normalized so that line breaks use U+000D CARRIAGE RETURN U+000A LINE FEED (CRLF) character pairs, and in addition, if necessary given the element’s wrap attribute, additional line breaks are inserted to wrap the text at the given width.

It should be sending CRLF on a post. A guess: It might be normalized differently in JavaScript across browsers (as the CRLF normalization is specified to happen on a post, not necessarily JavaScript).

[garrett]

In other words, the code should check for both CR and LF, not just one or the other. It may need to look for CRLF explicitly... or CR and/or LF.

This might also vary not just on browser, but also on OS, as Windows famously uses different line endings than UNIX.

https://en.wikipedia.org/wiki/Newline#Representation

[martinpitt]

cockpit-project/cockpit#19442 landed, so you should be able to bump the testlib SHA. We just did it this morning, so there should be near-zero chance of something regressing.

[skobyda]

Test failures seem unrelated

[martinpitt]

Thanks! Unfortunately this has a conflict, otherwise I had accepted it. But one small test thing to add still.

[martinpitt]

Thanks! Tests are still failing 😭

[skobyda]

Looking at failing tests, I don't understand why (stdin) is not a public key file is logged to journal. That message comes from ssh-keygen, and I'm ignoring message from there: https://github.com/cockpit-project/cockpit-machines/pull/1150/files#diff-c3b5a228e7299618756cc30089b5e5f0b1e3662e48459df036e31676a874bec2R761

@martinpitt any idea what am I doing wrong here?

EDIT: nevermind, found the issue

[martinpitt]

Cheers! Assuming green, let's land this at last 🎉

[cockpituous]

This added line is not executed by any test. Details

Jelle's feedback got addressed

[martinpitt]

@skobyda Can you please add a proper screenshot for the release notes, and add an initial text? Thanks!