Hi Damien, Thanks for following up! On Dec 10, 2019, at 2:10 PM, Damien Miller <djm@xxxxxxxxxxx> wrote: > On Mon, 2 Dec 2019, Ron Frederick wrote: >> As part of my testing, I’ve been experimenting with not only plan SK >> keys, but also various combinations of creating certificates of SK >> keys signed by non-SK CA keys and vice-versa. I’ve generally been >> able to get OpenSSH to work with certificates of SK keys to work when >> signed with either non-SK or SK CAs. However, I haven’t been able to >> get OpenSSH to work when signing a non-SK key with an SK CA. I haven’t >> dug into the OpenSSH code to figure out what might be going wrong >> here, but I wanted to let you know about it. > > It looks like I neglected to add the SK algorithms to the server's > CASignatureAlgorithms list. I'll fix that now. Ah, great! Once you have a fix, I’d be happy to give it a try again here. >> One other thing I wanted to ask you about was whether you had any >> plans to support SK keys as host keys. Your current PROTOCOL.u2f file >> says that these keys “are not used for host-based user authentication >> or server host key authentication”. However, I think there are a few >> use cases where this could make sense. > > It wasn't something we'd really considered, but it wouldn't be very > hard to add. I'll look at it. Thanks! >> One use case is for reverse-direction connections, such as NETCONF >> “Call Home” described in RFC 8071. I know this isn’t something OpenSSH >> supports yet, but I recently added such support to AsyncSSH. See >> https://asyncssh.readthedocs.io/en/latest/#reverse-direction-example >> <https://asyncssh.readthedocs.io/en/latest/#reverse-direction-example> >> for more details. Basically, this use case involves a client running >> a process that makes an outbound TCP connection to a server but then >> using the resulting TCP connection once it is set up to authentication >> the remote server as a “user” and the client as a “host”, reversing >> the normal SSH direction and allowing the remote server to run >> commands on the local system. So, in the case of security keys, >> you’d create an SK server host key, and when you make the outbound >> connection and begin the key exchange, you’d hit the user presence >> button on the security key to perform the signing operation with the >> server host key and allow the reverse connection to be established. > > I'm not sure that I'm ready to add support to sshd for prompting the > user to touch their security key, I think it's too confusing as we've > never intended sshd to be run interactively. Agreed on this point. In the case of the reverse direction support, it’s actually a “client” application that ends up validating the host key, as it’s the thing making the outbound TCP connection in this case. The “server” that’s listening for new TCP connections would still be non-interactive, and so that’s where you’d actually want to use client keys that don’t require prompting, either by using non-SK keys or using SK keys with the presence requirement turned off. >> A second use case would be to simply use a security key with user >> presence disabled as a more secure key store than just keeping a >> private key on the local disk of an SSH server. As long as the >> security key was present, the SSH server could accept new connections. >> However, you could remove the key at any time to disable this >> function, and there would be no concern about the key being remotely >> stolen from the server — someone would need physical access to steal >> the security key and need the enrollment information for that to do >> them any good. > > This is good motivation - we'd probably want to require that security > keys do not require user presence for this case. For regular sshd listening for new connections, I think requiring that would make sense. >> Finally, I noticed a few minor things in the PROTOCOL.u2f doc that >> didn’t look right. Specifically, that doc says: >> >>> In addition to the message to be signed, the U2F signature operation >>> requires a few additional parameters: >>> >>> byte control bits (e.g. "user presence required" flag) >>> byte[32] SHA256(message) >>> byte[32] SHA256(application) >>> byte key_handle length >>> byte[] key_handle >> >> This isn’t really the format that these parameters are provided during >> a signing operation, though, at least not to the middleware library >> documented later in the doc. You may just want to leave this part of >> the description out, or at least sync it up a bit better with that >> later description. For instance, the key handle length is a size_t >> there, not a single byte, and the “control bits” is later called >> “flags”. The arguments are also in a different order, and there’s a >> missing argument which specified the key type (“alg”). > > AFAIK this is how things appear on the wire to a U2F token, but yeah > it's probably superfluous here. I must admit I haven’t looked closely at the FIDO protocol itself. For now, I’ve only focused on your middleware library and what it expects as arguments and returns as a result. That might be something worth documenting in PROTOCOL.u2f, as that’s the thing people are most likely to either want to implement if they have their own security keys that don’t work with libsk-libfido2 or to call into if they are writing alternate SSH Implementations which want to use that library as I did. >> Following this you show the signed blob as: >> >>> This signature is signed over a blob that consists of: >>> >>> byte[32] SHA256(application) >>> byte flags (including "user present", extensions present) >>> uint32 counter >>> byte[] extensions >>> byte[32] SHA256(message) >> >> How would the “extensions” be encoded here, though, and how would the >> hardware know where they end and the SHA256 of the message begins? >> The doc mentions an “extensions present” flag, but I don’t see that >> defined. > > extensions are only possible for FIDO2 keys and nothing we've defined > requests them (if they did, they'd break the signature). I mentioned it > there because that's where they'll go in the future if we ever support > them. My main concern on this was not finding a definition for the “extensions present” flag in the current code, or knowing how the length of the extensions might be encoded. Without that, there’s no way to future-proof my current implementation. I suppose I could do something like extract the last 32 bytes of the blob and use that as a the signature, ignoring any bytes in between but that’s not something usually done when parsing SSH “packets". >> Just after this, you show the signature returned from the U2F hardware >> as: >> >>> The signature returned from U2F hardware takes the following format: >>> >>> byte flags (including "user present") >>> uint32 counter >>> byte[32] ecdsa_signature (in X9.62 format). >> >> The signature is more than 32 bytes here, though. The middleware >> library returns the signature as an (r, s) pair, where each is a >> 32-byte string value that is later converted to integers and then >> encoded as a pair of MPInts. I suspect the hardware might be returning >> (r, s) as DER encoded in some cases and that the middleware library is >> hiding that, but either way the text above isn’t quite right. > > Yes, it's DER encoded. I'll adjust the docs. Ok. When going through your middleware API, though, you don’t actually see the DER encoding. That library converts the output from the hardware into a byte string big-endian representation of the r & s integers values in the ECDSA case, or puts the Ed25519 signature into the r value, with s being left empty. That might be something you’d want to document as well, in a section about the middleware API. >> Later, in the description of the sk_enroll() call, you show a >> “challenge” argument, but it’s not clear how that’s used. Are you >> doing anything with that today? I tried looking in various online docs >> about U2F/FIDO to see if it was described there, but I couldn’t really >> find anything that matched up with that. Most of what I found was much >> too high-level, or focused on things like the Javascript APIs in the >> browser and not the underlying code talking to the hardware tokens. > > The challenge can be used as part of a workflow that uses the attestation > information coming back from the security key. The attestation object can > be used to prove that a particular key is hosted in hardware, typically > down to manufacturer/batch granularity. The challenge is used to ensure > that attestations are not subject to replay. > > We don't disclose the attestation in the public key as it gives away a > bit too much identifying data, but I have plans to optionally write it > out as a sidecar to the private key. Yeah - I like that choice to keep the attestation response out of the encoded keys, but still make it available to callers who want it. I just wasn’t sure what to pass in for the challenge when calling sk_enroll(). It turned out that I needed to pass in a 32-byte block, which I’m guessing would generally be a SHA256 hash of some challenge data. It worked fine to pass 32 null bytes, but it didn’t work if I passed in anything which wasn’t that length. >> Thanks for all your work on this! I look forward to doing more testing >> on this as I fill out things like key enrollment and talking to an SSH >> agent process. I’ll report back here if I run into any issues with >> that. > > Thanks for porting it to another SSH implementation. I should mention > that there's a small chance that some of the formats might change before > we make a release with this code in it, but I'll try to let you know if > this happens. Completely understood, and I actually included a caveat about exactly that in my check-in notice when I pushed this to my “develop” branch on Github. I’d be more than happy to make any changes needed to stay in sync with OpenSSH, and plan on waiting until you have a released version of this before I cut my own release so there are no interoperability problems between the two implementations. -- Ron Frederick ronf@xxxxxxxxxxxxx _______________________________________________ openssh-unix-dev mailing list openssh-unix-dev@xxxxxxxxxxx https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev
