Review comments 2024-06-14

[emlun]

Follow-up to #11. This review concerns the version as of commit 5f1a31d0.

Sorry, I didn't have much time to do an in depth review, but it definitely seems a bit clearer now. 🙂 And we probably should indeed extract and expose an ARKG-Derive-Blinding-Factor function in ARKG so that you don't need to do the "hack" here of extracting the blinding factor by using the identity scalar as the secret key argument to ARKG-Derive-Secret-Key. I'll try to get on that in July when I'm back from vacation.

the public key pk_bl associated with sk_device blinded by sk_bl0, and

Maybe call this pk_bl0 to emphasize the relationship with sk_bl0 (if I understood that correctly)?

H: SHA-256 [FIPS180-4] using hash_to_field from [RFC9380] Section 5 with:

I think this would rather be the other way around: "hash_to_field using SHA-256". The approach I took in ARKG was to just reference the hash_to_field function defined in the hash-to-curve suite for that curve, I'd suggest doing the same here:

H: hash_to_field with the parameters:

• [...]
• L: The L defined in the Hash-To-Curve suite P256_XMD:SHA-256_SSWU_RO_
• expand_message: The expand_message function defined in the Hash-To-Curve suite P256_XMD:SHA-256_SSWU_RO_

It's a bit awkward because the suite doesn't define the hash_to_field function directly, just its arguments, but I think this works well enough. At least this way you don't need to restate the L and expand_message arguments, and also outsource the rationale for those argument choices.

An instantiation based on ECDH and MAC

I don't see any MAC being used for proof of possession anymore? Is this implicit?

[sander-cb]

Thanks for this follow-up review @emlun!

This review concerns the version as of commit 06bf2131.

Note that 06bf213 from 2024-05-26 is superseded by 5f1a31d which you linked to, which was updated 2024-06-08.

And we probably should indeed extract and expose an ARKG-Derive-Blinding-Factor function in ARKG so that you don't need to do the "hack" here of extracting the blinding factor by using the identity scalar as the secret key argument to ARKG-Derive-Secret-Key. I'll try to get on that in July when I'm back from vacation.

In the 2024-06-08 version, the hack was not needed anymore. Let’s look in July if the current way of applying ARKG is more idiomatic.

An instantiation based on ECDH and MAC

I don't see any MAC being used for proof of possession anymore? Is this implicit?

Indeed I have left this implicit, since the MAC generation seems out of scope for the interaction with the WSCD.

To resolve the comments, we need to:

• rename pk_bl
• improve reference to hash_to_field
• clarify how ECDH could be applied with MAC for proof of possession, e.g. in the mdoc context

[emlun]

Note that 06bf213 from 2024-05-26 is superseded by 5f1a31d which you linked to, which was updated 2024-06-08.

Oops, my bad - I copied the Markdown from my previous comment and updated the URL but not the display text. Fixed now.

In the 2024-06-08 version, the hack was not needed anymore

Oh? I saw this passage still in the text: "Note that the HDK scheme does not apply ARKG-Derive-Private-Key to the actual device private key [...]. Instead, HDK applies ARKG-Derive-Private-Key to the tree node’s key blinding private key [...]"

[sander-cb]

In the 2024-06-08 version, the hack was not needed anymore

Oh? I saw this passage still in the text: "Note that the HDK scheme does not apply ARKG-Derive-Private-Key to the actual device private key [...]. Instead, HDK applies ARKG-Derive-Private-Key to the tree node’s key blinding private key [...]"

Right, this is still in. But it’s way less hacky than it was a few weeks ago – indeed with an identity scalar as ARKG-Derive-Private-Key input. It seems like the way HDK 5f1a31d applies ARKG to the blinding scalar is quite idiomatic, and the issuer-wallet interface may be compatible with alternative ARKG applications.

[emlun]

Alright, I'll have another look when I'm back from vacation in two weeks!

[sander]

Closing with followup in #35.