From 967876f15d4aba19c7804648a086b6218184c045 Mon Sep 17 00:00:00 2001 From: Jenny Shen Date: Wed, 6 Oct 2021 17:39:23 -0400 Subject: [PATCH] Add support to build and sign certificates with multiple key algorithms Co-Authored-By: Frederik Dudzik --- Manifest.txt | 1 + lib/rubygems/commands/cert_command.rb | 19 ++++-- lib/rubygems/security.rb | 64 +++++++++++++----- lib/rubygems/security/policy.rb | 8 ++- lib/rubygems/security/signer.rb | 7 +- test/rubygems/helper.rb | 4 +- test/rubygems/private_ec_key.pem | 9 +++ .../test_gem_commands_cert_command.rb | 67 +++++++++++++++++-- test/rubygems/test_gem_security.rb | 36 ++++++++-- 9 files changed, 177 insertions(+), 38 deletions(-) create mode 100644 test/rubygems/private_ec_key.pem diff --git a/Manifest.txt b/Manifest.txt index 3f4c4cc74784..47821ad4e461 100644 --- a/Manifest.txt +++ b/Manifest.txt @@ -541,6 +541,7 @@ test/rubygems/plugin/exception/rubygems_plugin.rb test/rubygems/plugin/load/rubygems_plugin.rb test/rubygems/plugin/standarderror/rubygems_plugin.rb test/rubygems/private3072_key.pem +test/rubygems/private_ec_key.pem test/rubygems/private_key.pem test/rubygems/public3072_cert.pem test/rubygems/public_cert.pem diff --git a/lib/rubygems/commands/cert_command.rb b/lib/rubygems/commands/cert_command.rb index bdfeb0ba6e84..867cb07cca0f 100644 --- a/lib/rubygems/commands/cert_command.rb +++ b/lib/rubygems/commands/cert_command.rb @@ -43,6 +43,11 @@ def initialize options[:key] = open_private_key(key_file) end + add_option('-A', '--key-algorithm ALGORITHM', + 'Select which key algorithm to use for --build') do |algorithm, options| + options[:key_algorithm] = algorithm + end + add_option('-s', '--sign CERT', 'Signs CERT with the key from -K', 'and the certificate from -C') do |cert_file, options| @@ -89,14 +94,14 @@ def open_cert(certificate_file) def open_private_key(key_file) check_openssl passphrase = ENV['GEM_PRIVATE_KEY_PASSPHRASE'] - key = OpenSSL::PKey::RSA.new File.read(key_file), passphrase + key = OpenSSL::PKey.read File.read(key_file), passphrase raise OptionParser::InvalidArgument, "#{key_file}: private key not found" unless key.private? key rescue Errno::ENOENT raise OptionParser::InvalidArgument, "#{key_file}: does not exist" - rescue OpenSSL::PKey::RSAError - raise OptionParser::InvalidArgument, "#{key_file}: invalid RSA key" + rescue OpenSSL::PKey::PKeyError, ArgumentError + raise OptionParser::InvalidArgument, "#{key_file}: invalid RSA, DSA, or EC key" end def execute @@ -170,7 +175,8 @@ def build_key # :nodoc: raise Gem::CommandLineError, "Passphrase and passphrase confirmation don't match" unless passphrase == passphrase_confirmation - key = Gem::Security.create_key + algorithm = options[:key_algorithm] || Gem::Security::DEFAULT_KEY_ALGORITHM + key = Gem::Security.create_key(algorithm) key_path = Gem::Security.write key, "gem-private_key.pem", 0600, passphrase return key, key_path @@ -255,13 +261,14 @@ def load_default_key key_file = File.join Gem.default_key_path key = File.read key_file passphrase = ENV['GEM_PRIVATE_KEY_PASSPHRASE'] - options[:key] = OpenSSL::PKey::RSA.new key, passphrase + options[:key] = OpenSSL::PKey.read key, passphrase + rescue Errno::ENOENT alert_error \ "--private-key not specified and ~/.gem/gem-private_key.pem does not exist" terminate_interaction 1 - rescue OpenSSL::PKey::RSAError + rescue OpenSSL::PKey::PKeyError alert_error \ "--private-key not specified and ~/.gem/gem-private_key.pem is not valid" diff --git a/lib/rubygems/security.rb b/lib/rubygems/security.rb index 28f705549c83..8240a1a05918 100644 --- a/lib/rubygems/security.rb +++ b/lib/rubygems/security.rb @@ -152,6 +152,7 @@ # certificate for EMAIL_ADDR # -C, --certificate CERT Signing certificate for --sign # -K, --private-key KEY Key for --sign or --build +# -A, --key-algorithm ALGORITHM Select key algorithm for --build from RSA, DSA, or EC. Defaults to RSA. # -s, --sign CERT Signs CERT with the key from -K # and the certificate from -C # -d, --days NUMBER_OF_DAYS Days before the certificate expires @@ -317,7 +318,6 @@ # * Honor extension restrictions # * Might be better to store the certificate chain as a PKCS#7 or PKCS#12 # file, instead of an array embedded in the metadata. -# * Flexible signature and key algorithms, not hard-coded to RSA and SHA1. # # == Original author # @@ -337,17 +337,19 @@ class Exception < Gem::Exception; end DIGEST_NAME = 'SHA256' # :nodoc: ## - # Algorithm for creating the key pair used to sign gems + # Length of keys created by RSA and DSA keys - KEY_ALGORITHM = - if defined?(OpenSSL::PKey::RSA) - OpenSSL::PKey::RSA - end + RSA_DSA_KEY_LENGTH = 3072 ## - # Length of keys created by KEY_ALGORITHM + # Default algorithm to use when building a key pair - KEY_LENGTH = 3072 + DEFAULT_KEY_ALGORITHM = 'RSA' + + ## + # Named curve used for Elliptic Curve + + EC_NAME = 'secp384r1' ## # Cipher used to encrypt the key pair used to sign gems. @@ -400,7 +402,7 @@ def self.create_cert(subject, key, age = ONE_YEAR, extensions = EXTENSIONS, serial = 1) cert = OpenSSL::X509::Certificate.new - cert.public_key = key.public_key + cert.public_key = get_public_key(key) cert.version = 2 cert.serial = serial @@ -418,6 +420,24 @@ def self.create_cert(subject, key, age = ONE_YEAR, extensions = EXTENSIONS, cert end + ## + # Gets the right public key from a PKey instance + + def self.get_public_key(key) + return key.public_key unless key.is_a?(OpenSSL::PKey::EC) + + ec_key = OpenSSL::PKey::EC.new(key.group.curve_name) + ec_key.public_key = key.public_key + ec_key + end + + ## + # In Ruby 2.3 EC doesn't implement the private_key? but not the private? method + + if defined?(OpenSSL::PKey::EC) && Gem::Version.new(String.new(RUBY_VERSION)) < Gem::Version.new("2.4.0") + OpenSSL::PKey::EC.send(:alias_method, :private?, :private_key?) + end + ## # Creates a self-signed certificate with an issuer and subject from +email+, # a subject alternative name of +email+ and the given +extensions+ for the @@ -459,11 +479,25 @@ def self.create_digest(algorithm = DIGEST_NAME) end ## - # Creates a new key pair of the specified +length+ and +algorithm+. The - # default is a 3072 bit RSA key. - - def self.create_key(length = KEY_LENGTH, algorithm = KEY_ALGORITHM) - algorithm.new length + # Creates a new key pair of the specified +algorithm+. RSA, DSA, and EC + # are supported. + + def self.create_key(algorithm) + if defined?(OpenSSL::PKey) + case algorithm.downcase + when 'dsa' + OpenSSL::PKey::DSA.new(RSA_DSA_KEY_LENGTH) + when 'rsa' + OpenSSL::PKey::RSA.new(RSA_DSA_KEY_LENGTH) + when 'ec' + domain_key = OpenSSL::PKey::EC.new(EC_NAME) + domain_key.generate_key + domain_key + else + raise Gem::Security::Exception, + "#{algorithm} algorithm not found. RSA, DSA, and EC algorithms are supported." + end + end end ## @@ -492,7 +526,7 @@ def self.re_sign(expired_certificate, private_key, age = ONE_YEAR, raise Gem::Security::Exception, "incorrect signing key for re-signing " + "#{expired_certificate.subject}" unless - expired_certificate.public_key.to_pem == private_key.public_key.to_pem + expired_certificate.public_key.to_pem == get_public_key(private_key).to_pem unless expired_certificate.subject.to_s == expired_certificate.issuer.to_s diff --git a/lib/rubygems/security/policy.rb b/lib/rubygems/security/policy.rb index 9683e55b327e..3c3cb647ee36 100644 --- a/lib/rubygems/security/policy.rb +++ b/lib/rubygems/security/policy.rb @@ -115,9 +115,11 @@ def check_key(signer, key) raise Gem::Security::Exception, 'missing key or signature' end + public_key = Gem::Security.get_public_key(key) + raise Gem::Security::Exception, "certificate #{signer.subject} does not match the signing key" unless - signer.public_key.to_pem == key.public_key.to_pem + signer.public_key.to_pem == public_key.to_pem true end @@ -164,9 +166,9 @@ def check_trust(chain, digester, trust_dir) end save_cert = OpenSSL::X509::Certificate.new File.read path - save_dgst = digester.digest save_cert.public_key.to_s + save_dgst = digester.digest save_cert.public_key.to_pem - pkey_str = root.public_key.to_s + pkey_str = root.public_key.to_pem cert_dgst = digester.digest pkey_str raise Gem::Security::Exception, diff --git a/lib/rubygems/security/signer.rb b/lib/rubygems/security/signer.rb index c5c2c4f22004..968cf889730a 100644 --- a/lib/rubygems/security/signer.rb +++ b/lib/rubygems/security/signer.rb @@ -83,8 +83,8 @@ def initialize(key, cert_chain, passphrase = nil, options = {}) @digest_name = Gem::Security::DIGEST_NAME @digest_algorithm = Gem::Security.create_digest(@digest_name) - if @key && !@key.is_a?(OpenSSL::PKey::RSA) - @key = OpenSSL::PKey::RSA.new(File.read(@key), @passphrase) + if @key && !@key.is_a?(OpenSSL::PKey::PKey) + @key = OpenSSL::PKey.read(File.read(@key), @passphrase) end if @cert_chain @@ -177,8 +177,7 @@ def re_sign_key(expiration_length: Gem::Security::ONE_YEAR) # :nodoc: disk_cert = File.read(disk_cert_path) rescue nil disk_key_path = File.join(Gem.default_key_path) - disk_key = - OpenSSL::PKey::RSA.new(File.read(disk_key_path), @passphrase) rescue nil + disk_key = OpenSSL::PKey.read(File.read(disk_key_path), @passphrase) rescue nil return unless disk_key diff --git a/test/rubygems/helper.rb b/test/rubygems/helper.rb index 17f7350f132b..504e25ea52e8 100644 --- a/test/rubygems/helper.rb +++ b/test/rubygems/helper.rb @@ -1519,14 +1519,14 @@ def self.cert_path(cert_name) end ## - # Loads an RSA private key named +key_name+ with +passphrase+ in test/rubygems/ + # Loads a private key named +key_name+ with +passphrase+ in test/rubygems/ def self.load_key(key_name, passphrase = nil) key_file = key_path key_name key = File.read key_file - OpenSSL::PKey::RSA.new key, passphrase + OpenSSL::PKey.read key, passphrase end ## diff --git a/test/rubygems/private_ec_key.pem b/test/rubygems/private_ec_key.pem new file mode 100644 index 000000000000..5d855d0dfc86 --- /dev/null +++ b/test/rubygems/private_ec_key.pem @@ -0,0 +1,9 @@ +-----BEGIN EC PRIVATE KEY----- +Proc-Type: 4,ENCRYPTED +DEK-Info: AES-256-CBC,4107F98A374CB8EC18F1AA4EA4B6A0DB + +BRklFxJGcz7gqQYxek8TZkt8qbPhB0FSR6nyw3SYuio/2tlT9ohs74mlK3EbG9Lt +Y4OquJbksBFmoB7fIoM4vnuIZ0Eoz2ooxn9tjhBtqJ3mVscYXwZmA3UDUWDMlviQ +Fu37OpikQv4TFA1jlmUK0LM8xmUCfUeLl0kHD17lFsz2gkO2kwg8mn/YUMOIaDOu +EnnmxbAwnZBpemQkQfpTt2mYL9gu3CcMt5gokBuGDxY= +-----END EC PRIVATE KEY----- diff --git a/test/rubygems/test_gem_commands_cert_command.rb b/test/rubygems/test_gem_commands_cert_command.rb index f722678a192d..077be11d55e5 100644 --- a/test/rubygems/test_gem_commands_cert_command.rb +++ b/test/rubygems/test_gem_commands_cert_command.rb @@ -14,9 +14,10 @@ class TestGemCommandsCertCommand < Gem::TestCase ALTERNATE_CERT = load_cert 'alternate' EXPIRED_PUBLIC_CERT = load_cert 'expired' - ALTERNATE_KEY_FILE = key_path 'alternate' - PRIVATE_KEY_FILE = key_path 'private' - PUBLIC_KEY_FILE = key_path 'public' + ALTERNATE_KEY_FILE = key_path 'alternate' + PRIVATE_KEY_FILE = key_path 'private' + PRIVATE_EC_KEY_FILE = key_path 'private_ec' + PUBLIC_KEY_FILE = key_path 'public' ALTERNATE_CERT_FILE = cert_path 'alternate' CHILD_CERT_FILE = cert_path 'child' @@ -142,6 +143,42 @@ def test_execute_build assert_path_exist File.join(@tempdir, 'gem-public_cert.pem') end + def test_execute_build_key_algorithm_ec_key + passphrase = 'Foo bar' + + @cmd.handle_options %W[--build nobody@example.com --key-algorithm ec] + + @build_ui = Gem::MockGemUi.new "#{passphrase}\n#{passphrase}" + + use_ui @build_ui do + @cmd.execute + end + + output = @build_ui.output.squeeze("\n").split "\n" + + assert_equal "Passphrase for your Private Key: ", + output.shift + assert_equal "Please repeat the passphrase for your Private Key: ", + output.shift + assert_equal "Certificate: #{File.join @tempdir, 'gem-public_cert.pem'}", + output.shift + assert_equal "Private Key: #{File.join @tempdir, 'gem-private_key.pem'}", + output.shift + + assert_equal "Don't forget to move the key file to somewhere private!", + output.shift + + assert_empty output + assert_empty @build_ui.error + + assert_path_exist File.join(@tempdir, 'gem-private_key.pem') + + cert_path = File.join(@tempdir, 'gem-public_cert.pem') + assert_path_exist cert_path + cert = OpenSSL::X509::Certificate.new(File.read(cert_path)) + assert cert.public_key.is_a? OpenSSL::PKey::EC + end + def test_execute_build_bad_email_address passphrase = 'Foo bar' email = "nobody@" @@ -279,6 +316,28 @@ def test_execute_build_encrypted_key assert_path_exist File.join(@tempdir, 'gem-public_cert.pem') end + def test_execute_build_ec_key + @cmd.handle_options %W[ + --build nobody@example.com + --private-key #{PRIVATE_EC_KEY_FILE} + ] + + use_ui @ui do + @cmd.execute + end + + output = @ui.output.split "\n" + + assert_equal "Certificate: #{File.join @tempdir, 'gem-public_cert.pem'}", + output.shift + + assert_empty output + assert_empty @ui.error + + assert_path_exist File.join(@tempdir, 'gem-public_cert.pem') + assert_path_not_exist File.join(@tempdir, 'gem-private_key.pem') + end + def test_execute_certificate use_ui @ui do @cmd.handle_options %W[--certificate #{PUBLIC_CERT_FILE}] @@ -742,7 +801,7 @@ def test_handle_options_key_bad @cmd.handle_options %W[--private-key #{bad}] end - assert_equal "invalid argument: --private-key #{bad}: invalid RSA key", + assert_equal "invalid argument: --private-key #{bad}: invalid RSA, DSA, or EC key", e.message e = assert_raise OptionParser::InvalidArgument do diff --git a/test/rubygems/test_gem_security.rb b/test/rubygems/test_gem_security.rb index 2eabbea3bfe6..d04bd4a8bd81 100644 --- a/test/rubygems/test_gem_security.rb +++ b/test/rubygems/test_gem_security.rb @@ -12,6 +12,7 @@ class TestGemSecurity < Gem::TestCase CHILD_KEY = load_key 'child' + EC_KEY = load_key 'private_ec', 'Foo bar' ALTERNATE_CERT = load_cert 'child' CHILD_CERT = load_cert 'child' @@ -103,11 +104,38 @@ def test_class_create_cert_email end def test_class_create_key - key = @SEC.create_key 1024 + key = @SEC.create_key 'rsa' assert_kind_of OpenSSL::PKey::RSA, key end + def test_class_create_key_downcases + key = @SEC.create_key 'DSA' + + assert_kind_of OpenSSL::PKey::DSA, key + end + + def test_class_create_key_raises_unknown_algorithm + e = assert_raise Gem::Security::Exception do + @SEC.create_key 'NOT_RSA' + end + + assert_equal "NOT_RSA algorithm not found. RSA, DSA, and EC algorithms are supported.", + e.message + end + + def test_class_get_public_key_rsa + pkey_pem = PRIVATE_KEY.public_key.to_pem + + assert_equal pkey_pem, @SEC.get_public_key(PRIVATE_KEY).to_pem + end + + def test_class_get_public_key_ec + pkey = @SEC.get_public_key(EC_KEY) + + assert_respond_to pkey, :to_pem + end + def test_class_email_to_name assert_equal '/CN=nobody/DC=example', @SEC.email_to_name('nobody@example').to_s @@ -259,7 +287,7 @@ def test_class_trust_dir end def test_class_write - key = @SEC.create_key 1024 + key = @SEC.create_key 'rsa' path = File.join @tempdir, 'test-private_key.pem' @@ -273,7 +301,7 @@ def test_class_write end def test_class_write_encrypted - key = @SEC.create_key 1024 + key = @SEC.create_key 'rsa' path = File.join @tempdir, 'test-private_encrypted_key.pem' @@ -289,7 +317,7 @@ def test_class_write_encrypted end def test_class_write_encrypted_cipher - key = @SEC.create_key 1024 + key = @SEC.create_key 'rsa' path = File.join @tempdir, 'test-private_encrypted__with_non_default_cipher_key.pem'