Merge pull request #177 from rhenium/ky/pkey-ec-point-octet-string

 pkey/ec: add support for octet string encoding of EC point

3 files changed, 131 insertions, 101 deletions

diff --git a/ext/openssl/ossl_pkey_ec.c b/ext/openssl/ossl_pkey_ec.c
index 9c406931..fbc71a6f 100644
--- a/ext/openssl/ossl_pkey_ec.c
+++ b/ext/openssl/ossl_pkey_ec.c
@@ -1319,76 +1319,61 @@ ec_point_new(const EC_POINT *point, const EC_GROUP *group)
     return obj;
 }
 
+static VALUE ossl_ec_point_initialize_copy(VALUE, VALUE);
 /*
  * call-seq:
  *   OpenSSL::PKey::EC::Point.new(point)
- *   OpenSSL::PKey::EC::Point.new(group)
- *   OpenSSL::PKey::EC::Point.new(group, bn)
+ *   OpenSSL::PKey::EC::Point.new(group [, encoded_point])
  *
- * See the OpenSSL documentation for EC_POINT_*
+ * Creates a new instance of OpenSSL::PKey::EC::Point. If the only argument is
+ * an instance of EC::Point, a copy is returned. Otherwise, creates a point
+ * that belongs to _group_.
+ *
+ * _encoded_point_ is the octet string representation of the point. This
+ * must be either a String or an OpenSSL::BN.
  */
 static VALUE ossl_ec_point_initialize(int argc, VALUE *argv, VALUE self)
 {
     EC_POINT *point;
-    VALUE arg1, arg2;
-    VALUE group_v = Qnil;
-    const EC_GROUP *group = NULL;
+    VALUE group_v, arg2;
+    const EC_GROUP *group;
 
     TypedData_Get_Struct(self, EC_POINT, &ossl_ec_point_type, point);
     if (point)
-        ossl_raise(eEC_POINT, "EC_POINT already initialized");
-
-    switch (rb_scan_args(argc, argv, "11", &arg1, &arg2)) {
-    case 1:
-        if (rb_obj_is_kind_of(arg1, cEC_POINT)) {
-            const EC_POINT *arg_point;
-
-	    group_v = rb_attr_get(arg1, id_i_group);
-	    GetECGroup(group_v, group);
-	    GetECPoint(arg1, arg_point);
-
-            point = EC_POINT_dup(arg_point, group);
-        } else if (rb_obj_is_kind_of(arg1, cEC_GROUP)) {
-            group_v = arg1;
-            GetECGroup(group_v, group);
-
-            point = EC_POINT_new(group);
-        } else {
-            ossl_raise(eEC_POINT, "wrong argument type: must be OpenSSL::PKey::EC::Point or OpenSSL::Pkey::EC::Group");
-        }
+	rb_raise(eEC_POINT, "EC_POINT already initialized");
 
-        break;
-     case 2:
-        if (!rb_obj_is_kind_of(arg1, cEC_GROUP))
-            ossl_raise(rb_eArgError, "1st argument must be OpenSSL::PKey::EC::Group");
-        group_v = arg1;
-        GetECGroup(group_v, group);
-
-        if (rb_obj_is_kind_of(arg2, cBN)) {
-            const BIGNUM *bn = GetBNPtr(arg2);
-
-            point = EC_POINT_bn2point(group, bn, NULL, ossl_bn_ctx);
-        } else {
-            BIO *in = ossl_obj2bio(&arg1);
-
-/* BUG: finish me */
-
-            BIO_free(in);
-
-            if (point == NULL) {
-                ossl_raise(eEC_POINT, "unknown type for 2nd arg");
-            }
-        }
-        break;
-    default:
-        ossl_raise(rb_eArgError, "wrong number of arguments");
+    rb_scan_args(argc, argv, "11", &group_v, &arg2);
+    if (rb_obj_is_kind_of(group_v, cEC_POINT)) {
+	if (argc != 1)
+	    rb_raise(rb_eArgError, "invalid second argument");
+	return ossl_ec_point_initialize_copy(self, group_v);
     }
 
-    if (point == NULL)
-        ossl_raise(eEC_POINT, NULL);
-
-    if (NIL_P(group_v))
-        ossl_raise(rb_eRuntimeError, "missing group (internal error)");
+    GetECGroup(group_v, group);
+    if (argc == 1) {
+	point = EC_POINT_new(group);
+	if (!point)
+	    ossl_raise(eEC_POINT, "EC_POINT_new");
+    }
+    else {
+	if (rb_obj_is_kind_of(arg2, cBN)) {
+	    point = EC_POINT_bn2point(group, GetBNPtr(arg2), NULL, ossl_bn_ctx);
+	    if (!point)
+		ossl_raise(eEC_POINT, "EC_POINT_bn2point");
+	}
+	else {
+	    StringValue(arg2);
+	    point = EC_POINT_new(group);
+	    if (!point)
+		ossl_raise(eEC_POINT, "EC_POINT_new");
+	    if (!EC_POINT_oct2point(group, point,
+				    (unsigned char *)RSTRING_PTR(arg2),
+				    RSTRING_LEN(arg2), ossl_bn_ctx)) {
+		EC_POINT_free(point);
+		ossl_raise(eEC_POINT, "EC_POINT_oct2point");
+	    }
+	}
+    }
 
     RTYPEDDATA_DATA(self) = point;
     rb_ivar_set(self, id_i_group, group_v);
@@ -1543,38 +1528,38 @@ static VALUE ossl_ec_point_set_to_infinity(VALUE self)
 
 /*
  * call-seq:
- *   point.to_bn(conversion_form = nil) => OpenSSL::BN
+ *    point.to_octet_string(conversion_form) -> String
  *
- * Convert the EC point into an octet string and store in an OpenSSL::BN. If
- * _conversion_form_ is given, the point data is converted using the specified
- * form. If not given, the default form set in the EC::Group object is used.
+ * Returns the octet string representation of the elliptic curve point.
  *
- * See also EC::Point#point_conversion_form=.
+ * _conversion_form_ specifies how the point is converted. Possible values are:
+ *
+ * - +:compressed+
+ * - +:uncompressed+
+ * - +:hybrid+
  */
 static VALUE
-ossl_ec_point_to_bn(int argc, VALUE *argv, VALUE self)
+ossl_ec_point_to_octet_string(VALUE self, VALUE conversion_form)
 {
     EC_POINT *point;
-    VALUE form_obj, bn_obj;
     const EC_GROUP *group;
     point_conversion_form_t form;
-    BIGNUM *bn;
+    VALUE str;
+    size_t len;
 
     GetECPoint(self, point);
     GetECPointGroup(self, group);
-    rb_scan_args(argc, argv, "01", &form_obj);
-    if (NIL_P(form_obj))
-	form = EC_GROUP_get_point_conversion_form(group);
-    else
-	form = parse_point_conversion_form_symbol(form_obj);
-
-    bn_obj = rb_obj_alloc(cBN);
-    bn = GetBNPtr(bn_obj);
-
-    if (EC_POINT_point2bn(group, point, form, bn, ossl_bn_ctx) == NULL)
-        ossl_raise(eEC_POINT, "EC_POINT_point2bn");
-
-    return bn_obj;
+    form = parse_point_conversion_form_symbol(conversion_form);
+
+    len = EC_POINT_point2oct(group, point, form, NULL, 0, ossl_bn_ctx);
+    if (!len)
+	ossl_raise(eEC_POINT, "EC_POINT_point2oct");
+    str = rb_str_new(NULL, (long)len);
+    if (!EC_POINT_point2oct(group, point, form,
+			    (unsigned char *)RSTRING_PTR(str), len,
+			    ossl_bn_ctx))
+	ossl_raise(eEC_POINT, "EC_POINT_point2oct");
+    return str;
 }
 
 /*
@@ -1799,7 +1784,7 @@ void Init_ossl_ec(void)
     rb_define_method(cEC_POINT, "set_to_infinity!", ossl_ec_point_set_to_infinity, 0);
 /* all the other methods */
 
-    rb_define_method(cEC_POINT, "to_bn", ossl_ec_point_to_bn, -1);
+    rb_define_method(cEC_POINT, "to_octet_string", ossl_ec_point_to_octet_string, 1);
     rb_define_method(cEC_POINT, "mul", ossl_ec_point_mul, -1);
 
     id_i_group = rb_intern("@group");
diff --git a/lib/openssl/pkey.rb b/lib/openssl/pkey.rb
index dcedd849..8a547c34 100644
--- a/lib/openssl/pkey.rb
+++ b/lib/openssl/pkey.rb
@@ -1,3 +1,25 @@
 # frozen_string_literal: false
-module OpenSSL
+#--
+# Ruby/OpenSSL Project
+# Copyright (C) 2017 Ruby/OpenSSL Project Authors
+#++
+
+module OpenSSL::PKey
+  if defined?(EC)
+  class EC::Point
+    # :call-seq:
+    #    point.to_bn([conversion_form]) -> OpenSSL::BN
+    #
+    # Returns the octet string representation of the EC point as an instance of
+    # OpenSSL::BN.
+    #
+    # If _conversion_form_ is not given, the _point_conversion_form_ attribute
+    # set to the group is used.
+    #
+    # See #to_octet_string for more information.
+    def to_bn(conversion_form = group.point_conversion_form)
+      OpenSSL::BN.new(to_octet_string(conversion_form), 2)
+    end
+  end
+  end
 end
diff --git a/test/test_pkey_ec.rb b/test/test_pkey_ec.rb
index 713c649a..dc5a14e6 100644
--- a/test/test_pkey_ec.rb
+++ b/test/test_pkey_ec.rb
@@ -120,14 +120,9 @@ class OpenSSL::TestEC < OpenSSL::PKeyTestCase
     asn1 = OpenSSL::ASN1::Sequence([
       OpenSSL::ASN1::Integer(1),
       OpenSSL::ASN1::OctetString(p256.private_key.to_s(2)),
-      OpenSSL::ASN1::ASN1Data.new(
-        [OpenSSL::ASN1::ObjectId("prime256v1")],
-        0, :CONTEXT_SPECIFIC
-      ),
-      OpenSSL::ASN1::ASN1Data.new(
-        [OpenSSL::ASN1::BitString(p256.public_key.to_bn.to_s(2))],
-        1, :CONTEXT_SPECIFIC
-      )
+      OpenSSL::ASN1::ObjectId("prime256v1", 0, :EXPLICIT),
+      OpenSSL::ASN1::BitString(p256.public_key.to_octet_string(:uncompressed),
+                               1, :EXPLICIT)
     ])
     key = OpenSSL::PKey::EC.new(asn1.to_der)
     assert_predicate key, :private?
@@ -181,7 +176,7 @@ class OpenSSL::TestEC < OpenSSL::PKeyTestCase
         OpenSSL::ASN1::ObjectId("prime256v1")
       ]),
       OpenSSL::ASN1::BitString(
-        p256.public_key.to_bn.to_s(2)
+        p256.public_key.to_octet_string(:uncompressed)
       )
     ])
     key = OpenSSL::PKey::EC.new(asn1.to_der)
