I was recently looking at verifying the attestation data (ssh-sk-attest-v00) for a SK key, but I believe the data saved in this structure is insufficient for completing verification of the attestation. While the structure has enough information for U2F devices, FIDO2 devices sign their attestation over a richer "authData" blob [1] (concatenated with the challenge hash). The authData blob contains data not derivable from the public/private key, such as a signature counter and the device's AAGUID. As I understand it, the attestation structure should probably persist the entire authData blob to enable validation of the attestation. (This is really only getting into support for verifying "packed" attestation statements. Figuring out what to extract and persist is likely even more nuanced for other formats, but I'm not terribly inclined to go there myself.) Is there something I'm missing that would enable verification of the attestation signature for FIDO2 devices, or is this a correct assessment that the ssh-sk-attest-v00 file saved from ssh-keygen would not be enough? [1] https://www.w3.org/TR/2019/REC-webauthn-1-20190304/#sctn-attestation
On Fri, 4 Sep 2020, Ian Haken wrote:> I was recently looking at verifying the attestation data > (ssh-sk-attest-v00) for a SK key, but I believe the data saved in this > structure is insufficient for completing verification of the attestation. > While the structure has enough information for U2F devices, FIDO2 devices > sign their attestation over a richer "authData" blob [1] (concatenated with > the challenge hash). The authData blob contains data not derivable from the > public/private key, such as a signature counter and the device's AAGUID. As > I understand it, the attestation structure should probably persist the > entire authData blob to enable validation of the attestation. (This is > really only getting into support for verifying "packed" attestation > statements. Figuring out what to extract and persist is likely even more > nuanced for other formats, but I'm not terribly inclined to go there > myself.) > > Is there something I'm missing that would enable verification of the > attestation signature for FIDO2 devices, or is this a correct assessment > that the ssh-sk-attest-v00 file saved from ssh-keygen would not be enough? > > [1] https://www.w3.org/TR/2019/REC-webauthn-1-20190304/#sctn-attestationI haven't tried to parse the attestation blob for FIDO2 devices, so it's entirely possible that it doesn't work. We rely on libfido2 to obtain the attestation cert from the token, so if it has functions that let us get at the rest of what we need then it wouldn't be too much work to append/prepend that extra information to the blob. Pedro, do you have any insight here? -d
On Mon, 7 Sep 2020, pedro martelletto wrote:> > I haven't tried to parse the attestation blob for FIDO2 devices, > > so it's entirely possible that it doesn't work. > > > > We rely on libfido2 to obtain the attestation cert from the token, > > so if it has functions that let us get at the rest of what we need > > then it wouldn't be too much work to append/prepend that extra > > information to the blob. > > > > Pedro, do you have any insight here? > > > > -d > > I agree with Ian -- we need to persist the contents of > fido_cred_authdata_ptr(), or an external verifier won't be able to form a > complete picture of what was signed. > > Alternatively, we could persist only the AAGUID and the signature counter, and > let the verifier reconstruct authdata. Since authdata isn't strictly bounded > (its last constituent part is a CBOR map of variable content and length), > reconstructing it can get tricky.Thanks for the pointers - I think the attached patch should do it. -d -------------- next part -------------- diff --git a/PROTOCOL.u2f b/PROTOCOL.u2f index 5b8a062..f11494c 100644 --- a/PROTOCOL.u2f +++ b/PROTOCOL.u2f @@ -154,6 +154,16 @@ by trusted hardware before it will issue a certificate. To support this case, OpenSSH optionally allows retaining the attestation information at the time of key generation. It will take the following format: + string "ssh-sk-attest-v01" + string attestation certificate + string enrollment signature + string authenicator data + uint32 reserved flags + string reserved string + +A previous version of this format, emitted prior to OpenSSH 8.4 omitted +the authenticator data. + string "ssh-sk-attest-v00" string attestation certificate string enrollment signature diff --git a/sk-api.h b/sk-api.h index 6bf7c09..7d40679 100644 --- a/sk-api.h +++ b/sk-api.h @@ -45,6 +45,8 @@ struct sk_enroll_response { size_t signature_len; uint8_t *attestation_cert; size_t attestation_cert_len; + uint8_t *authdata; + size_t authdata_len; }; struct sk_sign_response { @@ -70,7 +72,7 @@ struct sk_option { uint8_t required; }; -#define SSH_SK_VERSION_MAJOR 0x00060000 /* current API version */ +#define SSH_SK_VERSION_MAJOR 0x00070000 /* current API version */ #define SSH_SK_VERSION_MAJOR_MASK 0xffff0000 /* Return the version of the middleware API */ diff --git a/sk-usbhid.c b/sk-usbhid.c index 9dde5c1..e36930e 100644 --- a/sk-usbhid.c +++ b/sk-usbhid.c @@ -781,6 +781,16 @@ sk_enroll(uint32_t alg, const uint8_t *challenge, size_t challenge_len, memcpy(response->attestation_cert, ptr, len); response->attestation_cert_len = len; } + if ((ptr = fido_cred_authdata_ptr(cred)) != NULL) { + len = fido_cred_authdata_len(cred); + debug3("%s: authdata len=%zu", __func__, len); + if ((response->authdata = calloc(1, len)) == NULL) { + skdebug(__func__, "calloc authdata failed"); + goto out; + } + memcpy(response->authdata, ptr, len); + response->authdata_len = len; + } *enroll_response = response; response = NULL; ret = 0; @@ -791,6 +801,7 @@ sk_enroll(uint32_t alg, const uint8_t *challenge, size_t challenge_len, free(response->key_handle); free(response->signature); free(response->attestation_cert); + free(response->authdata); free(response); } sk_close(sk); diff --git a/ssh-sk.c b/ssh-sk.c index 6a2422f..cd1645a 100644 --- a/ssh-sk.c +++ b/ssh-sk.c @@ -409,6 +409,30 @@ make_options(const char *device, const char *user_id, return ret; } + +static int +fill_attestation_cert(struct sk_enroll_response *resp, struct sshbuf *attest) +{ + int r; + + if (attest == NULL) + return 0; /* nothing to do */ + if ((r = sshbuf_put_cstring(attest, "ssh-sk-attest-v01")) != 0 || + (r = sshbuf_put_string(attest, + resp->attestation_cert, resp->attestation_cert_len)) != 0 || + (r = sshbuf_put_string(attest, + resp->signature, resp->signature_len)) != 0 || + (r = sshbuf_put_string(attest, + resp->authdata, resp->authdata_len)) != 0 || + (r = sshbuf_put_u32(attest, 0)) != 0 || /* resvd flags */ + (r = sshbuf_put_string(attest, NULL, 0)) != 0 /* resvd */) { + error("%s: buffer error: %s", __func__, ssh_err(r)); + return r; + } + /* success */ + return 0; +} + int sshsk_enroll(int type, const char *provider_path, const char *device, const char *application, const char *userid, uint8_t flags, @@ -496,19 +520,9 @@ sshsk_enroll(int type, const char *provider_path, const char *device, goto out; /* Optionally fill in the attestation information */ - if (attest != NULL) { - if ((r = sshbuf_put_cstring(attest, - "ssh-sk-attest-v00")) != 0 || - (r = sshbuf_put_string(attest, - resp->attestation_cert, resp->attestation_cert_len)) != 0 || - (r = sshbuf_put_string(attest, - resp->signature, resp->signature_len)) != 0 || - (r = sshbuf_put_u32(attest, 0)) != 0 || /* resvd flags */ - (r = sshbuf_put_string(attest, NULL, 0)) != 0 /* resvd */) { - error("%s: buffer error: %s", __func__, ssh_err(r)); - goto out; - } - } + if ((r = fill_attestation_cert(resp, attest)) != 0) + goto out; + /* success */ *keyp = key; key = NULL; /* transferred */
Thanks folks! This looks like it's exactly what I was looking for. As I'm pulling the thread on this, one word of warning on the fido_cred_authdata_ptr method. The following is mentioned libfido2 docs [1]: "The authenticator data returned by fido_cred_authdata_ptr() is a CBOR-encoded byte string, as obtained from the authenticator." This is a bit unfortunate since it's the CBOR-decoded data over which the attestation signature is computed (concatenated with the challenge hash). And of course you would also want to CBOR-decode the byte string before parsing the auth data structure. I just opened a question [2] on the libfido2 GH page to ask if there shouldn't be an API to return the CBOR-decoded data instead since really that's what you would want for any uses of the function. Basically, I think the openssh docs might also want to clarify that the "ssh-sk-attest-v01" structure similarly has "authenticator data" as a CBOR-encoded byte array (since customers would need to decode it to verify attestation), or else you may want to just CBOR-decode the output of fido_cred_authdata_ptr in sk-usbhid.c, at least until libfido2 (hopefully) follows up on my question and provides a convenience method for getting that decoded value directly. [1] https://developers.yubico.com/libfido2/Manuals/fido_cred_authdata_ptr.html [2] https://github.com/Yubico/libfido2/issues/212 On Mon, Sep 7, 2020 at 5:31 PM Damien Miller <djm at mindrot.org> wrote:> On Mon, 7 Sep 2020, pedro martelletto wrote: > > > > I haven't tried to parse the attestation blob for FIDO2 devices, > > > so it's entirely possible that it doesn't work. > > > > > > We rely on libfido2 to obtain the attestation cert from the token, > > > so if it has functions that let us get at the rest of what we need > > > then it wouldn't be too much work to append/prepend that extra > > > information to the blob. > > > > > > Pedro, do you have any insight here? > > > > > > -d > > > > I agree with Ian -- we need to persist the contents of > > fido_cred_authdata_ptr(), or an external verifier won't be able to form a > > complete picture of what was signed. > > > > Alternatively, we could persist only the AAGUID and the signature > counter, and > > let the verifier reconstruct authdata. Since authdata isn't strictly > bounded > > (its last constituent part is a CBOR map of variable content and length), > > reconstructing it can get tricky. > > Thanks for the pointers - I think the attached patch should do it. > > -d > > >
Maybe Matching Threads
- [PATCH 0/2] Fix Memory Management Issue in `ssh-sk-helper` with External SK Libraries
- [PATCH] remove stray `;` after function definitions
- Call for testing: OpenSSH 8.2
- [Bug 3761] New: ssh-keygen fails for security keys without attestation
- U2F support in OpenSSH HEAD