cfrg / draft-irtf-cfrg-vdaf

VDAF specification
Other
17 stars 14 forks source link

Clarify that public share requirements are relevant to robustness #342

Closed tgeoghegan closed 2 months ago

tgeoghegan commented 2 months ago

I think I'm being fussy but I want to make sure we don't slip up here. Continuing from #336:

The risk that the Poplar paper talks about when the aggregators don't have a consistent view of the public parameters is that the malicious client could control the output of the DPF at two inputs, instead of one, like we want. This happens because choosing different group element correction words for one level, and giving each aggregator a different one, gives the client an extra degree of freedom it shouldn't have.

Since we're considering a malicious client when we consider attempts to break extractability in this manner, we don't need to protect this report's privacy, and thus the only goal that matters is robustness. For robustness, we get to assume that both aggregators are honest, and thus we can assume that the leader will distribute the public share to the helper correctly, without considering the HPKE AAD.

Including the public share in the HPKE AAD is probably more relevant for protecting against privacy attacks, as we wouldn't want to let a rogue leader pass an honest client's input share with a tampered public share.

_Originally posted by @divergentdave in https://github.com/cfrg/draft-irtf-cfrg-vdaf/pull/336#discussion_r1591715132_

I think I agree with that analysis, but it's hard to square with the MUST added by #336, which reads:

The Aggregators MUST verify that they have both received the same public share from the Client. It is sufficient, for example, to exchange a hash of the public share over a secure channel.

Perhaps this paragraph should say something like "In order to protect robustness, the Aggregators MUST verify...". That would give "permission" to a higher level protocol like DAP to apply David's analysis and opt out of doing something like an explicit exchange of the public share. I also wonder if David's analysis should be incorporated into the text of DAP.

cjpatton commented 2 months ago

I don't quite see how the text we landed doesn't square with David's analysis. Is it that "the Aggregators MUST verify" sounds like there needs to be some explicit procedure, like exchanging the hash? As David explains, explicit verification is not necessary: in DAP, the Leader is supposed to copy the public share from the report into the Helper's report share. This provides the property we need, just implicitly.

Perhaps "the Aggregators MUST ensure they hold the same public share"?

Note that including the the public share in the HPKE AAD is not strictly necessary, either for privacy or robustness. We do this just for defense-in-depth.

All that said, your suggestion sounds fine -- feel free to send a PR.

divergentdave commented 2 months ago

The VDAF security model just stipulates secure channels between protocol participants, so part of what's going on is just DAP's implementation decisions regarding instantiating the secure channel from the Client to Leader and Client to Helper. Splitting the report, encrypting part of it, and committing to the other part is complicated, but it saves some communication cost by avoiding duplicate data. (I think DAP does need to include the public share in the HPKE AAD in order to maintain integrity of the whole report while tunneling through the maybe-malicious Leader.)

I don't think we should say anything about the relationship between robustness and the Aggregators having the same public share, because that could then become a new requirement on VDAFs, depending on how it's phrased. Rephrasing it to make it clear an extra explicit check isn't necessary sounds good.

tgeoghegan commented 2 months ago

Is it that "the Aggregators MUST verify" sounds like there needs to be some explicit procedure, like exchanging the hash?

Yeah, that's what concerns me. Saying "MUST ensure" sounds like a good way to resolve this.

in DAP, the Leader is supposed to copy the public share from the report into the Helper's report share. This provides the property we need, just implicitly.

Oh, this is the part I was missing: the honest leader does get to enforce that the public shares match.

With all this understood, I'd be comfortable with changing "MUST verify" to "MUST ensure", but I'd also be OK with doing nothing and closing this without any change.

cjpatton commented 2 months ago

PR up: @tgeoghegan please take a look!