cfrg / draft-irtf-cfrg-voprf

Oblivious Pseudorandom Functions (OPRFs) using Prime-Order Groups
https://cfrg.github.io/draft-irtf-cfrg-voprf/#go.draft-irtf-cfrg-voprf.html
Other
39 stars 15 forks source link

Change decaf to use shake #238

Closed claucece closed 3 years ago

chris-wood commented 3 years ago

Taking a step back, how about we specify HashToScalar following the advice in hash-to-curve like so:

ristretto255:

HashToScalar(): Use hash_to_field from {{!I-D.irtf-cfrg-hash-to-curve}} using l=<order of field> as the prime modulus, L = 48, `expand_message_xmd` with SHA-512, and DST = "VOPRF06-HashToScalar-" || contextString.

decaf448:

HashToScalar(): Use hash_to_field from {{!I-D.irtf-cfrg-hash-to-curve}} using l=<order of field> as the prime modulus, L = 84, `expand_message_xof` with SHAKE-256, and DST = "VOPRF06-HashToScalar-" || contextString.

The values of L were derived from L = ceil((ceil(log2(p)) + k) / 8), where p = <order of scalar field> and k = 128/224 (for a 128-bit and 224-bit security level, respectively). That would make each of these consistent with the other HashToScalar invocations.

@armfazh, thoughts?

bytemare commented 3 years ago

If SHA-512, it would be expand_message_xmd (not xof)

claucece commented 3 years ago

@bytemare yeah, probably @chris-wood meant shake ;)

I'm checking this further and respond to this pr ;)

chris-wood commented 3 years ago

@bytemare yeah, probably @chris-wood meant shake ;)

Ugh, yes, I did. Typo. Thanks!

bytemare commented 3 years ago

Yes, and I also meant for Ristretto255 ;)

chris-wood commented 3 years ago

Yes, and I also meant for Ristretto255 ;)

We discussed this in #200. Basically, it makes most sense to use SHA-512 and _xmd for r225.

bytemare commented 3 years ago

I know, but you wrote _xof on the line for SHA-512/r255 :)

armfazh commented 3 years ago

Taking a step back, how about we specify HashToScalar following the advice in hash-to-curve like so: That would make each of these consistent with the other HashToScalar invocations.

this seems reasonable to me, one single hashToField implementation can serve to all the instantiations, if code reuse is a goal.

claucece commented 3 years ago

@bytemare noted ;) I think we all agree on that. On the pr, it will be expand_xmd with sha512 for r255; and expand_xof with shake256 for d448.

claucece commented 3 years ago

I like the last proposal @chris-wood . I'll change the need text ;) Thanks!

claucece commented 3 years ago

@chris-wood @armfazh let me know what you think now ;)

armfazh commented 3 years ago

let me know what you think now ;)

good,

chris-wood commented 3 years ago

@alxdavids are you OK with this? If so, let's merge it!

tmthrgd commented 3 years ago

As someone coming across and implementing this spec, I'm not a fan of this change in general. (I'll try and elaborate elsewhere). But specifically as it stands this change seems really odd to me. It replaces expand_message_xmd with SHA-512 with expand_message_xof with SHAKE-256, but it doesn't change Hash (required for Finalize, etc.) from SHA-512.

Far from removing a potential dependency this just adds another. OPRF(decaf448, SHA-512) would require both SHA-512 and SHAKE-256 to implement. That's the worst of both worlds. If there is a desire to do this, surely Hash should also be swapped out for SHA3/SHAKE?

chris-wood commented 3 years ago

Far from removing a potential dependency this just adds another. OPRF(decaf448, SHA-512) would require both SHA-512 and SHAKE-256 to implement. That's the worst of both worlds. If there is a desire to do this, surely Hash should also be swapped out for SHA3/SHAKE?

Hah, good catch :-) I believe the intent was to replace all instances of SHA-2 with SHAKE for decaf. I think we all just overlooked the corresponding ciphersuite hash.

claucece commented 3 years ago

@tmthrgd indeed, it will be changed as well.

would require both SHA-512 and SHAKE-256 to implement

Generally, SHA-512 is widely implemented and in most languages that have the SHA-3 family, SHA-2 is also available.

Is there a language that has SHA-3 family but not SHA-2?

I'll change the pr to have SHAKE eveywhere.

claucece commented 3 years ago

@tmthrgd should be changed now ;)

chris-wood commented 3 years ago

@claucece any updates on this?

claucece commented 3 years ago

@chris-wood email was sent asking for the clarifications that you needed. Last time I checked, this was not a big priority prior to IETF111.. is there a reason to make this a bigger priority now? Thanks!

chris-wood commented 3 years ago

is there a reason to make this a bigger priority now? Thanks!

Nope! I'm just curious to know what the status is for this change.

chris-wood commented 3 years ago

Closing in favor of #249.