Provides get/set for affine coordinates on OpenSSL::PKey::EC::Point #435

rickmark · 2021-04-04T00:55:21Z

This implements #433

Provides OpenSSL::PKey::EC::Point#affine_coords and OpenSSL::PKey::EC::Point#affine_coords= for uncompressed coords taking either an Integer or a OpenSSL::BN for set and always returns an array of OpenSSL::BN for get.

It also implements OpenSSL::PKey::EC::Point#set_compressed_coords taking an Integer or a OpenSSL::BN for the value of X and an Integer that is directly passed to EC_POINT_set_compressed_coordinates. I feel like it should be checked for [0..1] but was simpler to pass the value directly.

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?

rhenium · 2021-04-04T13:24:43Z

ext/openssl/ossl_bn.c

+        return Qtrue;
+    }
+
+    rb_raise(eBNError, "ossl_bn_is_odd didn't return a boolean");


We know this is unreachable.

Just defensive coding 🙃

rhenium · 2021-04-04T13:24:52Z

ext/openssl/ossl_bn.c

@@ -1217,14 +1248,15 @@ Init_ossl_bn(void)

    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);


BN implements #initialize_copy. The #dup method defined by Object should already work without this.

Will resolve with a comment to that effect so others know as well

Should we alias +@ to #dup then?

I think the current state (having #+@ as a normal method defined by ossl_bn_uplus()) is fine.

ext/openssl/ossl_pkey_ec.c

rhenium · 2021-04-04T13:33:41Z

ext/openssl/ossl_pkey_ec.c

+    y = ossl_try_convert_to_bn(RARRAY_AREF(coords, 1));
+
+    xBN = GetBNPtr(x);
+    yBN = GetBNPtr(y);


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.

rhenium · 2021-04-04T13:34:48Z

ext/openssl/ossl_pkey_ec.c

+
+    if (point == NULL || group == NULL) {
+        rb_raise(eEC_POINT, "unable to get point and group");
+    }


point and group are never NULL. GetECPoint() and GetECPointGroup() must be checking it.

rhenium · 2021-04-04T13:36:40Z

ext/openssl/ossl_pkey_ec.c

+        y = NUM2INT(y_bit);
+    } else {
+        rb_raise(eEC_POINT, "y_bit must be Integer");
+    }


No need to explicitly check here, NUM2INT() will also do a type check (and calls #to_int if necessary).

rhenium · 2021-04-04T13:40:45Z

ext/openssl/extconf.rb

+# 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")


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.

"alternate implementation" as in using the GFp and GF2m functions, or my nonsense encoding kludge?

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]>

rhenium · 2021-04-15T06:07:51Z

ext/openssl/ossl_bn.c

@@ -1217,14 +1248,15 @@ Init_ossl_bn(void)

    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);


I think the current state (having #+@ as a normal method defined by ossl_bn_uplus()) is fine.

rhenium · 2021-04-15T06:08:12Z

ext/openssl/ossl_pkey_ec.c

-static ID ID_uncompressed;
-static ID ID_compressed;
-static ID ID_hybrid;
+static ID id_odd, id_even;


These are not used.

rickmark · 2021-04-15T06:13:39Z

“Polyfill” patch is in progress to ensure that we can call get affine for either coordinate space Get Outlook for iOS<https://aka.ms/o0ukef>

________________________________ From: Kazuki Yamaguchi ***@***.***> Sent: Wednesday, April 14, 2021 11:04:34 PM To: ruby/openssl ***@***.***> Cc: Rick Mark ***@***.***>; Author ***@***.***> Subject: Re: [ruby/openssl] Provides get/set for affine coordinates on OpenSSL::PKey::EC::Point (#435) @rhenium commented on this pull request.

________________________________ In ext/openssl/extconf.rb<https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fruby%2Fopenssl%2Fpull%2F435%23discussion_r613778945&data=04%7C01%7C%7C21d0cb1270304c8e032b08d8ffd457d8%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C637540634760103083%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=%2BbwZYVL9%2Bg0JezAXaYrkBMBW8l7wHxJPrWMTcc8lbJ4%3D&reserved=0>:

@@ -174,6 +174,11 @@ def find_openssl_library

have_func("EVP_PBE_scrypt") have_func("SSL_CTX_set_post_handshake_auth") +# 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") I meant using the GFp and GF2m functions. They should both exist in OpenSSL <= 1.1.0 and LibreSSL. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub<https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fruby%2Fopenssl%2Fpull%2F435%23discussion_r613778945&data=04%7C01%7C%7C21d0cb1270304c8e032b08d8ffd457d8%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C637540634760113084%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=bRGYD94EAxoL6VjYe0fs31AZ7tionI%2BOJ0nrL2tDgH8%3D&reserved=0>, or unsubscribe<https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FAAA6TW7SPTOCSKT6OWLSBHLTIZ6XFANCNFSM42KZ7OXQ&data=04%7C01%7C%7C21d0cb1270304c8e032b08d8ffd457d8%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C637540634760123078%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=FjXPqx%2FkB8PHEyNqJKWfKYxSkd%2BPclgZpDMewpwVqzY%3D&reserved=0>.

Rick Mark added 10 commits April 3, 2021 14:03

Normalize symbol names and affine check

2f73ffb

Use `id_` for symbols Check OpenSSL for get/set affine coordinate functions

OpenSSL::PKey::EC::Point#dup

50cfdcc

Able to duplicate points

OpenSSL::PKey::EC::Point#affine_coords

0899f13

Gets affine coords from a point (array of two OpenSSL::BN, x then y uncompressed)

Expose OpenSSL::BN coercion

f7e6f64

OpenSSL::PKey::EC::Point#affine_coords=

ad43592

Implemetation of setting affine coordinates

Ensure no symbol conflict ossl_try_convert_to_bn

e40ad97

Renamed with prefix to prevent potential symbol conflicts

OpenSSL::BN#even?

22556fe

Number like duck types expect an even? where there is an odd?

OpenSSL::PKey::EC::Point#set_compressed_coords

86b9ced

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

OpenSSL::BN#dup

b0eb019

Same as unary plus

OpenSSL::BN#positive?

1a34823

Another expectation of Integer like objects that define negative?

rhenium reviewed Apr 4, 2021

View reviewed changes

Fixed array initialize

872fc73

Co-authored-by: Kazuki Yamaguchi <[email protected]>

rhenium requested changes Apr 15, 2021

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Provides get/set for affine coordinates on OpenSSL::PKey::EC::Point #435

Provides get/set for affine coordinates on OpenSSL::PKey::EC::Point #435

rickmark commented Apr 4, 2021

rhenium Apr 4, 2021

rickmark Apr 7, 2021

rhenium Apr 4, 2021

rickmark Apr 7, 2021

rickmark Apr 13, 2021

rhenium Apr 15, 2021

rhenium Apr 4, 2021

rhenium Apr 4, 2021

rhenium Apr 4, 2021

rhenium Apr 4, 2021

rickmark Apr 7, 2021

rhenium Apr 15, 2021

rhenium Apr 15, 2021

rhenium Apr 15, 2021

rickmark commented Apr 15, 2021 via email

Provides get/set for affine coordinates on OpenSSL::PKey::EC::Point #435

Are you sure you want to change the base?

Provides get/set for affine coordinates on OpenSSL::PKey::EC::Point #435

Conversation

rickmark commented Apr 4, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rickmark commented Apr 15, 2021 via email