Move the Ed25519 implementation to OpenSSL and stop depending on the Ed25519 gem

Author: vcsjones

Gemfile.lock

@@ -14,7 +14,6 @@ GEM
     coderay (1.1.3)
     debug_inspector (1.2.0)
     diff-lcs (1.5.0)
-    ed25519 (1.3.0)
     method_source (1.0.0)
     parser (3.2.2.4)
       ast (~> 2.4.1)
@@ -59,7 +58,6 @@ PLATFORMS
   ruby
 
 DEPENDENCIES
-  ed25519 (~> 1.2)
   pry (~> 0.14)
   rspec (~> 3.10)
   rspec-mocks (~> 3.10)

README.md

@@ -2,7 +2,7 @@
 
 This is a Ruby library for processing SSH keys and certificates.
 
-The scope of this project is limited to processing and directly using keys and certificates. It can be used to generate SSH private keys, verify signatures using public keys, sign data using private keys, issue certificates using private keys, and parse certificates and public and private keys. This library supports RSA, DSA, ECDSA, and ED25519<sup>[*](#ed25519-support)</sup> keys. This library does not offer or intend to offer functionality for SSH connectivity, processing of SSH wire protocol data, or processing of other key formats or types.
+The scope of this project is limited to processing and directly using keys and certificates. It can be used to generate SSH private keys, verify signatures using public keys, sign data using private keys, issue certificates using private keys, and parse certificates and public and private keys. This library supports RSA, DSA, ECDSA, and ED25519 keys. This library does not offer or intend to offer functionality for SSH connectivity, processing of SSH wire protocol data, or processing of other key formats or types.
 
 **Project Status:** Used by @github in production
 
@@ -32,26 +32,6 @@ cert.public_key
 #=> <SSHData::PublicKey::RSA>
 ```
 
-## ED25519 support
-
-Ruby's standard library does not include support for ED25519, though the algorithm is implemented by the [`ed25519` Gem](https://rubygems.org/gems/ed25519). This library can parse ED25519 public and private keys itself, but in order to generate keys or sign or verify messages, the calling application must load the `ed25519` Gem itself. This avoids the necessity of installing or loading this third party dependency when the calling application is only interested in parsing keys.
-
-```ruby
-require "ssh_data"
-
-key_data = File.read("~/.ssh/id_ed25519")
-key = SSHData::PrivateKey.parse_openssh(key_data)
-#=> <SSHData::PrivateKey::ED25519>
-
-SSHData::PrivateKey::ED25519.generate
-#=> raises SSHData::AlgorithmError
-
-require "ed25519"
-
-SSHData::PrivateKey::ED25519.generate
-#=> <SSHData::PrivateKey::ED25519>
-```
-
 ## Contributions
 
 This project is not currently seeking contributions for new features or functionality, though bug fixes are welcome. See [CONTRIBUTING.md](CONTRIBUTING.md) for more information.

lib/ssh_data/private_key/ed25519.rb

@@ -1,26 +1,25 @@
 module SSHData
   module PrivateKey
     class ED25519 < Base
-      attr_reader :pk, :sk, :ed25519_key
+      attr_reader :pk, :sk, :openssl
 
       # Generate a new private key.
       #
       # Returns a PublicKey::Base subclass instance.
       def self.generate
-        PublicKey::ED25519.ed25519_gem_required!
-        from_ed25519(Ed25519::SigningKey.generate)
+        from_openssl(OpenSSL::PKey.generate_key("ED25519"))
       end
 
-      # Create from a ::Ed25519::SigningKey instance.
+      # Create from a ::OpenSSL::PKey::PKey instance.
       #
-      # key - A ::Ed25519::SigningKey instance.
+      # key - A ::OpenSSL::PKey::PKey instance.
       #
       # Returns a ED25519 instance.
-      def self.from_ed25519(key)
+      def self.from_openssl(key)
         new(
           algo: PublicKey::ALGO_ED25519,
-          pk: key.verify_key.to_bytes,
-          sk: key.to_bytes + key.verify_key.to_bytes,
+          pk: key.raw_public_key,
+          sk: key.raw_private_key + key.raw_public_key,
           comment: "",
         )
       end
@@ -40,12 +39,10 @@ def initialize(algo:, pk:, sk:, comment:)
 
         super(algo: algo, comment: comment)
 
-        if PublicKey::ED25519.enabled?
-          @ed25519_key = Ed25519::SigningKey.new(sk.byteslice(0, 32))
+        @openssl = OpenSSL::PKey.read(raw_to_private_key_info_der(sk.byteslice(0, 32)))
 
-          if @ed25519_key.verify_key.to_bytes != pk
-            raise DecodeError, "bad pk"
-          end
+        if @openssl.raw_public_key != pk
+          raise DecodeError, "bad pk"
         end
 
         @public_key = PublicKey::ED25519.new(algo: algo, pk: pk)
@@ -57,12 +54,24 @@ def initialize(algo:, pk:, sk:, comment:)
       #
       # Returns a binary String signature.
       def sign(signed_data, algo: nil)
-        PublicKey::ED25519.ed25519_gem_required!
         algo ||= self.algo
         raise AlgorithmError unless algo == self.algo
-        raw_sig = ed25519_key.sign(signed_data)
+        raw_sig = openssl.sign(nil, signed_data)
         Encoding.encode_signature(algo, raw_sig)
       end
+
+      private
+
+      def raw_to_private_key_info_der(key)
+        inner_octet_string = OpenSSL::ASN1::OctetString.new(key)
+        private_key_field = OpenSSL::ASN1::OctetString.new(inner_octet_string.to_der)
+        version = OpenSSL::ASN1::Integer.new(0)
+        OpenSSL::ASN1::Sequence.new([
+          version,
+          PublicKey::ED25519.asn_algorithm_identifier,
+          private_key_field
+        ]).to_der
+      end
     end
   end
 end

lib/ssh_data/public_key/ed25519.rb

@@ -1,19 +1,14 @@
 module SSHData
   module PublicKey
     class ED25519 < Base
-      attr_reader :pk, :ed25519_key
+      attr_reader :pk, :openssl
 
-      # ed25519 isn't a hard requirement for using this Gem. We only do actual
-      # validation with the key if the ed25519 Gem has been loaded.
-      def self.enabled?
-        Object.const_defined?(:Ed25519)
-      end
+      @@alg_id = OpenSSL::ASN1::Sequence([
+        OpenSSL::ASN1::ObjectId("1.3.101.112") # id-Ed25519
+      ])
 
-      # Assert that the ed25519 gem has been loaded.
-      #
-      # Returns nothing, raises AlgorithmError.
-      def self.ed25519_gem_required!
-        raise AlgorithmError, "the ed25519 gem is not loaded" unless enabled?
+      def self.asn_algorithm_identifier
+        @@alg_id
       end
 
       def self.algorithm_identifier
@@ -26,10 +21,7 @@ def initialize(algo:, pk:)
         end
 
         @pk = pk
-
-        if self.class.enabled?
-          @ed25519_key = Ed25519::VerifyKey.new(pk)
-        end
+        @openssl = OpenSSL::PKey.read(raw_to_subject_public_key_info_der(pk))
 
         super(algo: algo)
       end
@@ -41,18 +33,12 @@ def initialize(algo:, pk:)
       #
       # Returns boolean.
       def verify(signed_data, signature)
-        self.class.ed25519_gem_required!
-
         sig_algo, raw_sig, _ = Encoding.decode_signature(signature)
         if sig_algo != self.class.algorithm_identifier
           raise DecodeError, "bad signature algorithm: #{sig_algo.inspect}"
         end
 
-        begin
-          ed25519_key.verify(raw_sig, signed_data)
-        rescue Ed25519::VerifyError
-          false
-        end
+        return openssl.verify(nil, raw_sig, signed_data)
       end
 
       # RFC4253 binary encoding of the public key.
@@ -73,6 +59,15 @@ def rfc4253
       def ==(other)
         super && other.pk == pk
       end
+
+      private
+
+      def raw_to_subject_public_key_info_der(key)
+        OpenSSL::ASN1::Sequence([
+          @@alg_id,
+          OpenSSL::ASN1::BitString.new(key)
+        ]).to_der
+      end
     end
   end
 end

lib/ssh_data/public_key/sked25519.rb

@@ -25,7 +25,6 @@ def rfc4253
       end
 
       def verify(signed_data, signature, **opts)
-        self.class.ed25519_gem_required!
         opts = DEFAULT_SK_VERIFY_OPTS.merge(opts)
         unknown_opts = opts.keys - DEFAULT_SK_VERIFY_OPTS.keys
         raise UnsupportedError, "Verification options #{unknown_opts.inspect} are not supported." unless unknown_opts.empty?
@@ -36,12 +35,7 @@ def verify(signed_data, signature, **opts)
           raise DecodeError, "bad signature algorithm: #{sig_algo.inspect}"
         end
 
-        result = begin
-            ed25519_key.verify(raw_sig, blob)
-          rescue Ed25519::VerifyError
-            false
-          end
-
+        result = openssl.verify(nil, raw_sig, blob)
         # We don't know that the flags are correct until after we've validated the signature
         # which embeds the flags, so always verify the signature first.
         return false if opts[:user_presence_required] && (sk_flags & SK_FLAG_USER_PRESENCE != SK_FLAG_USER_PRESENCE)

spec/private_key/ed25519_spec.rb

@@ -1,8 +1,9 @@
 require_relative "../spec_helper"
 
 describe SSHData::PrivateKey::ED25519 do
-  let(:signing_key) { Ed25519::SigningKey.generate }
-  let(:verify_key)  { signing_key.verify_key }
+  let(:signing_key) { OpenSSL::PKey.generate_key("ED25519") }
+  let(:verify_key)  { OpenSSL::PKey.read(signing_key.public_to_pem) }
+
   let(:comment)     { "asdf" }
   let(:message)     { "hello, world!" }
   let(:cert_key)    { SSHData::PrivateKey::DSA.generate.public_key }
@@ -12,8 +13,8 @@
   subject do
     described_class.new(
       algo: SSHData::PublicKey::ALGO_ED25519,
-      pk: verify_key.to_bytes,
-      sk: signing_key.to_bytes + verify_key.to_bytes,
+      pk: verify_key.raw_public_key,
+      sk: signing_key.raw_private_key + verify_key.raw_public_key,
       comment: comment,
     )
   end
@@ -54,22 +55,22 @@
   end
 
   it "has params" do
-    expect(subject.pk).to eq(verify_key.to_bytes)
-    expect(subject.sk).to eq(signing_key.to_bytes + verify_key.to_bytes)
+    expect(subject.pk).to eq(verify_key.raw_public_key)
+    expect(subject.sk).to eq(signing_key.raw_private_key + verify_key.raw_public_key)
   end
 
   it "has a comment" do
     expect(subject.comment).to eq(comment)
   end
 
-  it "has an Ed25519 representation" do
-    expect(subject.ed25519_key).to be_a(Ed25519::SigningKey)
-    expect(subject.ed25519_key.to_bytes).to eq(signing_key.to_bytes)
+  it "has an PKey representation" do
+    expect(subject.openssl).to be_a(OpenSSL::PKey::PKey)
+    expect(subject.openssl.raw_private_key).to eq(signing_key.raw_private_key)
   end
 
   it "has a public key" do
-    expect(subject.public_key).to be_a(SSHData::PublicKey::ED25519)
-    expect(subject.public_key.ed25519_key.to_bytes).to eq(verify_key.to_bytes)
+    expect(subject.openssl).to be_a(OpenSSL::PKey::PKey)
+    expect(subject.public_key.openssl.raw_public_key).to eq(verify_key.raw_public_key)
   end
 
   it "can parse openssh-generate keys" do
@@ -78,27 +79,4 @@
     expect(keys.size).to eq(1)
     expect(keys.first).to be_an(SSHData::PrivateKey::ED25519)
   end
-
-  it "fails cleanly if the ed25519 gem hasn't been loaded" do
-    backup = Object.send(:remove_const, :Ed25519)
-    key = openssh_key.first
-
-    begin
-      expect {
-        expect(key.sk).not_to be_nil
-        expect(key.pk).not_to be_nil
-        expect(key.public_key).not_to be_nil
-      }.not_to raise_error
-
-      expect {
-        described_class.generate
-      }.to raise_error(SSHData::AlgorithmError)
-
-      expect {
-        key.sign(message)
-      }.to raise_error(SSHData::AlgorithmError)
-    ensure
-      Object.const_set(:Ed25519, backup)
-    end
-  end
 end

spec/public_key/ed25519_spec.rb

@@ -1,33 +1,33 @@
 require_relative "../spec_helper"
 
 describe SSHData::PublicKey::ED25519 do
-  let(:signing_key) { Ed25519::SigningKey.generate }
-  let(:verify_key)  { signing_key.verify_key }
+  let(:signing_key) { OpenSSL::PKey.generate_key("ED25519") }
+  let(:verify_key)  { OpenSSL::PKey.read(signing_key.public_to_pem) }
 
   let(:msg)     { "hello, world!" }
-  let(:raw_sig) { signing_key.sign(msg) }
+  let(:raw_sig) { signing_key.sign(nil, msg) }
   let(:sig)     { SSHData::Encoding.encode_signature(SSHData::PublicKey::ALGO_ED25519, raw_sig) }
 
   let(:openssh_key) { SSHData::PublicKey.parse_openssh(fixture("ed25519_leaf_for_rsa_ca.pub")) }
 
   subject do
     described_class.new(
       algo: SSHData::PublicKey::ALGO_ED25519,
-      pk: verify_key.to_bytes
+      pk: verify_key.raw_public_key
     )
   end
 
   it "is equal to keys with the same params" do
     expect(subject).to eq(described_class.new(
       algo: SSHData::PublicKey::ALGO_ED25519,
-      pk: verify_key.to_bytes
+      pk: verify_key.raw_public_key
     ))
   end
 
   it "isnt equal to keys with different params" do
     expect(subject).not_to eq(described_class.new(
       algo: SSHData::PublicKey::ALGO_ED25519,
-      pk: verify_key.to_bytes.reverse
+      pk: verify_key.raw_public_key.reverse
     ))
   end
 
@@ -36,12 +36,12 @@
   end
 
   it "has parameters" do
-    expect(subject.pk).to eq(verify_key.to_bytes)
+    expect(subject.pk).to eq(verify_key.raw_public_key)
   end
 
-  it "has an Ed25519 representation" do
-    expect(subject.ed25519_key).to be_a(Ed25519::VerifyKey)
-    expect(subject.ed25519_key.to_bytes).to eq(verify_key.to_bytes)
+  it "has a pkey representation" do
+    expect(subject.openssl).to be_a(OpenSSL::PKey::PKey)
+    expect(subject.openssl.raw_public_key).to eq(verify_key.raw_public_key)
   end
 
   it "can verify signatures" do
@@ -50,7 +50,7 @@
   end
 
   it "can parse openssh-generate keys" do
-    expect { openssh_key.ed25519_key }.not_to raise_error
+    expect { openssh_key.openssl }.not_to raise_error
   end
 
   it "can be rencoded" do
@@ -64,20 +64,4 @@
       )
     }.not_to raise_error
   end
-
-  it "fails cleanly if the ed25519 gem hasn't been loaded" do
-    expect(described_class.enabled?).to be(true)
-    backup = Object.send(:remove_const, :Ed25519)
-    expect(described_class.enabled?).to be(false)
-
-    begin
-      expect {
-        SSHData::Certificate.parse_openssh(fixture("rsa_leaf_for_ed25519_ca-cert.pub"),
-          unsafe_no_verify: false
-        )
-      }.to raise_error(SSHData::AlgorithmError)
-    ensure
-      Object.const_set(:Ed25519, backup)
-    end
-  end
 end