@@ -224,7 +219,8 @@ class OpenSSL::TestEC < OpenSSL::PKeyTestCase
     assert_equal :uncompressed, group4.point_conversion_form
     assert_equal group1, group4
     assert_equal group1.curve_name, group4.curve_name
-    assert_equal group1.generator.to_bn, group4.generator.to_bn
+    assert_equal group1.generator.to_octet_string(:uncompressed),
+      group4.generator.to_octet_string(:uncompressed)
     assert_equal group1.order, group4.order
     assert_equal group1.cofactor, group4.cofactor
     assert_equal group1.seed, group4.seed
@@ -239,34 +235,57 @@ class OpenSSL::TestEC < OpenSSL::PKeyTestCase
     point2 = OpenSSL::PKey::EC::Point.new(group, point.to_bn)
     assert_equal point, point2
     assert_equal point.to_bn, point2.to_bn
+    assert_equal point.to_octet_string(:uncompressed),
+      point2.to_octet_string(:uncompressed)
+
+    point3 = OpenSSL::PKey::EC::Point.new(group,
+                                          point.to_octet_string(:uncompressed))
+    assert_equal point, point3
+    assert_equal point.to_bn, point3.to_bn
+    assert_equal point.to_octet_string(:uncompressed),
+      point3.to_octet_string(:uncompressed)
+
     point2.invert!
