15191 - Use UUIDs for request IDs rather than our hacky way

Previously we created our own request ids, these were not great multiple
requests in the same milli second on the same host could cause request
id clashes...not a problem in functionality with how the new message
flow works but it would be a auditing problem to have dupes.

We now use a more reliably random UUID implementation based on OpenSSL
128 bit random numbers and RFC compliant UUID formats

The SSL#uuid function returns standard format UUIDs for either a given
string or based on OpenSSL::BN.rand().  In the Message class we remove
the -s in the UUID thus keeping new and old request ids the same size
and format ensuring everyone who may have audit parsers wont break as a
result of this code

lib/mcollective/message.rb

@@ -233,7 +233,10 @@ def publish
     end
 
     def create_reqid
-      Digest::MD5.hexdigest("#{Config.instance.identity}-#{Time.now.to_f}-#{agent}-#{collective}")
+      # we gsub out the -s so that the format of the id does not
+      # change from previous versions, these should just be more
+      # unique than previous ones
+      SSL.uuid.gsub("-", "")
     end
   end
 end

lib/mcollective/monkey_patches.rb

@@ -59,6 +59,15 @@ def in_groups_of(chunk_size, padded_with=nil, &block)
   end unless method_defined?(:in_groups_of)
 end
 
+class String
+  def bytes(&block)
+    # This should not be necessary, really ...
+    require 'enumerator'
+    return to_enum(:each_byte) unless block_given?
+    each_byte(&block)
+  end unless method_defined?(:bytes)
+end
+
 class Dir
   def self.mktmpdir(prefix_suffix=nil, tmpdir=nil)
     case prefix_suffix

lib/mcollective/ssl.rb

@@ -204,6 +204,39 @@ def self.md5(string)
       Digest::MD5.hexdigest(string)
     end
 
+    # Creates a RFC 4122 version 5 UUID. If string is supplied it will produce repeatable
+    # UUIDs for that string else a random 128bit string will be used from OpenSSL::BN
+    #
+    # Code used with permission from:
+    #    https://github.com/kwilczynski/puppet-functions/blob/master/lib/puppet/parser/functions/uuid.rb
+    #
+    def self.uuid(string=nil)
+      string ||= OpenSSL::Random.random_bytes(16).unpack('H*').shift
+
+      uuid_name_space_dns = "\x6b\xa7\xb8\x10\x9d\xad\x11\xd1\x80\xb4\x00\xc0\x4f\xd4\x30\xc8"
+
+      sha1 = Digest::SHA1.new
+      sha1.update(uuid_name_space_dns)
+      sha1.update(string)
+
+      # first 16 bytes..
+      bytes = sha1.digest[0, 16].bytes.to_a
+
+      # version 5 adjustments
+      bytes[6] &= 0x0f
+      bytes[6] |= 0x50
+
+      # variant is DCE 1.1
+      bytes[8] &= 0x3f
+      bytes[8] |= 0x80
+
+      bytes = [4, 2, 2, 2, 6].collect do |i|
+        bytes.slice!(0, i).pack('C*').unpack('H*')
+      end
+
+      bytes.join('-')
+    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)

spec/unit/message_spec.rb

@@ -212,7 +212,7 @@ module MCollective
         security.expects(:encoderequest).with("identity", 'payload', '123', Util.empty_filter, 'rspec_agent', 'mcollective', 60).twice
         PluginManager.expects("[]").with("security_plugin").returns(security).twice
 
-        Config.instance.expects(:identity).returns("identity").times(4)
+        Config.instance.expects(:identity).returns("identity").twice
 
         Message.any_instance.expects(:requestid).returns("123").twice
 
@@ -414,10 +414,7 @@ module MCollective
       it "should create a valid request id" do
         m = Message.new("msg", "message", :agent => "rspec", :collective => "mc")
 
-        Config.instance.expects(:identity).returns("rspec")
-        Time.expects(:now).returns(1.1)
-
-        Digest::MD5.expects(:hexdigest).with("rspec-1.1-rspec-mc").returns("reqid")
+        SSL.expects(:uuid).returns("reqid")
 
         m.create_reqid.should == "reqid"
       end

spec/unit/ssl_spec.rb

@@ -240,5 +240,15 @@ module MCollective
         }.to raise_error("Crypted data should include data")
       end
     end
+
+    describe "#uuid" do
+      it "should produce repeatable uuids" do
+        SSL.uuid("hello world").should == SSL.uuid("hello world")
+      end
+
+      it "should not always produce the same uuid" do
+        SSL.uuid.should_not == SSL.uuid
+      end
+    end
   end
 end

website/changelog.md

@@ -10,6 +10,7 @@ toc: false
 |----|-----------|------|
 |2012/06/25|Batched RPC requests will now all have the same requestid|15195|
 |2012/06/25|Record the request id on M::Client and in the RPC client stats|15194|
+|2012/06/24|Use UUIDs for the request id rather than our own weak implementation|15191|
 |2012/06/18|The DDL can now define defaults for outputs and the RPC replies are pre-populated|15087|
 |2012/06/18|Remove unused agent help code|15084|
 |2012/06/18|Remove unused code from the *discovery* agent related to inventory and facts|15083|