13615 - The SSL security plugin should use the MCollective::SSL class

The SSL plugin predates the MCollective::SSL class but the intent is
that all crypto in the framework be done in the M::SSL class so that
testing and bug fixes in this critical code is contained.

This commit signature verification logic to the M::SSL class with tests
and adjust the SSL security plugin to use the M::SSL class for its
crypto needs.  The SSL security plugin gains access to the fixes for the
Ruby core SSL key reading bugs via the same

lib/mcollective/ssl.rb

@@ -1,5 +1,6 @@
 require 'openssl'
 require 'base64'
+require 'digest/sha1'
 
 module MCollective
   # A class that assists in encrypting and decrypting data using a
@@ -163,6 +164,20 @@ def aes_decrypt(key, crypt_string)
       decrypted_data = cipher.update(crypt_string) + cipher.final
     end
 
+    # Signs a string using the private key
+    def sign(string, base64=false)
+      sig = @private_key.sign(OpenSSL::Digest::SHA1.new, string)
+
+      base64 ? base64_encode(sig) : sig
+    end
+
+    # Using the public key verifies that a string was signed using the private key
+    def verify_signature(signature, string, base64=false)
+      signature = base64_decode(signature) if base64
+
+      @public_key.verify(OpenSSL::Digest::SHA1.new, signature, string)
+    end
+
     # base 64 encode a string
     def base64_encode(string)
       SSL.base64_encode(string)
@@ -181,6 +196,14 @@ def self.base64_decode(string)
       Base64.decode64(string)
     end
 
+    def md5(string)
+      SSL.md5(string)
+    end
+
+    def self.md5(string)
+      Digest::MD5.hexdigest(string)
+    end
+
     # Reads either a :public or :private key from disk, uses an
     # optional passphrase to read the private key
     def read_key(type, key=nil, passphrase=nil)

plugins/mcollective/security/ssl.rb

@@ -174,9 +174,7 @@ def validrequest?(req)
 
         Log.debug("Validating request from #{req[:callerid]}")
 
-        public_key = File.read(public_key_file(req[:callerid]))
-
-        if verify(public_key, signature, message.to_s)
+        if verify(public_key_file(req[:callerid]), signature, message.to_s)
           @stats.validated
           return true
         else
@@ -308,31 +306,23 @@ def client_cert_dir
       def makehash(body)
         Log.debug("Creating message hash using #{private_key_file}")
 
-        private_key = File.read(private_key_file)
-
-        sign(private_key, body.to_s)
+        sign(private_key_file, body.to_s)
       end
 
       # Code adapted from http://github.com/adamcooke/basicssl
       # signs a message
       def sign(key, string)
-        Base64.encode64(rsakey(key).sign(OpenSSL::Digest::SHA1.new, string))
+        SSL.new(nil, key).sign(string, true)
       end
 
       # verifies a signature
       def verify(key, signature, string)
-        rsakey(key).verify(OpenSSL::Digest::SHA1.new, Base64.decode64(signature), string)
-      end
-
-      # loads a ssl key from file
-      def rsakey(key)
-        OpenSSL::PKey::RSA.new(key)
+        SSL.new(key).verify_signature(signature, string, true)
       end
 
       def request_description(msg)
         "%s from %s@%s" % [msg[:requestid], msg[:callerid], msg[:senderid]]
       end
-
     end
   end
 end

spec/unit/ssl_spec.rb

@@ -168,6 +168,33 @@ module MCollective
       end
     end
 
+    describe "#md5" do
+      it "should produce correct md5 sums" do
+        # echo -n 'hello world'|md5sum
+        @ssl.md5("hello world").should == "5eb63bbbe01eeed093cb22bb8f5acdc3"
+      end
+    end
+    describe "#sign" do
+      it "should sign the message without base64 by default" do
+        SSL.md5(@ssl.sign("hello world")).should == "8269b23f55945aaa82efbff857c845a6"
+      end
+
+      it "should support base64 encoding messages" do
+        SSL.md5(@ssl.sign("hello world", true)).should == "8a4eb3c3d44d22c46dc36a7e441d8db0"
+      end
+    end
+
+    describe "#verify_signature" do
+      it "should correctly verify a message signed using the same keypair" do
+        @ssl.verify_signature(@ssl.sign("hello world"), "hello world").should == true
+        @ssl.verify_signature(@ssl.sign("hello world", true), "hello world", true).should == true
+      end
+
+      it "should fail to verify messages not signed by the key" do
+        @ssl.verify_signature("evil fake signature", "hello world").should == false
+      end
+    end
+
     describe "#decrypt_with_public" do
       it "should decrypt correctly given key and data in base64 format" do
         crypted = {:key=> "YaRcSDdcKgnRZ4Eu2eirl/+lzDgVkPZ41kXAQQNOi+6AfjdbbOW7Zblibx9r\n3TzZAi0ulA94gqNAXPvPC8LaO8W9TtJwlto/RHwDM7ZdfqEImSYoVACFNq28\n+0MLr3K3hIBsB1pyxgFTQul+MrCq+3Fik7Nj7ZKkJUT2veyqbg8=",

website/changelog.md

@@ -11,6 +11,7 @@ title: Changelog
 
 |Date|Description|Ticket|
 |----|-----------|------|
+|2012/04/04|Use the MCollective::SSL utility class for crypto functions in the SSL security plugin|13615|
 |2012/04/02|Support reading public keys from SSL Certificates as well as keys|13534|
 |2012/04/02|Move the help template to the common package for both Debian and RedHat|13434|
 |2012/03/30|Support Stomp 1.2.2 CA verified connection to ActiveMQ|10596|