-    assert_not_equal point.to_bn, point2.to_bn
+    point3.invert!
+    assert_not_equal point.to_octet_string(:uncompressed),
+      point2.to_octet_string(:uncompressed)
+    assert_equal point2.to_octet_string(:uncompressed),
+      point3.to_octet_string(:uncompressed)
 
     begin
       group = OpenSSL::PKey::EC::Group.new(:GFp, 17, 2, 2)
       group.point_conversion_form = :uncompressed
-      generator = OpenSSL::PKey::EC::Point.new(group, 0x040501.to_bn)
+      generator = OpenSSL::PKey::EC::Point.new(group, B(%w{ 04 05 01 }))
       group.set_generator(generator, 19, 1)
-      point = OpenSSL::PKey::EC::Point.new(group, 0x040603.to_bn)
+      point = OpenSSL::PKey::EC::Point.new(group, B(%w{ 04 06 03 }))
     rescue OpenSSL::PKey::EC::Group::Error
       pend "Patched OpenSSL rejected curve" if /unsupported field/ =~ $!.message
       raise
     end
 
+    assert_equal 0x040603.to_bn, point.to_bn
     assert_equal 0x040603.to_bn, point.to_bn(:uncompressed)
     assert_equal 0x0306.to_bn, point.to_bn(:compressed)
     assert_equal 0x070603.to_bn, point.to_bn(:hybrid)
 
