Re: [CFRG] Call for adoption for draft-dew-cfrg-signature-key-blinding

[Christopher Wood <[e-mail]>]

Thanks for the thorough review, Ian! I filed a couple issues to track various suggestions you made:

- https://github.com/chris-wood/draft-dew-cfrg-signature-key-blinding/issues/33
- https://github.com/chris-wood/draft-dew-cfrg-signature-key-blinding/issues/32
- https://github.com/chris-wood/draft-dew-cfrg-signature-key-blinding/issues/31

Please see inline below for responses to some of your specific comments and questions.

> On Jun 5, 2022, at 10:47 AM, Ian Goldberg <[e-mail]> wrote:
> 
> I support adoption.  My initial review follows.
> 
> Notes from 5 Jun 2022 on
> https://chris-wood.github.io/draft-dew-cfrg-signature-key-blinding/draft-dew-cfrg-signature-key-blinding.html
> version of 29 Apr 2022
> 
> In the abstract, the "core property" you give for key blinding (as well
> as the "moreover" property) would also apply to "just generate a fresh
> key".  It seems to me that the core property for key blinding should
> distinguish itself from that scenario in some way.  You have maybe one
> sentence more detail in Section 1 ("In some applications, it's useful
> for a signer to produce digital signatures using the same long-term
> private signing key", so you're insisting on the same long-term signing
> key), but that still admits the trivial construction of skR = KDF(skS,
> counter), pkR = skR*G, where skS is the long-term key.  It doesn't seem
> like this construction would satisfy what you need.  So what property
> distinguishes what you're looking for?  The clue appears to be in the
> paragraph after: "producing a public verification key which is a
> function of the long-term public verification key and same blinding
> key".  So this property, that the public verification key needs to be
> producible from the master public key and the blinding key, appears to
> be vital, and should be highlighted up front, perhaps explicitly
> distinguishing the two more trivial cases above?

Great suggestion! Indeed, as you observe, the two constructions you propose don't suffice. We did struggle concisely describing the desired property, and this comment certainly helps make the situation more clear.

> 
> As noted in Ted's talk the other day, UnblindPublicKey is actually
> optional (some key blinding systems intentionally don't provide a way to
> unblind), and indeed you cite Tor's onion services protocol as a
> motivating example, which is one such.  It would be good to be explicit
> about this, since this mode is actually useful (and used in practice
> already).  Note that this is not the same as the question of whether the
> blinding key remains secret.  For example, you can't unblind
> pkS = H(bk, pkR, timestamp) * pkR whether or not you know bk, but that
> still seems like a useful construction.

There's an open issue for this right now, and we'll use that issue to track UnblindPublicKey being optional:

   https://github.com/chris-wood/draft-dew-cfrg-signature-key-blinding/issues/30

> 
> You cite [TORDIRECTORY] for "the Tor onion services protocol", but it
> should be [TORONION]
> https://gitweb.torproject.org/torspec.git/tree/rend-spec-v3.txt instead.
> 
> "two separate key signing keys": you don't have key signing keys;
> presumably you meant "two independent signing keys", or something like
> that?
> 
> At the start of 3, now you _insist_ that bk is secret: "Similar to the
> signing key, the blinding key is also a private key that remains
> secret."  But then what of the critical property above that
> distinguishes key blinding from "generate a fresh key"?  If no one other
> than the key holder can blind or unblind public keys on their own, so
> that the key holder has to actually deliver the new public key each
> time, then why _not_ use one of the two trivial constructions above?

Thanks for pointing this out. This text needs to be updated to drop the claim that bk remains secret. Indeed, it's necessary to invoke the UnblindPublicKey operation. 

> 
> Nit: "a random scalar in the P-256 group": in my mind, "the P-256 group"
> is the group of elliptic curve points, not the scalar field.  But
> hopefully no one will get confused by this wording.
> 
> It's a little jarring that the list of four new functionalities is
> repeated in Section 3, just a page or so after it first appears in
> Section 1.
> 
> Again in 3, the correctness condition requires the existence of
> UnblindPublicKey, which may not exist.
> 
> Aren't you missing another correctness condition at the end of Section
> 3?  Namely, that public keys produced with BlindPublicKey will correctly
> verify signatures produced with BlindKeySign (using the standard
> signature verification function)?
> 
> In Section 4.1, "Interpret the lower 32 bytes buffer" isn't clear to me.
> What is the "lower 32 bytes buffer" of a 64-byte buffer?  The first 32
> bytes?  And you just throw away the last 32 bytes?  (You're using
> SHA-512 here because it's already needed for Ed25519, right?)

It's the first 32 bytes, yeah, though we should clarify. And yep, using SHA-512 due to RFC8032.

> Explicitly skipping the buffer pruning step: this could lead to
> complications.  At least the last time I looked at the PyNaCl library,
> for example, there was _no way_ to construct a PrivateKey object without
> the library doing the buffer pruning.

Indeed, but not skipping the pruning step leaks information about the underlying key. Ted discovered this bug in an earlier version of the draft. Rather than try to argue that this leakage can't be exploited, we deemed it simpler to simply skip the pruning step altogether. Libraries that implement this routine (and can skip the pruning step) can reasonably do this as needed, whereas implementations that reuse existing libraries may have a harder time. 

> 
> In Section 4.2, in which of steps 1 and 2 do you or don't you do buffer
> pruning?  It sounds like you do in step 1 but not in step 2?

Yep, that's right. We'll make this more clear. 

> In Section 4.2, step 3, again not all Ed25519 libraries expose scalar
> multiplication like this, particularly unpruned.
> 
> In Section 5.2, it seems more explicit that step 1 and step 2 both *do*
> do buffer pruning.  (Except that contradicts Section 5.1, which suggests
> it does *not*?)  Is Ed448 intended to be different from Ed25519 here?

No, this is laziness on our part. They should use the same sort of language here.

> 
> In Section 6.1, UnblindPublicKey should take an argument of pkR, not pk.

Fixed!

> 
> I did not check the test vectors.

Thanks again for the review. 

Best,
Chris