nickray
commented
4 years ago I think I am fine with keeping extState in the credentialMac, although probably for different reasons than you.
Current
Pro:
- extState has the role of "associated data", so "should" be authenticated along with the rest of the credential
- a conforming authenticator can, if I understand our current spec correctly, skip storage of extState completely (thus potentially freeing precious ROM, addressing @conorpp's concern https://github.com/dicekeys/fido-key-derivation/pull/7#issuecomment-697246944) and in particular simply not mix it in during MC (makeCredential) operations
- this also allows use case 1. above; previously generated credentials will keep the "typo", newly generated credentials can fix it, if there is a way to change the extState on the authenticator while keeping the seedKey
Con:
- a conforming authenticator must process and hash the entire extState as delivered by the RP during each and every getAssertion (GA) code path (a compute and RAM requirement) -- whereas the integrity of extState is only relevant during "recovery attempts"
- Similarly I dislike extState being an input to the ES256 key generation process.
The optionality of extState in MC is what brings me nearly on board, its necessity in verification during GA is what still bothers me.
Alternative
The only alternative I could think of is the following (which has downsides of its own), just writing it down to get this out of my system:
credentialId = version || uniqueId || coreMac [, || extState || extMac ]
coreMac = HMAC(seed, rpIdHash || version || uniqueId)
C[0] = HMAC(seed, coreMac) # NB: no extState used here!
extMac = HMAC(seed, rpIdHash || version || uniqueId || coreMac || extState)
# or possibly just: HMAC(seed, coreMac || extState)where square brackets in credentialId mean
- either len(extState) is zero, in which case extMac is optional (keeping it is a very verbose way of saying len = 0, and bring up minimum credential ID length to 97B)
- or extState is non-zero, in which case extMac is compulsory
During GA, it would be compulsory to recalculate/check coreMac ( = bound to RP, uniqueId not modified), but optional to do so for extMac (extState authentic).
Pros:
- a conforming authenticator can truncate credentialId at 65B on receipt for GA operations (and hence boycott extState entirely)
- in the recovery scenario, the extState is still authenticated
Cons:
- two MACs (compute + length of credential ID)
- if extState integrity is not checked every time, in some weird edge case RP could be storing a corrupted version without being caught
I guess the original alternative as discussed earlier today was not having the second MAC, but just removing extState from the usual MAC and switching order to credentialId = version || uniqueId || MAC || extState.
Remarks
In practice, STM32L432 has 64KB of RAM while LPC55S69 has 320KB, so we have enough RAM :)
I somewhat fear if extState handling is needed in GA operations, then some (anti-extState) implementations may only be interoperable with others that do not include extState in the credential ID (i.e., calculate coreMac instead of credentialMac, complain if they do not coincide).
UppaJung
The most conventional way to use a MAC for the credentialId would be to MAC all of the fields.
We discussed two reasons why we might not want to include the extState in the MAC.
Regarding (1), once the extState is part of a credentialId, the relying party will be using that ID as an identifier and will not want it changed. Hence, it's not really able to change it regardles.
Regarding (2), a replica authenticator doesn't need to have matching extState. It only uses extState when registering new keys. For any existing keys, the extState is sent to it. A replacement authenticator will work just fine even if it doesn't have the same extState.
As such, unless there's another good reason put forth or I've made a mistake in discarding the above two, I'm going to propose we stick with having a MAC behave in the most common way: MAC'ing all the data in the message that comes before it.