Merge branch 'maint-3.0'

rhenium · rhenium · commit 7f5b5adcc57f · 2022-09-08T20:50:19.000+09:00
* maint-3.0:
  Ruby/OpenSSL 3.0.1
  Ruby/OpenSSL 2.2.2
  Ruby/OpenSSL 2.1.4
  Make GitHub Actions happy on 2.1/2.2 branches
  test/openssl/test_cipher: skip AES-CCM tests on OpenSSL &lt;= 1.1.1b
  pkey/ec: check existence of public key component before exporting
  pkey: restore support for decoding "openssl ecparam -genkey" output
  pkey: clear error queue before each OSSL_DECODER_from_bio() call
  pkey/dsa: let PKey::DSA.generate choose appropriate q size
  hmac: use EVP_PKEY_new_raw_private_key() if available
  x509*: fix error queue leak in #extensions= and #attributes= methods
  Check if the option is an Hash in `pkey_ctx_apply_options0()`
diff --git a/History.md b/History.md
@@ -1,3 +1,27 @@
+Version 3.0.1
+=============
+
+Merged changes in 2.1.4 and 2.2.2. Additionally, the following issues are fixed
+by this release.
+
+Bug fixes
+---------
+
+* Add missing type check in OpenSSL::PKey::PKey#sign's optional parameters.
+  [[GitHub #531]](https://github.com/ruby/openssl/pull/531)
+* Work around OpenSSL 3.0's HMAC issues with a zero-length key.
+  [[GitHub #538]](https://github.com/ruby/openssl/pull/538)
+* Fix a regression in OpenSSL::PKey::DSA.generate's default of 'q' size.
+  [[GitHub #483]](https://github.com/ruby/openssl/issues/483)
+  [[GitHub #539]](https://github.com/ruby/openssl/pull/539)
+* Restore OpenSSL::PKey.read's ability to decode "openssl ecparam -genkey"
+  output when linked against OpenSSL 3.0.
+  [[GitHub #535]](https://github.com/ruby/openssl/pull/535)
+  [[GitHub #540]](https://github.com/ruby/openssl/pull/540)
+* Restore error checks in OpenSSL::PKey::EC#{to_der,to_pem}.
+  [[GitHub #541]](https://github.com/ruby/openssl/pull/541)
+
+
 Version 3.0.0
 =============
 
@@ -100,6 +124,12 @@ Notable changes
     [[GitHub #342]](https://github.com/ruby/openssl/issues/342)
 
 
+Version 2.2.2
+=============
+
+Merged changes in 2.1.4.
+
+
 Version 2.2.1
 =============
 
@@ -194,6 +224,16 @@ Notable changes
   [[GitHub #297]](https://github.com/ruby/openssl/pull/297)
 
 
+Version 2.1.4
+=============
+
+Bug fixes
+---------
+
+* Do not use pkg-config if --with-openssl-dir option is specified.
+ [[GitHub #486]](https://github.com/ruby/openssl/pull/486)
+
+
 Version 2.1.3
 =============
 
diff --git a/ext/openssl/extconf.rb b/ext/openssl/extconf.rb
@@ -174,6 +174,7 @@ def find_openssl_library
 
 # added in 1.1.1
 have_func("EVP_PKEY_check", evp_h)
+have_func("EVP_PKEY_new_raw_private_key", evp_h)
 have_func("SSL_CTX_set_ciphersuites", ssl_h)
 
 # added in 3.0.0
diff --git a/ext/openssl/ossl_hmac.c b/ext/openssl/ossl_hmac.c
@@ -97,11 +97,19 @@ ossl_hmac_initialize(VALUE self, VALUE key, VALUE digest)
 
     GetHMAC(self, ctx);
     StringValue(key);
+#ifdef HAVE_EVP_PKEY_NEW_RAW_PRIVATE_KEY
+    pkey = EVP_PKEY_new_raw_private_key(EVP_PKEY_HMAC, NULL,
+                                        (unsigned char *)RSTRING_PTR(key),
+                                        RSTRING_LENINT(key));
+    if (!pkey)
+        ossl_raise(eHMACError, "EVP_PKEY_new_raw_private_key");
+#else
     pkey = EVP_PKEY_new_mac_key(EVP_PKEY_HMAC, NULL,
                                 (unsigned char *)RSTRING_PTR(key),
                                 RSTRING_LENINT(key));
     if (!pkey)
         ossl_raise(eHMACError, "EVP_PKEY_new_mac_key");
+#endif
     if (EVP_DigestSignInit(ctx, NULL, ossl_evp_get_digestbyname(digest),
                            NULL, pkey) != 1) {
         EVP_PKEY_free(pkey);
diff --git a/ext/openssl/ossl_pkey.c b/ext/openssl/ossl_pkey.c
@@ -99,17 +99,56 @@ ossl_pkey_read_generic(BIO *bio, VALUE pass)
     /* First check DER */
     if (OSSL_DECODER_from_bio(dctx, bio) == 1)
         goto out;
+    OSSL_BIO_reset(bio);
 
     /* Then check PEM; multiple OSSL_DECODER_from_bio() calls may be needed */
-    OSSL_BIO_reset(bio);
     if (OSSL_DECODER_CTX_set_input_type(dctx, "PEM") != 1)
         goto out;
-    while (OSSL_DECODER_from_bio(dctx, bio) != 1) {
-        if (BIO_eof(bio))
+    /*
+     * First check for private key formats. This is to keep compatibility with
+     * ruby/openssl < 3.0 which decoded the following as a private key.
+     *
+     *     $ openssl ecparam -name prime256v1 -genkey -outform PEM
+     *     -----BEGIN EC PARAMETERS-----
+     *     BggqhkjOPQMBBw==
+     *     -----END EC PARAMETERS-----
+     *     -----BEGIN EC PRIVATE KEY-----
+     *     MHcCAQEEIAG8ugBbA5MHkqnZ9ujQF93OyUfL9tk8sxqM5Wv5tKg5oAoGCCqGSM49
+     *     AwEHoUQDQgAEVcjhJfkwqh5C7kGuhAf8XaAjVuG5ADwb5ayg/cJijCgs+GcXeedj
+     *     86avKpGH84DXUlB23C/kPt+6fXYlitUmXQ==
+     *     -----END EC PRIVATE KEY-----
+     *
+     * While the first PEM block is a proper encoding of ECParameters, thus
+     * OSSL_DECODER_from_bio() would pick it up, ruby/openssl used to return
+     * the latter instead. Existing applications expect this behavior.
+     *
+     * Note that normally, the input is supposed to contain a single decodable
+     * PEM block only, so this special handling should not create a new problem.
+     */
+    OSSL_DECODER_CTX_set_selection(dctx, EVP_PKEY_KEYPAIR);
+    while (1) {
+        if (OSSL_DECODER_from_bio(dctx, bio) == 1)
             goto out;
+        if (BIO_eof(bio))
+            break;
         pos2 = BIO_tell(bio);
         if (pos2 < 0 || pos2 <= pos)
+            break;
+        ossl_clear_error();
+        pos = pos2;
+    }
+
+    OSSL_BIO_reset(bio);
+    OSSL_DECODER_CTX_set_selection(dctx, 0);
+    while (1) {
+        if (OSSL_DECODER_from_bio(dctx, bio) == 1)
             goto out;
+        if (BIO_eof(bio))
+            break;
+        pos2 = BIO_tell(bio);
+        if (pos2 < 0 || pos2 <= pos)
+            break;
+        ossl_clear_error();
         pos = pos2;
     }
 
@@ -200,6 +239,7 @@ static VALUE
 pkey_ctx_apply_options0(VALUE args_v)
 {
     VALUE *args = (VALUE *)args_v;
+    Check_Type(args[1], T_HASH);
 
     rb_block_call(args[1], rb_intern("each"), 0, NULL,
                   pkey_ctx_apply_options_i, args[0]);
diff --git a/ext/openssl/ossl_pkey_ec.c b/ext/openssl/ossl_pkey_ec.c
@@ -414,6 +414,8 @@ ossl_ec_key_export(int argc, VALUE *argv, VALUE self)
     EC_KEY *ec;
 
     GetEC(self, ec);
+    if (EC_KEY_get0_public_key(ec) == NULL)
+        ossl_raise(eECError, "can't export - no public key set");
     if (EC_KEY_get0_private_key(ec))
         return ossl_pkey_export_traditional(argc, argv, self, 0);
     else
@@ -432,6 +434,8 @@ ossl_ec_key_to_der(VALUE self)
     EC_KEY *ec;
 
     GetEC(self, ec);
+    if (EC_KEY_get0_public_key(ec) == NULL)
+        ossl_raise(eECError, "can't export - no public key set");
     if (EC_KEY_get0_private_key(ec))
         return ossl_pkey_export_traditional(0, NULL, self, 1);
     else
diff --git a/ext/openssl/ossl_x509cert.c b/ext/openssl/ossl_x509cert.c
@@ -642,12 +642,12 @@ ossl_x509_set_extensions(VALUE self, VALUE ary)
 	OSSL_Check_Kind(RARRAY_AREF(ary, i), cX509Ext);
     }
     GetX509(self, x509);
-    while ((ext = X509_delete_ext(x509, 0)))
-	X509_EXTENSION_free(ext);
+    for (i = X509_get_ext_count(x509); i > 0; i--)
+        X509_EXTENSION_free(X509_delete_ext(x509, 0));
     for (i=0; i<RARRAY_LEN(ary); i++) {
 	ext = GetX509ExtPtr(RARRAY_AREF(ary, i));
 	if (!X509_add_ext(x509, ext, -1)) { /* DUPs ext */
-	    ossl_raise(eX509CertError, NULL);
+	    ossl_raise(eX509CertError, "X509_add_ext");
 	}
     }
 
diff --git a/ext/openssl/ossl_x509crl.c b/ext/openssl/ossl_x509crl.c
@@ -474,12 +474,12 @@ ossl_x509crl_set_extensions(VALUE self, VALUE ary)
 	OSSL_Check_Kind(RARRAY_AREF(ary, i), cX509Ext);
     }
     GetX509CRL(self, crl);
-    while ((ext = X509_CRL_delete_ext(crl, 0)))
-	X509_EXTENSION_free(ext);
+    for (i = X509_CRL_get_ext_count(crl); i > 0; i--)
+        X509_EXTENSION_free(X509_CRL_delete_ext(crl, 0));
     for (i=0; i<RARRAY_LEN(ary); i++) {
 	ext = GetX509ExtPtr(RARRAY_AREF(ary, i)); /* NO NEED TO DUP */
 	if (!X509_CRL_add_ext(crl, ext, -1)) {
-	    ossl_raise(eX509CRLError, NULL);
+	    ossl_raise(eX509CRLError, "X509_CRL_add_ext");
 	}
     }
 
diff --git a/ext/openssl/ossl_x509req.c b/ext/openssl/ossl_x509req.c
@@ -380,13 +380,13 @@ ossl_x509req_set_attributes(VALUE self, VALUE ary)
 	OSSL_Check_Kind(RARRAY_AREF(ary, i), cX509Attr);
     }
     GetX509Req(self, req);
-    while ((attr = X509_REQ_delete_attr(req, 0)))
-	X509_ATTRIBUTE_free(attr);
+    for (i = X509_REQ_get_attr_count(req); i > 0; i--)
+        X509_ATTRIBUTE_free(X509_REQ_delete_attr(req, 0));
     for (i=0;i<RARRAY_LEN(ary); i++) {
 	item = RARRAY_AREF(ary, i);
 	attr = GetX509AttrPtr(item);
 	if (!X509_REQ_add1_attr(req, attr)) {
-	    ossl_raise(eX509ReqError, NULL);
+	    ossl_raise(eX509ReqError, "X509_REQ_add1_attr");
 	}
     }
     return ary;
diff --git a/ext/openssl/ossl_x509revoked.c b/ext/openssl/ossl_x509revoked.c
@@ -223,13 +223,13 @@ ossl_x509revoked_set_extensions(VALUE self, VALUE ary)
 	OSSL_Check_Kind(RARRAY_AREF(ary, i), cX509Ext);
     }
     GetX509Rev(self, rev);
-    while ((ext = X509_REVOKED_delete_ext(rev, 0)))
-	X509_EXTENSION_free(ext);
+    for (i = X509_REVOKED_get_ext_count(rev); i > 0; i--)
+        X509_EXTENSION_free(X509_REVOKED_delete_ext(rev, 0));
     for (i=0; i<RARRAY_LEN(ary); i++) {
 	item = RARRAY_AREF(ary, i);
 	ext = GetX509ExtPtr(item);
 	if(!X509_REVOKED_add_ext(rev, ext, -1)) {
-	    ossl_raise(eX509RevError, NULL);
+	    ossl_raise(eX509RevError, "X509_REVOKED_add_ext");
 	}
     }
 
diff --git a/lib/openssl/pkey.rb b/lib/openssl/pkey.rb
@@ -167,8 +167,16 @@ class << self
       # +size+::
       #   The desired key size in bits.
       def generate(size, &blk)
+        # FIPS 186-4 specifies four (L,N) pairs: (1024,160), (2048,224),
+        # (2048,256), and (3072,256).
+        #
+        # q size is derived here with compatibility with
+        # DSA_generator_parameters_ex() which previous versions of ruby/openssl
+        # used to call.
+        qsize = size >= 2048 ? 256 : 160
         dsaparams = OpenSSL::PKey.generate_parameters("DSA", {
           "dsa_paramgen_bits" => size,
+          "dsa_paramgen_q_bits" => qsize,
         }, &blk)
         OpenSSL::PKey.generate_key(dsaparams)
       end
diff --git a/lib/openssl/version.rb b/lib/openssl/version.rb
@@ -1,5 +1,5 @@
 # frozen_string_literal: true
 
 module OpenSSL
-  VERSION = "3.0.0"
+  VERSION = "3.0.1"
 end
diff --git a/openssl.gemspec b/openssl.gemspec
@@ -1,6 +1,6 @@
 Gem::Specification.new do |spec|
   spec.name          = "openssl"
-  spec.version       = "3.0.0"
+  spec.version       = "3.0.1"
   spec.authors       = ["Martin Bosslet", "SHIBATA Hiroshi", "Zachary Scott", "Kazuki Yamaguchi"]
   spec.email         = ["ruby-core@ruby-lang.org"]
   spec.summary       = %q{OpenSSL provides SSL, TLS and general purpose cryptography.}
diff --git a/test/openssl/test_hmac.rb b/test/openssl/test_hmac.rb
@@ -62,6 +62,14 @@ def test_singleton_methods
     b64digest = OpenSSL::HMAC.base64digest("MD5", key, "Hi There")
     assert_equal "kpRyejY4uxwT9I74FYv8nQ==", b64digest
   end
+
+  def test_zero_length_key
+    # Empty string as the key
+    hexdigest = OpenSSL::HMAC.hexdigest("SHA256", "\0"*32, "test")
+    assert_equal "43b0cef99265f9e34c10ea9d3501926d27b39f57c6d674561d8ba236e7a819fb", hexdigest
+    hexdigest = OpenSSL::HMAC.hexdigest("SHA256", "", "test")
+    assert_equal "43b0cef99265f9e34c10ea9d3501926d27b39f57c6d674561d8ba236e7a819fb", hexdigest
+  end
 end
 
 end
diff --git a/test/openssl/test_pkey_dsa.rb b/test/openssl/test_pkey_dsa.rb
@@ -28,6 +28,25 @@ def test_new_break
     end
   end
 
+  def test_generate
+    # DSA.generate used to call DSA_generate_parameters_ex(), which adjusts the
+    # size of q according to the size of p
+    key1024 = OpenSSL::PKey::DSA.generate(1024)
+    assert_predicate key1024, :private?
+    assert_equal 1024, key1024.p.num_bits
+    assert_equal 160, key1024.q.num_bits
+
+    key2048 = OpenSSL::PKey::DSA.generate(2048)
+    assert_equal 2048, key2048.p.num_bits
+    assert_equal 256, key2048.q.num_bits
+
+    if ENV["OSSL_TEST_ALL"] == "1" # slow
+      key3072 = OpenSSL::PKey::DSA.generate(3072)
+      assert_equal 3072, key3072.p.num_bits
+      assert_equal 256, key3072.q.num_bits
+    end
+  end
+
   def test_sign_verify
     dsa512 = Fixtures.pkey("dsa512")
     data = "Sign me!"
diff --git a/test/openssl/test_pkey_ec.rb b/test/openssl/test_pkey_ec.rb
@@ -61,8 +61,10 @@ def test_generate
   def test_generate_key
     ec = OpenSSL::PKey::EC.new("prime256v1")
     assert_equal false, ec.private?
+    assert_raise(OpenSSL::PKey::ECError) { ec.to_der }
     ec.generate_key!
     assert_equal true, ec.private?
+    assert_nothing_raised { ec.to_der }
   end if !openssl?(3, 0, 0)
 
   def test_marshal
@@ -199,6 +201,29 @@ def test_ECPrivateKey
     assert_equal pem, p256.export
   end
 
+  def test_ECPrivateKey_with_parameters
+    p256 = Fixtures.pkey("p256")
+
+    # The format used by "openssl ecparam -name prime256v1 -genkey -outform PEM"
+    #
+    # "EC PARAMETERS" block should be ignored if it is followed by an
+    # "EC PRIVATE KEY" block
+    in_pem = <<~EOF
+    -----BEGIN EC PARAMETERS-----
+    BggqhkjOPQMBBw==
+    -----END EC PARAMETERS-----
+    -----BEGIN EC PRIVATE KEY-----
+    MHcCAQEEIID49FDqcf1O1eO8saTgG70UbXQw9Fqwseliit2aWhH1oAoGCCqGSM49
+    AwEHoUQDQgAEFglk2c+oVUIKQ64eZG9bhLNPWB7lSZ/ArK41eGy5wAzU/0G51Xtt
+    CeBUl+MahZtn9fO1JKdF4qJmS39dXnpENg==
+    -----END EC PRIVATE KEY-----
+    EOF
+
+    key = OpenSSL::PKey::EC.new(in_pem)
+    assert_same_ec p256, key
+    assert_equal p256.to_der, key.to_der
+  end
+
   def test_ECPrivateKey_encrypted
     p256 = Fixtures.pkey("p256")
     # key = abcdef
diff --git a/test/openssl/test_pkey_rsa.rb b/test/openssl/test_pkey_rsa.rb
@@ -108,6 +108,11 @@ def test_sign_verify_options
                                       salt_length: 20, mgf1_hash: "SHA1")
     # Defaults to PKCS #1 v1.5 padding => verification failure
     assert_equal false, key.verify("SHA256", sig_pss, data)
+
+    # option type check
+    assert_raise_with_message(TypeError, /expected Hash/) {
+      key.sign("SHA256", data, ["x"])
+    }
   end
 
   def test_sign_verify_raw

Original file line number	Diff line number	Diff line change
`@@ -642,12 +642,12 @@ ossl_x509_set_extensions(VALUE self, VALUE ary)`
`642`	`642`	`OSSL_Check_Kind(RARRAY_AREF(ary, i), cX509Ext);`
`643`	`643`	`}`
`644`	`644`	`GetX509(self, x509);`
`645`		`- while ((ext = X509_delete_ext(x509, 0)))`
`646`		`- X509_EXTENSION_free(ext);`
	`645`	`+ for (i = X509_get_ext_count(x509); i > 0; i--)`
	`646`	`+ X509_EXTENSION_free(X509_delete_ext(x509, 0));`
`647`	`647`	`for (i=0; i<RARRAY_LEN(ary); i++) {`
`648`	`648`	`ext = GetX509ExtPtr(RARRAY_AREF(ary, i));`
`649`	`649`	`if (!X509_add_ext(x509, ext, -1)) { /* DUPs ext */`
`650`		`- ossl_raise(eX509CertError, NULL);`
	`650`	`+ ossl_raise(eX509CertError, "X509_add_ext");`
`651`	`651`	`}`
`652`	`652`	`}`
`653`	`653`
Original file line number	Diff line number	Diff line change
`@@ -474,12 +474,12 @@ ossl_x509crl_set_extensions(VALUE self, VALUE ary)`
`474`	`474`	`OSSL_Check_Kind(RARRAY_AREF(ary, i), cX509Ext);`
`475`	`475`	`}`
`476`	`476`	`GetX509CRL(self, crl);`
`477`		`- while ((ext = X509_CRL_delete_ext(crl, 0)))`
`478`		`- X509_EXTENSION_free(ext);`
	`477`	`+ for (i = X509_CRL_get_ext_count(crl); i > 0; i--)`
	`478`	`+ X509_EXTENSION_free(X509_CRL_delete_ext(crl, 0));`
`479`	`479`	`for (i=0; i<RARRAY_LEN(ary); i++) {`
`480`	`480`	`ext = GetX509ExtPtr(RARRAY_AREF(ary, i)); /* NO NEED TO DUP */`
`481`	`481`	`if (!X509_CRL_add_ext(crl, ext, -1)) {`
`482`		`- ossl_raise(eX509CRLError, NULL);`
	`482`	`+ ossl_raise(eX509CRLError, "X509_CRL_add_ext");`
`483`	`483`	`}`
`484`	`484`	`}`
`485`	`485`
Original file line number	Diff line number	Diff line change
`@@ -380,13 +380,13 @@ ossl_x509req_set_attributes(VALUE self, VALUE ary)`
`380`	`380`	`OSSL_Check_Kind(RARRAY_AREF(ary, i), cX509Attr);`
`381`	`381`	`}`
`382`	`382`	`GetX509Req(self, req);`
`383`		`- while ((attr = X509_REQ_delete_attr(req, 0)))`
`384`		`- X509_ATTRIBUTE_free(attr);`
	`383`	`+ for (i = X509_REQ_get_attr_count(req); i > 0; i--)`
	`384`	`+ X509_ATTRIBUTE_free(X509_REQ_delete_attr(req, 0));`
`385`	`385`	`for (i=0;i<RARRAY_LEN(ary); i++) {`
`386`	`386`	`item = RARRAY_AREF(ary, i);`
`387`	`387`	`attr = GetX509AttrPtr(item);`
`388`	`388`	`if (!X509_REQ_add1_attr(req, attr)) {`
`389`		`- ossl_raise(eX509ReqError, NULL);`
	`389`	`+ ossl_raise(eX509ReqError, "X509_REQ_add1_attr");`
`390`	`390`	`}`
`391`	`391`	`}`
`392`	`392`	`return ary;`
Original file line number	Diff line number	Diff line change
`@@ -223,13 +223,13 @@ ossl_x509revoked_set_extensions(VALUE self, VALUE ary)`
`223`	`223`	`OSSL_Check_Kind(RARRAY_AREF(ary, i), cX509Ext);`
`224`	`224`	`}`
`225`	`225`	`GetX509Rev(self, rev);`
`226`		`- while ((ext = X509_REVOKED_delete_ext(rev, 0)))`
`227`		`- X509_EXTENSION_free(ext);`
	`226`	`+ for (i = X509_REVOKED_get_ext_count(rev); i > 0; i--)`
	`227`	`+ X509_EXTENSION_free(X509_REVOKED_delete_ext(rev, 0));`
`228`	`228`	`for (i=0; i<RARRAY_LEN(ary); i++) {`
`229`	`229`	`item = RARRAY_AREF(ary, i);`
`230`	`230`	`ext = GetX509ExtPtr(item);`
`231`	`231`	`if(!X509_REVOKED_add_ext(rev, ext, -1)) {`
`232`		`- ossl_raise(eX509RevError, NULL);`
	`232`	`+ ossl_raise(eX509RevError, "X509_REVOKED_add_ext");`
`233`	`233`	`}`
`234`	`234`	`}`
`235`	`235`