-    assert_equal 0x040603.to_bn, point.to_bn
+    group2 = group.dup; group2.point_conversion_form = :compressed
+    point2 = OpenSSL::PKey::EC::Point.new(group2, B(%w{ 04 06 03 }))
+    assert_equal 0x0306.to_bn, point2.to_bn
+
+    assert_equal B(%w{ 04 06 03 }), point.to_octet_string(:uncompressed)
+    assert_equal B(%w{ 03 06 }), point.to_octet_string(:compressed)
+    assert_equal B(%w{ 07 06 03 }), point.to_octet_string(:hybrid)
+
     assert_equal true, point.on_curve?
     point.invert! # 8.5
-    assert_equal 0x04060E.to_bn, point.to_bn
+    assert_equal B(%w{ 04 06 0E }), point.to_octet_string(:uncompressed)
     assert_equal true, point.on_curve?
 
     assert_equal false, point.infinity?
     point.set_to_infinity!
     assert_equal true, point.infinity?
     assert_equal 0.to_bn, point.to_bn
+    assert_equal B(%w{ 00 }), point.to_octet_string(:uncompressed)
     assert_equal true, point.on_curve?
   end
 
@@ -276,25 +295,25 @@ class OpenSSL::TestEC < OpenSSL::PKeyTestCase
       # generator is (5, 1)
       group = OpenSSL::PKey::EC::Group.new(:GFp, 17, 2, 2)
       group.point_conversion_form = :uncompressed
-      gen = OpenSSL::PKey::EC::Point.new(group, OpenSSL::BN.new("040501", 16))
+      gen = OpenSSL::PKey::EC::Point.new(group, B(%w{ 04 05 01 }))
       group.set_generator(gen, 19, 1)
 
       # 3 * (6, 3) = (16, 13)
-      point_a = OpenSSL::PKey::EC::Point.new(group, OpenSSL::BN.new("040603", 16))
+      point_a = OpenSSL::PKey::EC::Point.new(group, B(%w{ 04 06 03 }))
       result_a1 = point_a.mul(3)
-      assert_equal("04100D", result_a1.to_bn.to_s(16))
+      assert_equal B(%w{ 04 10 0D }), result_a1.to_octet_string(:uncompressed)
       # 3 * (6, 3) + 3 * (5, 1) = (7, 6)
       result_a2 = point_a.mul(3, 3)
-      assert_equal("040706", result_a2.to_bn.to_s(16))
+      assert_equal B(%w{ 04 07 06 }), result_a2.to_octet_string(:uncompressed)
       # 3 * point_a = 3 * (6, 3) = (16, 13)
       result_b1 = point_a.mul([3], [])
-      assert_equal("04100D", result_b1.to_bn.to_s(16))
+      assert_equal B(%w{ 04 10 0D }), result_b1.to_octet_string(:uncompressed)
       # 3 * point_a + 2 * point_a = 3 * (6, 3) + 2 * (6, 3) = (7, 11)
       result_b1 = point_a.mul([3, 2], [point_a])
-      assert_equal("04070B", result_b1.to_bn.to_s(16))
+      assert_equal B(%w{ 04 07 0B }), result_b1.to_octet_string(:uncompressed)
       # 3 * point_a + 5 * point_a.group.generator = 3 * (6, 3) + 5 * (5, 1) = (13, 10)
       result_b1 = point_a.mul([3], [], 5)
-      assert_equal("040D0A", result_b1.to_bn.to_s(16))
+      assert_equal B(%w{ 04 0D 0A }), result_b1.to_octet_string(:uncompressed)
     rescue OpenSSL::PKey::EC::Group::Error
       # CentOS patches OpenSSL to reject curves defined over Fp where p < 256 bits
       raise if $!.message !~ /unsupported field/
@@ -316,6 +335,10 @@ class OpenSSL::TestEC < OpenSSL::PKeyTestCase
 
   private
 
+  def B(ary)
+    [Array(ary).join].pack("H*")
+  end
+
   def assert_same_ec(expected, key)
     check_component(expected, key, [:group, :public_key, :private_key])
   end