Handle identity point in GroupEncoding
#444
Conversation
daxpedda
changed the title
GroupEncoding
Codecov Report
@@ Coverage Diff @@
## master #444 +/- ##
==========================================
+ Coverage 64.51% 65.39% +0.88%
==========================================
Files 28 28
Lines 3590 3624 +34
==========================================
+ Hits 2316 2370 +54
+ Misses 1274 1254 -20
Continue to review full report at Codecov.
|
| .or_else(|| { | ||
| CtOption::new( | ||
| AffinePoint::identity(), | ||
| tag.ct_eq(&sec1::Tag::Identity.into()), | ||
| ) | ||
| }) |
There was a problem hiding this comment.
This seemed to me to be the cleanest solution, but not entirely correct because it doesn't check if the rest of the bytes are also 0s.
I think the best approach if this is desired is to change DecompressPoint::decompress() to handle the identity point, which would require a breaking change.
Just realized what I said here is nonsense, I guess we could do the check right there.
There was a problem hiding this comment.
This would accept a 0-tagged value with garbage bytes. Not sure that's a good idea. It should probably check all of them.
This is a lot of logic to repeat on a per-crate basis. It seems like it might be worth hoisting into the elliptic-curve crate in some form. It could probably be composed in terms of FromDecodedPoint.
There was a problem hiding this comment.
I'm unsure what you mean, FromDecodedPoint::from_encoded_point() requires an EncodedPoint and doesn't do the check we want. Do you mean adding a new method to FromDecodedPoint?
There was a problem hiding this comment.
No, I mean something which uses FromDecodedPoint to do the decoding, possibly after changing its result to a CtOption, and offloading the SEC1 tag handling there
There was a problem hiding this comment.
I just realized, I thought you meant FromEncodedPoint, I can't find FromDecodedPoint. Did you mean creating a new trait called FromDecodedPoint?
There was a problem hiding this comment.
Err yes sorry (responding on my phone here)
`GroupEncoding` uses a fixed-width `Repr` whereas SEC1 is variable-width with respect to compressed points versus the identity. This change allows 33-bytes of zeroes to be used as the identity. Technically that's not a valid SEC1 encoding: the SEC1 encoding for the identity is 1-byte: 0x00. However, this is the best we can do with a fixed-width encoding. See also: #443 and #444
`GroupEncoding` uses a fixed-width `Repr` whereas SEC1 is variable-width with respect to compressed points versus the identity. This change allows 33-bytes of zeroes to be used as the identity. Technically that's not a valid SEC1 encoding: the SEC1 encoding for the identity is 1-byte: 0x00. However, this is the best we can do with a fixed-width encoding. See also: #443 and #444
|
Closing in favor of #446 |
3 participants
This enabled
GroupEncoding::to_bytes()andGroupEncoding::from_bytes()forAffinePointandProjectivePointfor p256 and k256 to handle the identity point.See #443.