Provides get/set for affine coordinates on OpenSSL::PKey::EC::Point#435
Provides get/set for affine coordinates on OpenSSL::PKey::EC::Point#435rickmark wants to merge 11 commits intoruby:masterfrom
Conversation
Use `id_` for symbols Check OpenSSL for get/set affine coordinate functions
Able to duplicate points
Gets affine coords from a point (array of two OpenSSL::BN, x then y uncompressed)
Implemetation of setting affine coordinates
Renamed with prefix to prevent potential symbol conflicts
Number like duck types expect an even? where there is an odd?
Allows setting a OpenSSL::BN for X and the Y bit indicating if 0 if the point Y is even and 1 if the point Y is odd
Same as unary plus
Another expectation of Integer like objects that define negative?
| return Qtrue; | ||
| } | ||
|
|
||
| rb_raise(eBNError, "ossl_bn_is_odd didn't return a boolean"); |
There was a problem hiding this comment.
Just defensive coding 🙃
|
|
||
| rb_define_method(cBN, "initialize_copy", ossl_bn_copy, 1); | ||
| rb_define_method(cBN, "copy", ossl_bn_copy, 1); | ||
| rb_define_method(cBN, "dup", ossl_bn_dup, 0); |
There was a problem hiding this comment.
BN implements #initialize_copy. The #dup method defined by Object should already work without this.
There was a problem hiding this comment.
Will resolve with a comment to that effect so others know as well
There was a problem hiding this comment.
Should we alias +@ to #dup then?
There was a problem hiding this comment.
I think the current state (having #+@ as a normal method defined by ossl_bn_uplus()) is fine.
| y = ossl_try_convert_to_bn(RARRAY_AREF(coords, 1)); | ||
|
|
||
| xBN = GetBNPtr(x); | ||
| yBN = GetBNPtr(y); |
There was a problem hiding this comment.
GetBNPtr() uses try_convert_to_bn() internally. So, simply x = RARRAY_AREF(coords, 0); followed by xBN = GetBNPtr(x) should be enough.
The value from the array still needs to be stored into a separate VALUE variable (x, y) once, though.
|
|
||
| if (point == NULL || group == NULL) { | ||
| rb_raise(eEC_POINT, "unable to get point and group"); | ||
| } |
There was a problem hiding this comment.
point and group are never NULL. GetECPoint() and GetECPointGroup() must be checking it.
| y = NUM2INT(y_bit); | ||
| } else { | ||
| rb_raise(eEC_POINT, "y_bit must be Integer"); | ||
| } |
There was a problem hiding this comment.
No need to explicitly check here, NUM2INT() will also do a type check (and calls #to_int if necessary).
| # added in 1.1.1 | ||
| have_func("EC_POINT_get_affine_coordinates") | ||
| have_func("EC_POINT_set_affine_coordinates") | ||
| have_func("EC_POINT_set_compressed_coordinates") |
There was a problem hiding this comment.
Can we have an alternate implementation in ext/openssl/openssl_missing.c using the suffixed variants? EC_POINT_get_affine_coordinates() only exists in OpenSSL 1.1.1, and the latest LibreSSL still doesn't have it.
Checking only EC_POINT_get_affine_coordinates should be good enough since it's unlikely a future LibreSSL version implements just one or two of the three functions.
There was a problem hiding this comment.
"alternate implementation" as in using the GFp and GF2m functions, or my nonsense encoding kludge?
There was a problem hiding this comment.
I meant using the GFp and GF2m functions. They should both exist in OpenSSL <= 1.1.0 and LibreSSL.
Co-authored-by: Kazuki Yamaguchi <[email protected]>
|
|
||
| rb_define_method(cBN, "initialize_copy", ossl_bn_copy, 1); | ||
| rb_define_method(cBN, "copy", ossl_bn_copy, 1); | ||
| rb_define_method(cBN, "dup", ossl_bn_dup, 0); |
There was a problem hiding this comment.
I think the current state (having #+@ as a normal method defined by ossl_bn_uplus()) is fine.
| static ID ID_uncompressed; | ||
| static ID ID_compressed; | ||
| static ID ID_hybrid; | ||
| static ID id_odd, id_even; |
2 participants
This implements #433
Provides
OpenSSL::PKey::EC::Point#affine_coordsandOpenSSL::PKey::EC::Point#affine_coords=for uncompressed coords taking either anIntegeror aOpenSSL::BNfor set and always returns an array ofOpenSSL::BNfor get.It also implements
OpenSSL::PKey::EC::Point#set_compressed_coordstaking anIntegeror aOpenSSL::BNfor the value of X and anIntegerthat is directly passed toEC_POINT_set_compressed_coordinates. I feel like it should be checked for[0..1]but was simpler to pass the value directly.