Open BjoernMHaase opened 4 years ago
It is also necessary to fix the issue that "steve tobtu's" correct name is actually steve thomas.
I received an an additional off-the-list feedback from Frank Denis:
"Hi Björn,
There hasn't been any formal release of that code, so it can totally be changed as well as the specification.
The I-D is excellent, and contains everything an implementer would need.
Just some minor remarks:
"Cipher suites for modern Montgomery or Edwards curves are recommended to be specified in line with the definitions for Curve25519".
Not exactly. Points on Edwards curves usually represented as their Y coordinate. And the Montgomery ladder doesn't work here, so stripping the bit sign may not be required.
Maybe adding a few lines about Edwards curves, possibly with edwards255 being included in the ciphersuite, as well as relevant test vectors, could be useful.
DSI1 & DS2
DSI1 and DSI2 use Cpace25519- for X25519 and CPace-P256-.
This is not a consistent naming scheme. Maybe use Cpace-X25519- instead of Cpace25519-? That would also leave room for Cpace-Edwards25519-.
CI data examples: "One first example of CI data is an encoding of the concatenation of IP addresses and port numbers of both parties"
Source port numbers change for every datagram, and for TCP, the destination port number is often fixed (and, today, mostly only 443). Applications may not even have access to source port numbers. I'm not sure that port numbers are a good example of data to include."
In response to a correct remark of Filippo Valsorda, the way how party identity strings contribute to the calculation of the generator should be modified.
Currently it is specified that CI = A || B || AD.
Most likely, one should be modifying this definition such that the length of the strings is prepended.
CI = I2OSP(len(A), 1) || A || I2OSP(len(B), 1) || B || I2OSP(len(AD), 1) || AD
In case that any one of A, B or AD gets longer than 255 bytes, one would be using an approach similar to the H2C draft, such that any one of an oversized A, B and AD is replaced by
A = H("CPACE-OVERSIZE-A-" || a_very_long_A) B = H("CPACE-OVERSIZE-B-" || a_very_long_B) AD = H("CPACE-OVERSIZE-AD-" || a_very_long_AD) .
Link to Frank Denis' feedback
Link to Filippo Valsorda's GO implementation
https://github.com/FiloSottile/go-cpace-ristretto255/issues/1
Two of the above issues are fixed in the first draft-irtf-cfrg-cpace-00 version of the I-D.
"It is also necessary to fix the issue that "steve tobtu's" correct name is actually steve thomas."
"The section about iterative hash function "IHF" in the CPace draft (https://tools.ietf.org/html/draft-haase-cpace-01) is orphan documentation and should be removed."
The feedbacks 1.) and 2.) below have been incorporated into the recent update of the AuCPace-02 draft. 1.) "It is also necessary to fix the issue that "steve tobtu's" correct name is actually steve thomas."
2.) "The function "IHF" is used inconsistently in the AuCPace draft (https://tools.ietf.org/html/draft-haase-aucpace-01). Sometimes omitting sigma (the work parameters) and in different order than defined.
It's defined as: IHF(salt, username, pw, sigma)
but used like: IHF(username,pw,salt) IHF(pwd,username,salt) IHF(salt, username, password) IHF(sigma,username,password, salt)"
The section about iterative hash function "IHF" in the CPace draft (https://tools.ietf.org/html/draft-haase-cpace-01) is orphan documentation and should be removed.
The function "IHF" is used inconsistently in the AuCPace draft (https://tools.ietf.org/html/draft-haase-aucpace-01). Sometimes omitting sigma (the work parameters) and in different order than defined.
It's defined as: IHF(salt, username, pw, sigma)
but used like: IHF(username,pw,salt) IHF(pwd,username,salt) IHF(salt, username, password) IHF(sigma,username,password, salt)