Add seed generator metadata

[UppaJung]

That works for me, and is kinda what I was going for when I did the prior re-ordering. lib-sodium typically puts MACS and signatures at the start, so that everything that comes after that MAC block can be fed into the MAC calculation and then the MAC can just be prepended to the start of the array. The end works fine too and, looking at it again, I think it's better that way because we always want to start with the version byte.

On Tue, Aug 25, 2020 at 10:21 PM Nicolas Stalder notifications@github.com wrote:

@nickray commented on this pull request.

In fido-2-derivation-nicolas.md https://github.com/dicekeys/fido-key-derivation/pull/1#discussion_r476443105 :

@@ -53,7 +56,7 @@ We should specify (independent of used crypto library) whether P256 seed is inte

Outputs (sent to RP):

-- Credential ID: [version, raw_credential_id, tag] (1 + 32 + 32 = 65B) +- Credential ID: version || tag || raw_credential_id || seed_generator_metadata

An alternative might also be: version || raw_credential_id || metadata || tag.

• could set tag = H(K2, version || raw_credential_id || metadata) (advantage being don't have to "piece together" the inputs to the hash function, can just pass the slice)
• I first thought this would need a "length" byte/word, but it's not strictly necessary: tag is "last 16 bytes", version is first byte, raw_credential_id is bytes 2 to 17 (authenticator can simply treat metadata as "blog" in normal use)

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/dicekeys/fido-key-derivation/pull/1#pullrequestreview-474473961, or unsubscribe https://github.com/notifications/unsubscribe-auth/AB7AOZ5EOTHDXDRXOAV2YVLSCO3GZANCNFSM4QKRTNWA .

[UppaJung]

Thinking about this again, I'd really like to avoid "configurability", so if possible for the purposes of the "keys" spec, I'd like to treat this "additional metadata" (seed generator related or not) as opaque / independent of the "keys" spec.

Agreed. I called it seed_generator_metadata because it comes from the app that generated and wrote the seed. You don't need to know anything more about it than that and it shouldn't be part of your spec. All you have done is envision that the generator of a seed might also want to keep the recovery data state in the credential id, and it has allowed data to be stored there. The spec doesn't have to contain anything more about it.

We do need to figure out what a reasonable supportable length is. Since this is a new feature, we can say it is only available on keys that can support n bytes of storage for the metadata.

On Tue, Aug 25, 2020 at 10:36 PM Stuart Schechter < stuart.schechter@gmail.com> wrote:

That works for me, and is kinda what I was going for when I did the prior re-ordering. lib-sodium typically puts MACS and signatures at the start, so that everything that comes after that MAC block can be fed into the MAC calculation and then the MAC can just be prepended to the start of the array. The end works fine too and, looking at it again, I think it's better that way because we always want to start with the version byte.

On Tue, Aug 25, 2020 at 10:21 PM Nicolas Stalder notifications@github.com wrote:

@nickray commented on this pull request.

In fido-2-derivation-nicolas.md https://github.com/dicekeys/fido-key-derivation/pull/1#discussion_r476443105 :

@@ -53,7 +56,7 @@ We should specify (independent of used crypto library) whether P256 seed is inte

Outputs (sent to RP):

-- Credential ID: [version, raw_credential_id, tag] (1 + 32 + 32 = 65B) +- Credential ID: version || tag || raw_credential_id || seed_generator_metadata

An alternative might also be: version || raw_credential_id || metadata || tag.

• could set tag = H(K2, version || raw_credential_id || metadata) (advantage being don't have to "piece together" the inputs to the hash function, can just pass the slice)
• I first thought this would need a "length" byte/word, but it's not strictly necessary: tag is "last 16 bytes", version is first byte, raw_credential_id is bytes 2 to 17 (authenticator can simply treat metadata as "blog" in normal use)

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/dicekeys/fido-key-derivation/pull/1#pullrequestreview-474473961, or unsubscribe https://github.com/notifications/unsubscribe-auth/AB7AOZ5EOTHDXDRXOAV2YVLSCO3GZANCNFSM4QKRTNWA .

[nickray]

Hmm a side thought of pros/cons of metadata in middle / at end:

• if it's at the end (and not included in the tag), a resource limited authenticator can just skip it completely (and we don't really need a length in the spec).
• in the middle: as above, if it's to be authenticated, makes calculating the tag easier. need a length.

[UppaJung]

I'm confused as to why different orderings impact whether length information is needed. Regardless of orderings, the size of the variable length can be determined by taking the length of the credential id and subtracting the length of the other fields (all fixed length).

That said, the ordering issue is really one of simplicity of implementation, to which I defer to your team.

[nickray]

I'm confused as to why different orderings impact whether length information is needed. Regardless of orderings, the size of the variable length can be determined by taking the length of the credential id and subtracting the length of the other fields (all fixed length).

Yes agreed, as long as there's one field that's variable and the others are all fixed, as is the case currently.

[UppaJung]

Okay, then before I get back to Joe for another round, where do we want the tag? Is there a reason the end is harder than the middle? I didn't catch why.

[nickray]

Sorry, I missed this.

My current preference would be version || raw_credential_id || unspecified_metadata || tag, assuming the tag signs over the metadata too. The option version || raw || tag || metadata would be my preference if the tag does not sign the metadata (which it kinda doesn't have to if this metadata is only a hint how to regenerate the seed; because either the hint works or it doesn't..), as then in 99% of use cases one can just keep the first 65B and drop the rest immediately.

[nickray]

What is your proposal for max length of metadata? This needs to be specified so authenticators can allocate RAM appropriately.

[UppaJung]

Grreat. We're agreed on the field order.

I thinking about what to MAC, I agree it's helpful to think of keeping decoding simple:

1. version = CREDENTIAL_ID [0] (the first byte in the byte array) If version == 0, continue with the following steps, else throw error that version is not supported by this firmware.
2. Calculate MAC over the entire CREDENTIAL_ID with the exception of the tag (bytes [0...length-32, inclusive]
3. Return error if computed MAC doesn't equal the tag
4. Extract raw_credential_id as bytes CREDENTIAL_ID [1,32]
5. If for any reason you need the metadata, extract it as bytes CREDENTIAL_ID [33, length-32]

That would include both the version byte and the metadata in the MAC. I'd propose doing it this way as having a MAC cover an entire message is unlikely to change even if a future version adds more fields, so there's a lower chance the code would need to change in the future to accommodate new versions. The extra integrity protection never hurts since you can't verify the integrity without the key.

I'd suggest 256 bytes as the limit as that's the equivalent to the storage of eight 256-bit keys, which I'd hope most modern hardware would have enough space for. However, 128 might also work. In 64 bytes, you'd likely need to only use binary formats, and in anything below 64 and it becomes hard to incorporate a random 256-bit salt into the metadata if you want to to have a salt.

On Sat, Sep 5, 2020 at 9:16 AM Nicolas Stalder notifications@github.com wrote:

What is your proposal for max length of metadata? This needs to be specified so authenticators can allocate RAM appropriately.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/dicekeys/fido-key-derivation/pull/1#issuecomment-687486179, or unsubscribe https://github.com/notifications/unsubscribe-auth/AB7AOZYQRSC7YFQYTL32CADSEF7OJANCNFSM4QKRTNWA .

[nickray]

So 321 bytes in total? This is certainly do-able for both STM32L432 and LPC55, especially as in normal use you only need to do the checks described, and then keep the 32B raw credential / nonce.

[UppaJung]

321 bytes for the CREDENTIAL_ID (well under the 2048 byte limit servers are required to hold)

You'd need 288 bytes of long-term storage, 32 bytes for the master seed and up to 256 bytes for metadata)

On Sat, Sep 5, 2020 at 10:14 AM Nicolas Stalder notifications@github.com wrote:

So 321 bytes in total? This is certainly do-able for both STM32L432 and LPC55, especially as in normal use you only need to do the checks described, and then keep the 32B raw credential / nonce.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/dicekeys/fido-key-derivation/pull/1#issuecomment-687505720, or unsubscribe https://github.com/notifications/unsubscribe-auth/AB7AOZZE7CTZ7JWGGOJNGO3SEGGH7ANCNFSM4QKRTNWA .

[UppaJung]

I think the changes address our past conversations. I'll let @nickray officially approve/merge, but in the meantime I'll work on next set of changes before sending to joe.

[nickray]

The only open point for me is whether to use HMAC(X, Y) throughout vs H(X || Y) in some places. It's probably an easier sell to stick with HMAC throughout? And then Joe can tell us whether we should actually prefer raw hashes over HMACs anywhere?

That said, this PR is about adding metadata, so I "approved" (however, I cannot merge).