Msf::Module::UUID#generate_uuid: Replace Rex::Text with SecureRandom.uuid #20170
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Module UUIDs were added in commit 9277f06 titled "Store a uuid for each module, track this in sessions" in 2011. Module UUIDs differ between Metasploit runs, as they are dynamically generated at runtime using
Msf::Module::UUID#generate_uuid
.This method was authored in 2011 when Metasploit contained far fewer modules. Now this method is called approximately 27 thousand (!) times during startup. Due to the ever-increasing number of modules, this behaviour will cause the startup time to grow.
The UUIDs are generated with
Rex::Text.rand_text_alphanumeric(8).downcase
, which does not generate standard UUIDs.metasploit-framework/lib/msf/core/module/uuid.rb
Line 27 in 57032a3
The generated UUID is 8 characters in length (64bit). Most of the key space is wasted, as only 36 values (a-z and 0-9) of 256 are used.
Generating collisions is unlikely, and the impact to startup time is minimal, but the computation is needlessly expensive and wasteful:
rand_text_alphanumeric
returns mixed-case alphanumic characters, but we ultimately discard uppercaseA-Z
rand_text_alphanumeric
constructs an array, then callsRex::Text#rand_base
,[1] which:All we really want is a unique value that is unlikely to cause collisions. If UUID collisions or speed were an issue we could pre-compute values (which has the added benefit of consistency between Metasploit runs).
Instead, this PR replaces
Rex::Text.rand_text_alphanumeric
withSecureRandom.uuid
which is significantly faster (almost 10x 🚀):Benchmarked on a system with 2 cores and 4GB RAM:
This does not introduce an extra dependency as we already use SecureRandom in Framework:
Note: I'm not sure what effect this change will have on Metasploit Pro, if any.
Note: Maybe there is a reason we only want UUIDs to be only 8 characters? If so, we could simply truncate the generated UUID. This would still be much faster than using
Rex::Text
.[1] https://github.com/rapid7/rex-text/blob/0d30d394c4378dbaddf7b489f0d22c2db4024ec7/lib/rex/text/rand.rb#L110-L118
[2] https://github.com/rapid7/rex-text/blob/0d30d394c4378dbaddf7b489f0d22c2db4024ec7/lib/rex/text/rand.rb#L144-L151