From 7cab903b02b2c952bf5182650dd70667f4a25360 Mon Sep 17 00:00:00 2001 From: Mathieu Date: Fri, 15 Nov 2024 17:47:17 +0100 Subject: [PATCH 01/11] CredentialCollection: adding various tests that currently fails to demonstrate multiple bugs in the each method --- .../framework/credential_collection_spec.rb | 250 ++++++++++++++++++ 1 file changed, 250 insertions(+) diff --git a/spec/lib/metasploit/framework/credential_collection_spec.rb b/spec/lib/metasploit/framework/credential_collection_spec.rb index 792020ac5eb0..2b843f1763af 100644 --- a/spec/lib/metasploit/framework/credential_collection_spec.rb +++ b/spec/lib/metasploit/framework/credential_collection_spec.rb @@ -98,6 +98,8 @@ Metasploit::Framework::Credential.new(public: "foo", private: "bar"), ) end + + # REMOVE BEFORE COMMIT: question for the userpass file: do we want to make options work with it or not? end context "when given a pass_file and user_file" do @@ -311,6 +313,20 @@ Metasploit::Framework::Credential.new(public: username, private: password), ) end + + context "when using password spraying" do + let(:password_spray) { true } + + # REMOVE BEFORE COMMIT: yields nothings, fails because of bug in method + context "without password" do + let(:password) { nil } + specify do + expect { |b| collection.each(&b) }.to yield_successive_args( + Metasploit::Framework::Credential.new(public: username, private: nil), + ) + end + end + end end context "when :blank_passwords is true" do @@ -323,6 +339,240 @@ end end + context "when given additional_publics" do + let(:username) { nil } + let(:password) { nil } + let(:additional_publics) { [ "test_public" ] } + + context "when :user_as_pass is true" do + let(:user_as_pass) { true } + + # REMOVE BEFORE COMMIT currently failing + specify do + expect { |b| collection.each(&b) }.to yield_successive_args( + Metasploit::Framework::Credential.new(public: "test_public", private: "test_public"), + ) + end + + + context "when using password spraying" do + let(:password_spray) { true } + + # REMOVE BEFORE COMMIT currently failing + specify do + expect { |b| collection.each(&b) }.to yield_successive_args( + Metasploit::Framework::Credential.new(public: "test_public", private: "test_public"), + ) + end + end + end + + context "when :nil_passwords is true" do + let(:nil_passwords) { true } + + # REMOVE BEFORE COMMIT: this option is ignored currently for additional_publics + specify do + expect { |b| collection.each(&b) }.to yield_successive_args( + Metasploit::Framework::Credential.new(public: "test_public", private: nil), + ) + end + end + + context "when using password spraying" do + let(:password_spray) { true } + + context "when :blank_passwords and :nil_password are true" do + let(:blank_passwords) { true } + let(:nil_passwords) { true } + + context "with 2 additional_publics" do + let(:additional_publics) { [ "test_public1", "test_public2" ] } + + # REMOVE BEFORE COMMIT: fails because no pwd spraying + specify do + expect { |b| collection.each(&b) }.to yield_successive_args( + Metasploit::Framework::Credential.new(public: "test_public1", private: ""), + Metasploit::Framework::Credential.new(public: "test_public2", private: ""), + Metasploit::Framework::Credential.new(public: "test_public1", private: nil), + Metasploit::Framework::Credential.new(public: "test_public2", private: nil), + ) + end + end + end + + context "when given a user file" do + let(:user_file) do + filename = "user_file" + stub_file = StringIO.new("asdf\njkl\n") + allow(File).to receive(:open).with(filename, /^r/).and_return stub_file + + filename + end + + # REMOVE BEFORE COMMIT: this also yields the usernames as passwords for the additional_public + context "when given a password" do + let(:password) { "password" } + + specify do + expect { |b| collection.each(&b) }.to yield_successive_args( + Metasploit::Framework::Credential.new(public: "adsf", private: "password"), + Metasploit::Framework::Credential.new(public: "jkl", private: "password"), + Metasploit::Framework::Credential.new(public: "test_public", private: "password"), + ) + end + end + end + end + end + + context "when using password spraying" do + let(:password_spray) { true } + let(:username) { nil } + let(:password) { nil } + + context "when :blank_passwords is true" do + let(:blank_passwords) { true } + + context "with password (but no username)" do + let(:password) { "pass" } + + # REMOVE BEFORE COMMIT: this yields empty creds (no username, no pass) + specify do + expect { |b| collection.each(&b) }.to yield_successive_args() + end + end + + # REMOVE BEFORE COMMIT: yields nothings, fails because of bug in method + context "with username (but no password)" do + let(:username) { "user" } + + specify do + expect { |b| collection.each(&b) }.to yield_successive_args( + Metasploit::Framework::Credential.new(public: username, private: ''), + ) + end + end + + context "when given a user_file" do + let(:user_file) do + filename = "foo" + stub_file = StringIO.new("asdf\njkl\n") + allow(File).to receive(:open).with(filename,/^r/).and_return stub_file + + filename + end + + # REMOVE BEFORE COMMIT: yields nothing, same for blank passwords option + specify do + expect { |b| collection.each(&b) }.to yield_successive_args( + Metasploit::Framework::Credential.new(public: "asdf", private: ''), + Metasploit::Framework::Credential.new(public: "jkl", private: ''), + ) + end + end + end + end + + context "when every possible option is used" do + let(:nil_passwords) { true } + let(:blank_passwords) { true } + let(:username) { "user" } + let(:password) { "pass" } + let(:user_file) do + filename = "user_file" + stub_file = StringIO.new("userfile") + allow(File).to receive(:open).with(filename,/^r/).and_yield stub_file + + filename + end + let(:pass_file) do + filename = "pass_file" + stub_file = StringIO.new("passfile\n") + allow(File).to receive(:open).with(filename,/^r/).and_return stub_file + + filename + end + let(:user_as_pass) { false } + let(:userpass_file) do + filename = "userpass_file" + stub_file = StringIO.new("userpass_user userpass_pass\n") + allow(File).to receive(:open).with(filename,/^r/).and_yield stub_file + + filename + end + let(:prepended_creds) { ['test_prepend'] } + let(:additional_privates) { ['test_private'] } + let(:additional_publics) { ['test_public'] } + + # REMOVE BEFORE COMMIT: fails because of the useraspass error, then fails because of the nil value for addiitonal publics and should be ok then + specify do + expect { |b| collection.each(&b) }.to yield_successive_args( + "test_prepend", + Metasploit::Framework::Credential.new(public: "user", private: nil), + Metasploit::Framework::Credential.new(public: "user", private: "pass"), + Metasploit::Framework::Credential.new(public: "user", private: "user"), + Metasploit::Framework::Credential.new(public: "user", private: ""), + Metasploit::Framework::Credential.new(public: "user", private: "passfile"), + Metasploit::Framework::Credential.new(public: "user", private: "test_private"), + Metasploit::Framework::Credential.new(public: "userfile", private: nil), + Metasploit::Framework::Credential.new(public: "userfile", private: "pass"), + Metasploit::Framework::Credential.new(public: "userfile", private: "userfile"), + Metasploit::Framework::Credential.new(public: "userfile", private: ""), + Metasploit::Framework::Credential.new(public: "userfile", private: "passfile"), + Metasploit::Framework::Credential.new(public: "userfile", private: "test_private"), + Metasploit::Framework::Credential.new(public: "userpass_user", private: "userpass_pass"), + Metasploit::Framework::Credential.new(public: "test_public", private: nil), # missing this case + Metasploit::Framework::Credential.new(public: "test_public", private: "pass"), + Metasploit::Framework::Credential.new(public: "test_public", private: "test_public"), + Metasploit::Framework::Credential.new(public: "test_public", private: ""), + Metasploit::Framework::Credential.new(public: "test_public", private: "passfile"), + Metasploit::Framework::Credential.new(public: "test_public", private: "test_private") + ) + end + + context "when using password spraying" do + let(:password_spray) { true } + let(:user_file) do + filename = "user_file" + stub_file = StringIO.new("userfile") + allow(File).to receive(:open).with(filename,/^r/).and_return stub_file + + filename + end + let(:pass_file) do + filename = "pass_file" + stub_file = StringIO.new("passfile\n") + allow(File).to receive(:open).with(filename,/^r/).and_yield stub_file + + filename + end + + specify do + expect { |b| collection.each(&b) }.to yield_successive_args( + "test_prepend", + Metasploit::Framework::Credential.new(public: "user", private: nil), + Metasploit::Framework::Credential.new(public: "userfile", private: nil), + Metasploit::Framework::Credential.new(public: "test_public", private: nil), + Metasploit::Framework::Credential.new(public: "user", private: "pass"), + Metasploit::Framework::Credential.new(public: "userfile", private: "pass"), + Metasploit::Framework::Credential.new(public: "test_public", private: "pass"), + Metasploit::Framework::Credential.new(public: "user", private: "user"), + Metasploit::Framework::Credential.new(public: "userfile", private: "userfile"), + Metasploit::Framework::Credential.new(public: "test_public", private: "test_public"), + Metasploit::Framework::Credential.new(public: "user", private: ""), + Metasploit::Framework::Credential.new(public: "userfile", private: ""), + Metasploit::Framework::Credential.new(public: "test_public", private: ""), + Metasploit::Framework::Credential.new(public: "user", private: "passfile"), + Metasploit::Framework::Credential.new(public: "userfile", private: "passfile"), + Metasploit::Framework::Credential.new(public: "test_public", private: "passfile"), + Metasploit::Framework::Credential.new(public: "userpass_user", private: "userpass_pass"), + Metasploit::Framework::Credential.new(public: "user", private: "test_private"), + Metasploit::Framework::Credential.new(public: "userfile", private: "test_private"), + Metasploit::Framework::Credential.new(public: "test_public", private: "test_private"), + ) + end + end + end end describe "#empty?" do From 5d7d8cfe3688283179e840eb6e9eec822d6807a2 Mon Sep 17 00:00:00 2001 From: Mathieu Date: Mon, 18 Nov 2024 15:38:17 +0100 Subject: [PATCH 02/11] Fixes trivial bugs when combining additional_publics with user_as_pass --- lib/metasploit/framework/credential_collection.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/metasploit/framework/credential_collection.rb b/lib/metasploit/framework/credential_collection.rb index 57f1f7726cfb..761dcea036f8 100644 --- a/lib/metasploit/framework/credential_collection.rb +++ b/lib/metasploit/framework/credential_collection.rb @@ -360,7 +360,7 @@ def each_unfiltered_password_first yield Metasploit::Framework::Credential.new(public: add_public, private: password, realm: realm, private_type: private_type(password) ) end if user_as_pass - yield Metasploit::Framework::Credential.new(public: add_public, private: user_from_file, realm: realm, private_type: :password) + yield Metasploit::Framework::Credential.new(public: add_public, private: add_public, realm: realm, private_type: :password) end if blank_passwords yield Metasploit::Framework::Credential.new(public: add_public, private: "", realm: realm, private_type: :password) @@ -479,7 +479,7 @@ def each_unfiltered_username_first yield Metasploit::Framework::Credential.new(public: add_public, private: password, realm: realm, private_type: private_type(password) ) end if user_as_pass - yield Metasploit::Framework::Credential.new(public: add_public, private: user_from_file, realm: realm, private_type: :password) + yield Metasploit::Framework::Credential.new(public: add_public, private: add_public, realm: realm, private_type: :password) end if blank_passwords yield Metasploit::Framework::Credential.new(public: add_public, private: "", realm: realm, private_type: :password) From 397f4666d8bbe7266d7a1baa811e70582a3cdf6d Mon Sep 17 00:00:00 2001 From: Mathieu Date: Mon, 18 Nov 2024 15:48:09 +0100 Subject: [PATCH 03/11] Fixes the fact that no nil credential is generated for additional_publics --- lib/metasploit/framework/credential_collection.rb | 3 +++ spec/lib/metasploit/framework/credential_collection_spec.rb | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/lib/metasploit/framework/credential_collection.rb b/lib/metasploit/framework/credential_collection.rb index 761dcea036f8..d640f411356a 100644 --- a/lib/metasploit/framework/credential_collection.rb +++ b/lib/metasploit/framework/credential_collection.rb @@ -475,6 +475,9 @@ def each_unfiltered_username_first end additional_publics.each do |add_public| + if nil_passwords + yield Metasploit::Framework::Credential.new(public: add_public, private: nil, realm: realm, private_type: :password) + end if password.present? yield Metasploit::Framework::Credential.new(public: add_public, private: password, realm: realm, private_type: private_type(password) ) end diff --git a/spec/lib/metasploit/framework/credential_collection_spec.rb b/spec/lib/metasploit/framework/credential_collection_spec.rb index 2b843f1763af..cf1dcbae4164 100644 --- a/spec/lib/metasploit/framework/credential_collection_spec.rb +++ b/spec/lib/metasploit/framework/credential_collection_spec.rb @@ -504,7 +504,7 @@ let(:additional_privates) { ['test_private'] } let(:additional_publics) { ['test_public'] } - # REMOVE BEFORE COMMIT: fails because of the useraspass error, then fails because of the nil value for addiitonal publics and should be ok then + # REMOVE BEFORE COMMIT: fails because of the useraspass error, then fails because of the nil value for addittonal publics and should be ok then specify do expect { |b| collection.each(&b) }.to yield_successive_args( "test_prepend", From 5fce8d2bf7226825c2534c53f750863327923f97 Mon Sep 17 00:00:00 2001 From: Mathieu Date: Mon, 18 Nov 2024 16:09:18 +0100 Subject: [PATCH 04/11] Fix incorrect expectations of currently failing tests --- .../metasploit/framework/credential_collection_spec.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/spec/lib/metasploit/framework/credential_collection_spec.rb b/spec/lib/metasploit/framework/credential_collection_spec.rb index cf1dcbae4164..7de4a3f2132a 100644 --- a/spec/lib/metasploit/framework/credential_collection_spec.rb +++ b/spec/lib/metasploit/framework/credential_collection_spec.rb @@ -391,10 +391,10 @@ # REMOVE BEFORE COMMIT: fails because no pwd spraying specify do expect { |b| collection.each(&b) }.to yield_successive_args( - Metasploit::Framework::Credential.new(public: "test_public1", private: ""), - Metasploit::Framework::Credential.new(public: "test_public2", private: ""), Metasploit::Framework::Credential.new(public: "test_public1", private: nil), Metasploit::Framework::Credential.new(public: "test_public2", private: nil), + Metasploit::Framework::Credential.new(public: "test_public1", private: ""), + Metasploit::Framework::Credential.new(public: "test_public2", private: ""), ) end end @@ -415,7 +415,7 @@ specify do expect { |b| collection.each(&b) }.to yield_successive_args( - Metasploit::Framework::Credential.new(public: "adsf", private: "password"), + Metasploit::Framework::Credential.new(public: "asdf", private: "password"), Metasploit::Framework::Credential.new(public: "jkl", private: "password"), Metasploit::Framework::Credential.new(public: "test_public", private: "password"), ) @@ -492,7 +492,7 @@ filename end - let(:user_as_pass) { false } + let(:user_as_pass) { true } let(:userpass_file) do filename = "userpass_file" stub_file = StringIO.new("userpass_user userpass_pass\n") From 383dc0da32c6f0fc3be8917ca5fb46456e7e0247 Mon Sep 17 00:00:00 2001 From: Mathieu Date: Mon, 18 Nov 2024 16:11:36 +0100 Subject: [PATCH 05/11] Re-implement the each_unfiltered_password_first method (used in case of password spraying) to make all tests pass --- .../framework/credential_collection.rb | 98 ++++++++++++------- 1 file changed, 63 insertions(+), 35 deletions(-) diff --git a/lib/metasploit/framework/credential_collection.rb b/lib/metasploit/framework/credential_collection.rb index d640f411356a..8bab6b88053d 100644 --- a/lib/metasploit/framework/credential_collection.rb +++ b/lib/metasploit/framework/credential_collection.rb @@ -280,52 +280,98 @@ def each_unfiltered_password_first yield Metasploit::Framework::Credential.new(public: '', private: '', realm: realm, private_type: :password) end - if user_as_pass + if nil_passwords + if username.present? + yield Metasploit::Framework::Credential.new(public: username, private: nil, realm: realm, private_type: :password) + end + if user_fd user_fd.each_line do |user_from_file| user_from_file.chomp! - yield Metasploit::Framework::Credential.new(public: user_from_file, private: user_from_file, realm: realm, private_type: private_type(password)) + yield Metasploit::Framework::Credential.new(public: user_from_file, private: nil, realm: realm, private_type: :password) end user_fd.seek(0) end + + additional_publics.each do |add_public| + yield Metasploit::Framework::Credential.new(public: add_public, private: nil, realm: realm, private_type: :password) + end end if password.present? - if nil_passwords - yield Metasploit::Framework::Credential.new(public: username, private: nil, realm: realm, private_type: :password) - end if username.present? yield Metasploit::Framework::Credential.new(public: username, private: password, realm: realm, private_type: private_type(password)) end - if user_as_pass + + if user_fd + user_fd.each_line do |user_from_file| + user_from_file.chomp! + yield Metasploit::Framework::Credential.new(public: user_from_file, private: password, realm: realm, private_type: private_type(password)) + end + user_fd.seek(0) + end + + additional_publics.each do |add_public| + yield Metasploit::Framework::Credential.new(public: add_public, private: password, realm: realm, private_type: private_type(password)) + end + end + + if user_as_pass + if username.present? yield Metasploit::Framework::Credential.new(public: username, private: username, realm: realm, private_type: :password) end - if blank_passwords + + if user_fd + user_fd.each_line do |user_from_file| + user_from_file.chomp! + yield Metasploit::Framework::Credential.new(public: user_from_file, private: user_from_file, realm: realm, private_type: private_type(password)) + end + user_fd.seek(0) + end + + additional_publics.each do |add_public| + yield Metasploit::Framework::Credential.new(public: add_public, private: add_public, realm: realm, private_type: :password) + end + end + + if blank_passwords + if username.present? yield Metasploit::Framework::Credential.new(public: username, private: "", realm: realm, private_type: :password) end + if user_fd user_fd.each_line do |user_from_file| user_from_file.chomp! - yield Metasploit::Framework::Credential.new(public: user_from_file, private: password, realm: realm, private_type: private_type(password)) + yield Metasploit::Framework::Credential.new(public: user_from_file, private: "", realm: realm, private_type: :password) end user_fd.seek(0) end + + additional_publics.each do |add_public| + yield Metasploit::Framework::Credential.new(public: add_public, private: "", realm: realm, private_type: :password) + end end if pass_file.present? File.open(pass_file, 'r:binary') do |pass_fd| pass_fd.each_line do |pass_from_file| pass_from_file.chomp! + if username.present? - yield Metasploit::Framework::Credential.new(public: username, private: pass_from_file, realm: realm, private_type: :password) + yield Metasploit::Framework::Credential.new(public: username, private: pass_from_file, realm: realm, private_type: private_type(pass_from_file)) + end + + if user_fd + user_fd.each_line do |user_from_file| + user_from_file.chomp! + yield Metasploit::Framework::Credential.new(public: user_from_file, private: pass_from_file, realm: realm, private_type: private_type(pass_from_file)) + end + user_fd.seek(0) end - next unless user_fd - user_fd.each_line do |user_from_file| - user_from_file.chomp! - yield Metasploit::Framework::Credential.new(public: user_from_file, private: pass_from_file, realm: realm, private_type: private_type(pass_from_file)) + additional_publics.each do |add_public| + yield Metasploit::Framework::Credential.new(public: add_public, private: pass_from_file, realm: realm, private_type: private_type(pass_from_file)) end - user_fd.seek(0) end end end @@ -348,34 +394,16 @@ def each_unfiltered_password_first if username.present? yield Metasploit::Framework::Credential.new(public: username, private: add_private, realm: realm, private_type: private_type(add_private)) end - user_fd.each_line do |user_from_file| - user_from_file.chomp! - yield Metasploit::Framework::Credential.new(public: user_from_file, private: add_private, realm: realm, private_type: private_type(add_private)) - end - user_fd.seek(0) - end - additional_publics.each do |add_public| - if password.present? - yield Metasploit::Framework::Credential.new(public: add_public, private: password, realm: realm, private_type: private_type(password) ) - end - if user_as_pass - yield Metasploit::Framework::Credential.new(public: add_public, private: add_public, realm: realm, private_type: :password) - end - if blank_passwords - yield Metasploit::Framework::Credential.new(public: add_public, private: "", realm: realm, private_type: :password) - end - if nil_passwords - yield Metasploit::Framework::Credential.new(public: add_public, private: nil, realm: realm, private_type: :password) - end if user_fd user_fd.each_line do |user_from_file| user_from_file.chomp! - yield Metasploit::Framework::Credential.new(public: add_public, private: user_from_file, realm: realm, private_type: private_type(user_from_file)) + yield Metasploit::Framework::Credential.new(public: user_from_file, private: add_private, realm: realm, private_type: private_type(add_private)) end user_fd.seek(0) end - additional_privates.each do |add_private| + + additional_publics.each do |add_public| yield Metasploit::Framework::Credential.new(public: add_public, private: add_private, realm: realm, private_type: private_type(add_private)) end end From e03f2e2ea7b4eac85d589276d96d5f358a576621 Mon Sep 17 00:00:00 2001 From: Mathieu Date: Fri, 13 Dec 2024 17:13:56 +0100 Subject: [PATCH 06/11] Inline all specs to avoid nesting context --- .../framework/credential_collection_spec.rb | 385 ++++++++++-------- 1 file changed, 207 insertions(+), 178 deletions(-) diff --git a/spec/lib/metasploit/framework/credential_collection_spec.rb b/spec/lib/metasploit/framework/credential_collection_spec.rb index 7de4a3f2132a..dcb16827964e 100644 --- a/spec/lib/metasploit/framework/credential_collection_spec.rb +++ b/spec/lib/metasploit/framework/credential_collection_spec.rb @@ -68,7 +68,7 @@ let(:pass_file) do filename = "foo" stub_file = StringIO.new("asdf\njkl\n") - allow(File).to receive(:open).with(filename,/^r/).and_return stub_file + allow(File).to receive(:open).with(filename,/^r/).and_yield stub_file filename end @@ -81,7 +81,7 @@ end end - context "when given a userspass_file" do + context "when given a userpass_file" do let(:username) { nil } let(:password) { nil } let(:userpass_file) do @@ -98,8 +98,6 @@ Metasploit::Framework::Credential.new(public: "foo", private: "bar"), ) end - - # REMOVE BEFORE COMMIT: question for the userpass file: do we want to make options work with it or not? end context "when given a pass_file and user_file" do @@ -115,7 +113,7 @@ let(:pass_file) do filename = "pass_file" stub_file = StringIO.new("asdf\njkl\n") - allow(File).to receive(:open).with(filename,/^r/).and_return stub_file + allow(File).to receive(:open).with(filename,/^r/).and_yield stub_file filename end @@ -144,7 +142,7 @@ let(:user_file) do filename = "user_file" stub_file = StringIO.new("user1\nuser2\nuser3\n") - allow(File).to receive(:open).with(filename,/^r/).and_return stub_file + allow(File).to receive(:open).with(filename,/^r/).and_yield stub_file filename end @@ -159,23 +157,40 @@ Metasploit::Framework::Credential.new(public: "user3", private: "password2"), ) end + end - context 'when :user_as_pass is true' do - let(:user_as_pass) { true } + context 'when given a pass_file and user_file and password spray and :user_as_pass is true' do + let(:password) { nil } + let(:username) { nil } + let(:password_spray) { true } + let(:pass_file) do + filename = "pass_file" + stub_file = StringIO.new("password1\npassword2\n") + allow(File).to receive(:open).with(filename,/^r/).and_yield stub_file - specify do - expect { |b| collection.each(&b) }.to yield_successive_args( - Metasploit::Framework::Credential.new(public: "user1", private: "user1"), - Metasploit::Framework::Credential.new(public: "user2", private: "user2"), - Metasploit::Framework::Credential.new(public: "user3", private: "user3"), - Metasploit::Framework::Credential.new(public: "user1", private: "password1"), - Metasploit::Framework::Credential.new(public: "user2", private: "password1"), - Metasploit::Framework::Credential.new(public: "user3", private: "password1"), - Metasploit::Framework::Credential.new(public: "user1", private: "password2"), - Metasploit::Framework::Credential.new(public: "user2", private: "password2"), - Metasploit::Framework::Credential.new(public: "user3", private: "password2"), - ) - end + filename + end + let(:user_file) do + filename = "user_file" + stub_file = StringIO.new("user1\nuser2\nuser3\n") + allow(File).to receive(:open).with(filename,/^r/).and_yield stub_file + + filename + end + let(:user_as_pass) { true } + + specify do + expect { |b| collection.each(&b) }.to yield_successive_args( + Metasploit::Framework::Credential.new(public: "user1", private: "user1"), + Metasploit::Framework::Credential.new(public: "user2", private: "user2"), + Metasploit::Framework::Credential.new(public: "user3", private: "user3"), + Metasploit::Framework::Credential.new(public: "user1", private: "password1"), + Metasploit::Framework::Credential.new(public: "user2", private: "password1"), + Metasploit::Framework::Credential.new(public: "user3", private: "password1"), + Metasploit::Framework::Credential.new(public: "user1", private: "password2"), + Metasploit::Framework::Credential.new(public: "user2", private: "password2"), + Metasploit::Framework::Credential.new(public: "user3", private: "password2"), + ) end end @@ -205,7 +220,7 @@ let(:user_file) do filename = "user_file" stub_file = StringIO.new("user1\nuser2\nuser3\n") - allow(File).to receive(:open).with(filename,/^r/).and_return stub_file + allow(File).to receive(:open).with(filename,/^r/).and_yield stub_file filename end @@ -240,7 +255,7 @@ let(:user_file) do filename = "user_file" stub_file = StringIO.new("user1\nuser2\nuser3\n") - allow(File).to receive(:open).with(filename,/^r/).and_return stub_file + allow(File).to receive(:open).with(filename,/^r/).and_yield stub_file filename end @@ -278,7 +293,7 @@ let(:pass_file) do filename = "pass_file" stub_file = StringIO.new("asdf\njkl\n") - allow(File).to receive(:open).with(filename, /^r/).and_return stub_file + allow(File).to receive(:open).with(filename, /^r/).and_yield stub_file filename end @@ -313,18 +328,19 @@ Metasploit::Framework::Credential.new(public: username, private: password), ) end + end - context "when using password spraying" do - let(:password_spray) { true } + context "when using password spraying and :nil_passwords is true" do + let(:password_spray) { true } + let(:nil_passwords) { true } - # REMOVE BEFORE COMMIT: yields nothings, fails because of bug in method - context "without password" do - let(:password) { nil } - specify do - expect { |b| collection.each(&b) }.to yield_successive_args( - Metasploit::Framework::Credential.new(public: username, private: nil), - ) - end + # REMOVE BEFORE COMMIT: yields nothings, fails because of bug in method + context "without password" do + let(:password) { nil } + specify do + expect { |b| collection.each(&b) }.to yield_successive_args( + Metasploit::Framework::Credential.new(public: username, private: nil), + ) end end end @@ -339,137 +355,135 @@ end end - context "when given additional_publics" do + context "when given additional_publics and :user_as_pass is true" do let(:username) { nil } let(:password) { nil } let(:additional_publics) { [ "test_public" ] } + let(:user_as_pass) { true } - context "when :user_as_pass is true" do - let(:user_as_pass) { true } - - # REMOVE BEFORE COMMIT currently failing - specify do - expect { |b| collection.each(&b) }.to yield_successive_args( - Metasploit::Framework::Credential.new(public: "test_public", private: "test_public"), - ) - end - + # REMOVE BEFORE COMMIT currently failing + specify do + expect { |b| collection.each(&b) }.to yield_successive_args( + Metasploit::Framework::Credential.new(public: "test_public", private: "test_public"), + ) + end + end - context "when using password spraying" do - let(:password_spray) { true } + context "when given additional_publics, :user_as_pass is true and using password spraying" do + let(:username) { nil } + let(:password) { nil } + let(:additional_publics) { [ "test_public" ] } + let(:user_as_pass) { true } + let(:password_spray) { true } - # REMOVE BEFORE COMMIT currently failing - specify do - expect { |b| collection.each(&b) }.to yield_successive_args( - Metasploit::Framework::Credential.new(public: "test_public", private: "test_public"), - ) - end - end + # REMOVE BEFORE COMMIT currently failing + specify do + expect { |b| collection.each(&b) }.to yield_successive_args( + Metasploit::Framework::Credential.new(public: "test_public", private: "test_public"), + ) end + end - context "when :nil_passwords is true" do - let(:nil_passwords) { true } + context "when given additional_publics and :nil_password is true" do + let(:username) { nil } + let(:password) { nil } + let(:additional_publics) { [ "test_public" ] } + let(:nil_passwords) { true } - # REMOVE BEFORE COMMIT: this option is ignored currently for additional_publics - specify do - expect { |b| collection.each(&b) }.to yield_successive_args( - Metasploit::Framework::Credential.new(public: "test_public", private: nil), - ) - end + # REMOVE BEFORE COMMIT: this option is ignored currently for additional_publics + specify do + expect { |b| collection.each(&b) }.to yield_successive_args( + Metasploit::Framework::Credential.new(public: "test_public", private: nil), + ) end + end - context "when using password spraying" do - let(:password_spray) { true } - - context "when :blank_passwords and :nil_password are true" do - let(:blank_passwords) { true } - let(:nil_passwords) { true } + context "when given additional_publics, :nil_password is true, :blank_passwords is true and using password spraying" do + let(:username) { nil } + let(:password) { nil } + let(:additional_publics) { [ "test_public1", "test_public2" ] } + let(:nil_passwords) { true } + let(:blank_passwords) { true } + let(:password_spray) { true } - context "with 2 additional_publics" do - let(:additional_publics) { [ "test_public1", "test_public2" ] } - - # REMOVE BEFORE COMMIT: fails because no pwd spraying - specify do - expect { |b| collection.each(&b) }.to yield_successive_args( - Metasploit::Framework::Credential.new(public: "test_public1", private: nil), - Metasploit::Framework::Credential.new(public: "test_public2", private: nil), - Metasploit::Framework::Credential.new(public: "test_public1", private: ""), - Metasploit::Framework::Credential.new(public: "test_public2", private: ""), - ) - end - end - end + # REMOVE BEFORE COMMIT: fails because no pwd spraying + specify do + expect { |b| collection.each(&b) }.to yield_successive_args( + Metasploit::Framework::Credential.new(public: "test_public1", private: nil), + Metasploit::Framework::Credential.new(public: "test_public2", private: nil), + Metasploit::Framework::Credential.new(public: "test_public1", private: ""), + Metasploit::Framework::Credential.new(public: "test_public2", private: ""), + ) + end + end - context "when given a user file" do - let(:user_file) do - filename = "user_file" - stub_file = StringIO.new("asdf\njkl\n") - allow(File).to receive(:open).with(filename, /^r/).and_return stub_file + context "when given additional_publics, a user_file, a password and using password spraying" do + let(:username) { nil } + let(:password) { "password" } + let(:additional_publics) { [ "test_public" ] } + let(:user_file) do + filename = "user_file" + stub_file = StringIO.new("asdf\njkl\n") + allow(File).to receive(:open).with(filename, /^r/).and_yield stub_file - filename - end + filename + end - # REMOVE BEFORE COMMIT: this also yields the usernames as passwords for the additional_public - context "when given a password" do - let(:password) { "password" } - - specify do - expect { |b| collection.each(&b) }.to yield_successive_args( - Metasploit::Framework::Credential.new(public: "asdf", private: "password"), - Metasploit::Framework::Credential.new(public: "jkl", private: "password"), - Metasploit::Framework::Credential.new(public: "test_public", private: "password"), - ) - end - end - end + # REMOVE BEFORE COMMIT: this also yields the usernames as passwords for the additional_public + specify do + expect { |b| collection.each(&b) }.to yield_successive_args( + Metasploit::Framework::Credential.new(public: "asdf", private: "password"), + Metasploit::Framework::Credential.new(public: "jkl", private: "password"), + Metasploit::Framework::Credential.new(public: "test_public", private: "password"), + ) end end - context "when using password spraying" do + context "when using password spraying with blank_passwords and a password (but no username)" do let(:password_spray) { true } + let(:blank_passwords) { true } let(:username) { nil } - let(:password) { nil } - - context "when :blank_passwords is true" do - let(:blank_passwords) { true } - - context "with password (but no username)" do - let(:password) { "pass" } + let(:password) { "pass" } - # REMOVE BEFORE COMMIT: this yields empty creds (no username, no pass) - specify do - expect { |b| collection.each(&b) }.to yield_successive_args() - end - end + # REMOVE BEFORE COMMIT: this yields empty creds (no username, no pass) + specify do + expect { |b| collection.each(&b) }.to yield_successive_args() + end + end - # REMOVE BEFORE COMMIT: yields nothings, fails because of bug in method - context "with username (but no password)" do - let(:username) { "user" } + context "when using password spraying with blank_passwords and a username (but no password)" do + let(:password_spray) { true } + let(:blank_passwords) { true } + let(:username) { "user" } + let(:password) { nil } - specify do - expect { |b| collection.each(&b) }.to yield_successive_args( - Metasploit::Framework::Credential.new(public: username, private: ''), - ) - end - end + # REMOVE BEFORE COMMIT: this yields empty creds (no username, no pass) + specify do + expect { |b| collection.each(&b) }.to yield_successive_args( + Metasploit::Framework::Credential.new(public: username, private: ''), + ) + end + end - context "when given a user_file" do - let(:user_file) do - filename = "foo" - stub_file = StringIO.new("asdf\njkl\n") - allow(File).to receive(:open).with(filename,/^r/).and_return stub_file + context "when using password spraying with blank_passwords and given a user_file" do + let(:password_spray) { true } + let(:blank_passwords) { true } + let(:username) { nil } + let(:password) { nil } + let(:user_file) do + filename = "foo" + stub_file = StringIO.new("asdf\njkl\n") + allow(File).to receive(:open).with(filename,/^r/).and_yield stub_file - filename - end + filename + end - # REMOVE BEFORE COMMIT: yields nothing, same for blank passwords option - specify do - expect { |b| collection.each(&b) }.to yield_successive_args( - Metasploit::Framework::Credential.new(public: "asdf", private: ''), - Metasploit::Framework::Credential.new(public: "jkl", private: ''), - ) - end - end + # REMOVE BEFORE COMMIT: yields nothing, same for blank passwords option + specify do + expect { |b| collection.each(&b) }.to yield_successive_args( + Metasploit::Framework::Credential.new(public: "asdf", private: ''), + Metasploit::Framework::Credential.new(public: "jkl", private: ''), + ) end end @@ -488,7 +502,7 @@ let(:pass_file) do filename = "pass_file" stub_file = StringIO.new("passfile\n") - allow(File).to receive(:open).with(filename,/^r/).and_return stub_file + allow(File).to receive(:open).with(filename,/^r/).and_yield stub_file filename end @@ -529,48 +543,63 @@ Metasploit::Framework::Credential.new(public: "test_public", private: "test_private") ) end + end - context "when using password spraying" do - let(:password_spray) { true } - let(:user_file) do - filename = "user_file" - stub_file = StringIO.new("userfile") - allow(File).to receive(:open).with(filename,/^r/).and_return stub_file + context "when using password spraying in combination with every other option" do + let(:password_spray) { true } + let(:nil_passwords) { true } + let(:blank_passwords) { true } + let(:username) { "user" } + let(:password) { "pass" } + let(:user_file) do + filename = "user_file" + stub_file = StringIO.new("userfile") + allow(File).to receive(:open).with(filename,/^r/).and_yield stub_file - filename - end - let(:pass_file) do - filename = "pass_file" - stub_file = StringIO.new("passfile\n") - allow(File).to receive(:open).with(filename,/^r/).and_yield stub_file + filename + end + let(:pass_file) do + filename = "pass_file" + stub_file = StringIO.new("passfile\n") + allow(File).to receive(:open).with(filename,/^r/).and_yield stub_file - filename - end + filename + end + let(:user_as_pass) { true } + let(:userpass_file) do + filename = "userpass_file" + stub_file = StringIO.new("userpass_user userpass_pass\n") + allow(File).to receive(:open).with(filename,/^r/).and_yield stub_file - specify do - expect { |b| collection.each(&b) }.to yield_successive_args( - "test_prepend", - Metasploit::Framework::Credential.new(public: "user", private: nil), - Metasploit::Framework::Credential.new(public: "userfile", private: nil), - Metasploit::Framework::Credential.new(public: "test_public", private: nil), - Metasploit::Framework::Credential.new(public: "user", private: "pass"), - Metasploit::Framework::Credential.new(public: "userfile", private: "pass"), - Metasploit::Framework::Credential.new(public: "test_public", private: "pass"), - Metasploit::Framework::Credential.new(public: "user", private: "user"), - Metasploit::Framework::Credential.new(public: "userfile", private: "userfile"), - Metasploit::Framework::Credential.new(public: "test_public", private: "test_public"), - Metasploit::Framework::Credential.new(public: "user", private: ""), - Metasploit::Framework::Credential.new(public: "userfile", private: ""), - Metasploit::Framework::Credential.new(public: "test_public", private: ""), - Metasploit::Framework::Credential.new(public: "user", private: "passfile"), - Metasploit::Framework::Credential.new(public: "userfile", private: "passfile"), - Metasploit::Framework::Credential.new(public: "test_public", private: "passfile"), - Metasploit::Framework::Credential.new(public: "userpass_user", private: "userpass_pass"), - Metasploit::Framework::Credential.new(public: "user", private: "test_private"), - Metasploit::Framework::Credential.new(public: "userfile", private: "test_private"), - Metasploit::Framework::Credential.new(public: "test_public", private: "test_private"), - ) - end + filename + end + let(:prepended_creds) { ['test_prepend'] } + let(:additional_privates) { ['test_private'] } + let(:additional_publics) { ['test_public'] } + + specify do + expect { |b| collection.each(&b) }.to yield_successive_args( + "test_prepend", + Metasploit::Framework::Credential.new(public: "user", private: nil), + Metasploit::Framework::Credential.new(public: "userfile", private: nil), + Metasploit::Framework::Credential.new(public: "test_public", private: nil), + Metasploit::Framework::Credential.new(public: "user", private: "pass"), + Metasploit::Framework::Credential.new(public: "userfile", private: "pass"), + Metasploit::Framework::Credential.new(public: "test_public", private: "pass"), + Metasploit::Framework::Credential.new(public: "user", private: "user"), + Metasploit::Framework::Credential.new(public: "userfile", private: "userfile"), + Metasploit::Framework::Credential.new(public: "test_public", private: "test_public"), + Metasploit::Framework::Credential.new(public: "user", private: ""), + Metasploit::Framework::Credential.new(public: "userfile", private: ""), + Metasploit::Framework::Credential.new(public: "test_public", private: ""), + Metasploit::Framework::Credential.new(public: "user", private: "passfile"), + Metasploit::Framework::Credential.new(public: "userfile", private: "passfile"), + Metasploit::Framework::Credential.new(public: "test_public", private: "passfile"), + Metasploit::Framework::Credential.new(public: "userpass_user", private: "userpass_pass"), + Metasploit::Framework::Credential.new(public: "user", private: "test_private"), + Metasploit::Framework::Credential.new(public: "userfile", private: "test_private"), + Metasploit::Framework::Credential.new(public: "test_public", private: "test_private"), + ) end end end From b2cc86381ed8ed39414034e451a4108838636fb2 Mon Sep 17 00:00:00 2001 From: cgranleese-r7 Date: Tue, 28 Jan 2025 14:30:35 +0000 Subject: [PATCH 07/11] Reverts some test expectations --- .../framework/credential_collection_spec.rb | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/spec/lib/metasploit/framework/credential_collection_spec.rb b/spec/lib/metasploit/framework/credential_collection_spec.rb index dcb16827964e..55b4bc2b12bc 100644 --- a/spec/lib/metasploit/framework/credential_collection_spec.rb +++ b/spec/lib/metasploit/framework/credential_collection_spec.rb @@ -68,7 +68,7 @@ let(:pass_file) do filename = "foo" stub_file = StringIO.new("asdf\njkl\n") - allow(File).to receive(:open).with(filename,/^r/).and_yield stub_file + allow(File).to receive(:open).with(filename,/^r/).and_return stub_file filename end @@ -113,7 +113,7 @@ let(:pass_file) do filename = "pass_file" stub_file = StringIO.new("asdf\njkl\n") - allow(File).to receive(:open).with(filename,/^r/).and_yield stub_file + allow(File).to receive(:open).with(filename,/^r/).and_return stub_file filename end @@ -142,7 +142,7 @@ let(:user_file) do filename = "user_file" stub_file = StringIO.new("user1\nuser2\nuser3\n") - allow(File).to receive(:open).with(filename,/^r/).and_yield stub_file + allow(File).to receive(:open).with(filename,/^r/).and_return stub_file filename end @@ -173,7 +173,7 @@ let(:user_file) do filename = "user_file" stub_file = StringIO.new("user1\nuser2\nuser3\n") - allow(File).to receive(:open).with(filename,/^r/).and_yield stub_file + allow(File).to receive(:open).with(filename,/^r/).and_return stub_file filename end @@ -220,7 +220,7 @@ let(:user_file) do filename = "user_file" stub_file = StringIO.new("user1\nuser2\nuser3\n") - allow(File).to receive(:open).with(filename,/^r/).and_yield stub_file + allow(File).to receive(:open).with(filename,/^r/).and_return stub_file filename end @@ -255,7 +255,7 @@ let(:user_file) do filename = "user_file" stub_file = StringIO.new("user1\nuser2\nuser3\n") - allow(File).to receive(:open).with(filename,/^r/).and_yield stub_file + allow(File).to receive(:open).with(filename,/^r/).and_return stub_file filename end @@ -293,7 +293,7 @@ let(:pass_file) do filename = "pass_file" stub_file = StringIO.new("asdf\njkl\n") - allow(File).to receive(:open).with(filename, /^r/).and_yield stub_file + allow(File).to receive(:open).with(filename, /^r/).and_return stub_file filename end @@ -473,7 +473,7 @@ let(:user_file) do filename = "foo" stub_file = StringIO.new("asdf\njkl\n") - allow(File).to receive(:open).with(filename,/^r/).and_yield stub_file + allow(File).to receive(:open).with(filename,/^r/).and_return stub_file filename end @@ -502,7 +502,7 @@ let(:pass_file) do filename = "pass_file" stub_file = StringIO.new("passfile\n") - allow(File).to receive(:open).with(filename,/^r/).and_yield stub_file + allow(File).to receive(:open).with(filename,/^r/).and_return stub_file filename end @@ -554,7 +554,7 @@ let(:user_file) do filename = "user_file" stub_file = StringIO.new("userfile") - allow(File).to receive(:open).with(filename,/^r/).and_yield stub_file + allow(File).to receive(:open).with(filename,/^r/).and_return stub_file filename end From 2047671e24e5bdf19e060a768350c2f6ff8ca716 Mon Sep 17 00:00:00 2001 From: cgranleese-r7 Date: Tue, 28 Jan 2025 14:54:32 +0000 Subject: [PATCH 08/11] Some final tidy up --- .../framework/credential_collection_spec.rb | 48 ++++++++----------- 1 file changed, 19 insertions(+), 29 deletions(-) diff --git a/spec/lib/metasploit/framework/credential_collection_spec.rb b/spec/lib/metasploit/framework/credential_collection_spec.rb index 55b4bc2b12bc..ff616a78dc12 100644 --- a/spec/lib/metasploit/framework/credential_collection_spec.rb +++ b/spec/lib/metasploit/framework/credential_collection_spec.rb @@ -334,12 +334,11 @@ let(:password_spray) { true } let(:nil_passwords) { true } - # REMOVE BEFORE COMMIT: yields nothings, fails because of bug in method context "without password" do let(:password) { nil } - specify do + specify do expect { |b| collection.each(&b) }.to yield_successive_args( - Metasploit::Framework::Credential.new(public: username, private: nil), + Metasploit::Framework::Credential.new(public: username, private: nil) ) end end @@ -361,10 +360,9 @@ let(:additional_publics) { [ "test_public" ] } let(:user_as_pass) { true } - # REMOVE BEFORE COMMIT currently failing - specify do + specify do expect { |b| collection.each(&b) }.to yield_successive_args( - Metasploit::Framework::Credential.new(public: "test_public", private: "test_public"), + Metasploit::Framework::Credential.new(public: "test_public", private: "test_public") ) end end @@ -376,10 +374,9 @@ let(:user_as_pass) { true } let(:password_spray) { true } - # REMOVE BEFORE COMMIT currently failing - specify do + specify do expect { |b| collection.each(&b) }.to yield_successive_args( - Metasploit::Framework::Credential.new(public: "test_public", private: "test_public"), + Metasploit::Framework::Credential.new(public: "test_public", private: "test_public") ) end end @@ -390,10 +387,9 @@ let(:additional_publics) { [ "test_public" ] } let(:nil_passwords) { true } - # REMOVE BEFORE COMMIT: this option is ignored currently for additional_publics specify do expect { |b| collection.each(&b) }.to yield_successive_args( - Metasploit::Framework::Credential.new(public: "test_public", private: nil), + Metasploit::Framework::Credential.new(public: "test_public", private: nil) ) end end @@ -406,13 +402,12 @@ let(:blank_passwords) { true } let(:password_spray) { true } - # REMOVE BEFORE COMMIT: fails because no pwd spraying - specify do + specify do expect { |b| collection.each(&b) }.to yield_successive_args( Metasploit::Framework::Credential.new(public: "test_public1", private: nil), Metasploit::Framework::Credential.new(public: "test_public2", private: nil), Metasploit::Framework::Credential.new(public: "test_public1", private: ""), - Metasploit::Framework::Credential.new(public: "test_public2", private: ""), + Metasploit::Framework::Credential.new(public: "test_public2", private: "") ) end end @@ -429,12 +424,11 @@ filename end - # REMOVE BEFORE COMMIT: this also yields the usernames as passwords for the additional_public - specify do + specify do expect { |b| collection.each(&b) }.to yield_successive_args( Metasploit::Framework::Credential.new(public: "asdf", private: "password"), Metasploit::Framework::Credential.new(public: "jkl", private: "password"), - Metasploit::Framework::Credential.new(public: "test_public", private: "password"), + Metasploit::Framework::Credential.new(public: "test_public", private: "password") ) end end @@ -445,8 +439,7 @@ let(:username) { nil } let(:password) { "pass" } - # REMOVE BEFORE COMMIT: this yields empty creds (no username, no pass) - specify do + specify do expect { |b| collection.each(&b) }.to yield_successive_args() end end @@ -457,10 +450,9 @@ let(:username) { "user" } let(:password) { nil } - # REMOVE BEFORE COMMIT: this yields empty creds (no username, no pass) - specify do + specify do expect { |b| collection.each(&b) }.to yield_successive_args( - Metasploit::Framework::Credential.new(public: username, private: ''), + Metasploit::Framework::Credential.new(public: username, private: '') ) end end @@ -478,11 +470,10 @@ filename end - # REMOVE BEFORE COMMIT: yields nothing, same for blank passwords option - specify do + specify do expect { |b| collection.each(&b) }.to yield_successive_args( Metasploit::Framework::Credential.new(public: "asdf", private: ''), - Metasploit::Framework::Credential.new(public: "jkl", private: ''), + Metasploit::Framework::Credential.new(public: "jkl", private: '') ) end end @@ -518,8 +509,7 @@ let(:additional_privates) { ['test_private'] } let(:additional_publics) { ['test_public'] } - # REMOVE BEFORE COMMIT: fails because of the useraspass error, then fails because of the nil value for addittonal publics and should be ok then - specify do + specify do expect { |b| collection.each(&b) }.to yield_successive_args( "test_prepend", Metasploit::Framework::Credential.new(public: "user", private: nil), @@ -577,7 +567,7 @@ let(:additional_privates) { ['test_private'] } let(:additional_publics) { ['test_public'] } - specify do + specify do expect { |b| collection.each(&b) }.to yield_successive_args( "test_prepend", Metasploit::Framework::Credential.new(public: "user", private: nil), @@ -598,7 +588,7 @@ Metasploit::Framework::Credential.new(public: "userpass_user", private: "userpass_pass"), Metasploit::Framework::Credential.new(public: "user", private: "test_private"), Metasploit::Framework::Credential.new(public: "userfile", private: "test_private"), - Metasploit::Framework::Credential.new(public: "test_public", private: "test_private"), + Metasploit::Framework::Credential.new(public: "test_public", private: "test_private") ) end end From 033dd2f06a560a29b881b8538cd2542170c41328 Mon Sep 17 00:00:00 2001 From: Mathieu Date: Fri, 13 Dec 2024 17:30:11 +0100 Subject: [PATCH 09/11] First refactoring pass in order to dry the code that iterates on passwords and usernames --- .../framework/credential_collection.rb | 275 ++++++------------ 1 file changed, 88 insertions(+), 187 deletions(-) diff --git a/lib/metasploit/framework/credential_collection.rb b/lib/metasploit/framework/credential_collection.rb index 8bab6b88053d..de9135b93234 100644 --- a/lib/metasploit/framework/credential_collection.rb +++ b/lib/metasploit/framework/credential_collection.rb @@ -240,23 +240,29 @@ def add_public(public_str='') # @yieldparam credential [Metasploit::Framework::Credential] # @return [void] def each_filtered - if password_spray - each_unfiltered_password_first do |credential| - next unless self.filter.nil? || self.filter.call(credential) - - yield credential - end - else - each_unfiltered_username_first do |credential| - next unless self.filter.nil? || self.filter.call(credential) + each_unfiltered do |credential| + next unless self.filter.nil? || self.filter.call(credential) - yield credential - end + yield credential end end alias each each_filtered + def each_unfiltered(&block) + prepended_creds.each { |c| yield c } + + if anonymous_login + yield Metasploit::Framework::Credential.new(public: '', private: '', realm: realm, private_type: :password) + end + + if password_spray + each_unfiltered_password_first(&block) + else + each_unfiltered_username_first(&block) + end + end + # When password spraying is enabled, do first passwords then usernames # i.e. # username1:password1 @@ -270,86 +276,28 @@ def each_filtered # @yieldparam credential [Metasploit::Framework::Credential] # @return [void] def each_unfiltered_password_first - if user_file.present? - user_fd = File.open(user_file, 'r:binary') - end - - prepended_creds.each { |c| yield c } - - if anonymous_login - yield Metasploit::Framework::Credential.new(public: '', private: '', realm: realm, private_type: :password) - end - if nil_passwords - if username.present? + each_username do |username| yield Metasploit::Framework::Credential.new(public: username, private: nil, realm: realm, private_type: :password) end - - if user_fd - user_fd.each_line do |user_from_file| - user_from_file.chomp! - yield Metasploit::Framework::Credential.new(public: user_from_file, private: nil, realm: realm, private_type: :password) - end - user_fd.seek(0) - end - - additional_publics.each do |add_public| - yield Metasploit::Framework::Credential.new(public: add_public, private: nil, realm: realm, private_type: :password) - end end if password.present? - if username.present? + each_username do |username| yield Metasploit::Framework::Credential.new(public: username, private: password, realm: realm, private_type: private_type(password)) end - - if user_fd - user_fd.each_line do |user_from_file| - user_from_file.chomp! - yield Metasploit::Framework::Credential.new(public: user_from_file, private: password, realm: realm, private_type: private_type(password)) - end - user_fd.seek(0) - end - - additional_publics.each do |add_public| - yield Metasploit::Framework::Credential.new(public: add_public, private: password, realm: realm, private_type: private_type(password)) - end end if user_as_pass - if username.present? + each_username do |username| yield Metasploit::Framework::Credential.new(public: username, private: username, realm: realm, private_type: :password) end - - if user_fd - user_fd.each_line do |user_from_file| - user_from_file.chomp! - yield Metasploit::Framework::Credential.new(public: user_from_file, private: user_from_file, realm: realm, private_type: private_type(password)) - end - user_fd.seek(0) - end - - additional_publics.each do |add_public| - yield Metasploit::Framework::Credential.new(public: add_public, private: add_public, realm: realm, private_type: :password) - end end if blank_passwords - if username.present? + each_username do |username| yield Metasploit::Framework::Credential.new(public: username, private: "", realm: realm, private_type: :password) end - - if user_fd - user_fd.each_line do |user_from_file| - user_from_file.chomp! - yield Metasploit::Framework::Credential.new(public: user_from_file, private: "", realm: realm, private_type: :password) - end - user_fd.seek(0) - end - - additional_publics.each do |add_public| - yield Metasploit::Framework::Credential.new(public: add_public, private: "", realm: realm, private_type: :password) - end end if pass_file.present? @@ -357,58 +305,43 @@ def each_unfiltered_password_first pass_fd.each_line do |pass_from_file| pass_from_file.chomp! - if username.present? + each_username do |username| yield Metasploit::Framework::Credential.new(public: username, private: pass_from_file, realm: realm, private_type: private_type(pass_from_file)) end - - if user_fd - user_fd.each_line do |user_from_file| - user_from_file.chomp! - yield Metasploit::Framework::Credential.new(public: user_from_file, private: pass_from_file, realm: realm, private_type: private_type(pass_from_file)) - end - user_fd.seek(0) - end - - additional_publics.each do |add_public| - yield Metasploit::Framework::Credential.new(public: add_public, private: pass_from_file, realm: realm, private_type: private_type(pass_from_file)) - end end end end - if userpass_file.present? - File.open(userpass_file, 'r:binary') do |userpass_fd| - userpass_fd.each_line do |line| - user, pass = line.split(" ", 2) - if pass.blank? - pass = '' - else - pass.chomp! - end - yield Metasploit::Framework::Credential.new(public: user, private: pass, realm: realm) - end - end + each_user_pass_from_userpass_file do |user, pass| + yield Metasploit::Framework::Credential.new(public: user, private: pass, realm: realm) end additional_privates.each do |add_private| - if username.present? + each_username do |username| yield Metasploit::Framework::Credential.new(public: username, private: add_private, realm: realm, private_type: private_type(add_private)) end + end + end - if user_fd + # Iterates over all possible usernames + def each_username + if username.present? + yield username + end + + if user_file.present? + File.open(user_file, 'r:binary') do |user_fd| user_fd.each_line do |user_from_file| user_from_file.chomp! - yield Metasploit::Framework::Credential.new(public: user_from_file, private: add_private, realm: realm, private_type: private_type(add_private)) + yield user_from_file end user_fd.seek(0) end + end - additional_publics.each do |add_public| - yield Metasploit::Framework::Credential.new(public: add_public, private: add_private, realm: realm, private_type: private_type(add_private)) - end + additional_publics.each do |add_public| + yield add_public end - ensure - user_fd.close if user_fd && !user_fd.closed? end # When password spraying is not enabled, do first usernames then passwords @@ -423,38 +356,9 @@ def each_unfiltered_password_first # @yieldparam credential [Metasploit::Framework::Credential] # @return [void] def each_unfiltered_username_first - if pass_file.present? - pass_fd = File.open(pass_file, 'r:binary') - end - - prepended_creds.each { |c| yield c } - - if anonymous_login - yield Metasploit::Framework::Credential.new(public: '', private: '', realm: realm, private_type: :password) - end - if username.present? - if nil_passwords - yield Metasploit::Framework::Credential.new(public: username, private: nil, realm: realm, private_type: :password) - end - if password.present? - yield Metasploit::Framework::Credential.new(public: username, private: password, realm: realm, private_type: private_type(password)) - end - if user_as_pass - yield Metasploit::Framework::Credential.new(public: username, private: username, realm: realm, private_type: :password) - end - if blank_passwords - yield Metasploit::Framework::Credential.new(public: username, private: "", realm: realm, private_type: :password) - end - if pass_fd - pass_fd.each_line do |pass_from_file| - pass_from_file.chomp! - yield Metasploit::Framework::Credential.new(public: username, private: pass_from_file, realm: realm, private_type: private_type(pass_from_file)) - end - pass_fd.seek(0) - end - additional_privates.each do |add_private| - yield Metasploit::Framework::Credential.new(public: username, private: add_private, realm: realm, private_type: private_type(add_private)) + each_password(username) do |password, private_type| + yield Metasploit::Framework::Credential.new(public: username, private: password, realm: realm, private_type: private_type) end end @@ -462,72 +366,69 @@ def each_unfiltered_username_first File.open(user_file, 'r:binary') do |user_fd| user_fd.each_line do |user_from_file| user_from_file.chomp! - if nil_passwords - yield Metasploit::Framework::Credential.new(public: user_from_file, private: nil, realm: realm, private_type: :password) - end - if password.present? - yield Metasploit::Framework::Credential.new(public: user_from_file, private: password, realm: realm, private_type: private_type(password) ) - end - if user_as_pass - yield Metasploit::Framework::Credential.new(public: user_from_file, private: user_from_file, realm: realm, private_type: :password) - end - if blank_passwords - yield Metasploit::Framework::Credential.new(public: user_from_file, private: "", realm: realm, private_type: :password) - end - if pass_fd - pass_fd.each_line do |pass_from_file| - pass_from_file.chomp! - yield Metasploit::Framework::Credential.new(public: user_from_file, private: pass_from_file, realm: realm, private_type: private_type(pass_from_file)) - end - pass_fd.seek(0) - end - additional_privates.each do |add_private| - yield Metasploit::Framework::Credential.new(public: user_from_file, private: add_private, realm: realm, private_type: private_type(add_private)) + each_password(user_from_file) do |password, private_type| + yield Metasploit::Framework::Credential.new(public: user_from_file, private: password, realm: realm, private_type: private_type) end end end end - if userpass_file.present? - File.open(userpass_file, 'r:binary') do |userpass_fd| - userpass_fd.each_line do |line| - user, pass = line.split(" ", 2) - if pass.blank? - pass = '' - else - pass.chomp! - end - yield Metasploit::Framework::Credential.new(public: user, private: pass, realm: realm) - end - end + each_user_pass_from_userpass_file do |user, pass| + yield Metasploit::Framework::Credential.new(public: user, private: pass, realm: realm) end additional_publics.each do |add_public| - if nil_passwords - yield Metasploit::Framework::Credential.new(public: add_public, private: nil, realm: realm, private_type: :password) + each_password(add_public) do |password, private_type| + yield Metasploit::Framework::Credential.new(public: add_public, private: password, realm: realm, private_type: private_type) end - if password.present? - yield Metasploit::Framework::Credential.new(public: add_public, private: password, realm: realm, private_type: private_type(password) ) - end - if user_as_pass - yield Metasploit::Framework::Credential.new(public: add_public, private: add_public, realm: realm, private_type: :password) - end - if blank_passwords - yield Metasploit::Framework::Credential.new(public: add_public, private: "", realm: realm, private_type: :password) - end - if pass_fd + end + end + + # Iterates over all possible passwords + def each_password(user) + if nil_passwords + yield [nil, :password] + end + + if password.present? + yield [password, private_type(password)] + end + + if user_as_pass + yield [user, :password] + end + + if blank_passwords + yield ["", :password] + end + + if pass_file + File.open(pass_file, 'r:binary') do |pass_fd| pass_fd.each_line do |pass_from_file| pass_from_file.chomp! - yield Metasploit::Framework::Credential.new(public: add_public, private: pass_from_file, realm: realm, private_type: private_type(pass_from_file)) + yield [pass_from_file, private_type(pass_from_file)] end pass_fd.seek(0) end - additional_privates.each do |add_private| - yield Metasploit::Framework::Credential.new(public: add_public, private: add_private, realm: realm, private_type: private_type(add_private)) + end + + additional_privates.each do |add_private| + yield [add_private, private_type(add_private)] + end + end + + # Iterates on userpass file if present + def each_user_pass_from_userpass_file + return unless userpass_file.present? + + File.open(userpass_file, 'r:binary') do |userpass_fd| + userpass_fd.each_line do |line| + user, pass = line.split(" ", 2) + pass = pass.blank? ? '' : pass.chomp! + + yield [user, pass] end end - ensure - pass_fd.close if pass_fd && !pass_fd.closed? end # Returns true when #each will have no results to iterate From 7612b31158a88d20733890a90dcbb73e1cef1e0c Mon Sep 17 00:00:00 2001 From: Mathieu Date: Fri, 7 Feb 2025 17:37:06 +0100 Subject: [PATCH 10/11] Re-revert tests expectations --- .../framework/credential_collection_spec.rb | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/spec/lib/metasploit/framework/credential_collection_spec.rb b/spec/lib/metasploit/framework/credential_collection_spec.rb index ff616a78dc12..a4d3cc365012 100644 --- a/spec/lib/metasploit/framework/credential_collection_spec.rb +++ b/spec/lib/metasploit/framework/credential_collection_spec.rb @@ -68,7 +68,7 @@ let(:pass_file) do filename = "foo" stub_file = StringIO.new("asdf\njkl\n") - allow(File).to receive(:open).with(filename,/^r/).and_return stub_file + allow(File).to receive(:open).with(filename,/^r/).and_yield stub_file filename end @@ -113,7 +113,7 @@ let(:pass_file) do filename = "pass_file" stub_file = StringIO.new("asdf\njkl\n") - allow(File).to receive(:open).with(filename,/^r/).and_return stub_file + allow(File).to receive(:open).with(filename,/^r/).and_yield stub_file filename end @@ -142,7 +142,7 @@ let(:user_file) do filename = "user_file" stub_file = StringIO.new("user1\nuser2\nuser3\n") - allow(File).to receive(:open).with(filename,/^r/).and_return stub_file + allow(File).to receive(:open).with(filename,/^r/).and_yield stub_file filename end @@ -173,7 +173,7 @@ let(:user_file) do filename = "user_file" stub_file = StringIO.new("user1\nuser2\nuser3\n") - allow(File).to receive(:open).with(filename,/^r/).and_return stub_file + allow(File).to receive(:open).with(filename,/^r/).and_yield stub_file filename end @@ -220,7 +220,7 @@ let(:user_file) do filename = "user_file" stub_file = StringIO.new("user1\nuser2\nuser3\n") - allow(File).to receive(:open).with(filename,/^r/).and_return stub_file + allow(File).to receive(:open).with(filename,/^r/).and_yield stub_file filename end @@ -255,7 +255,7 @@ let(:user_file) do filename = "user_file" stub_file = StringIO.new("user1\nuser2\nuser3\n") - allow(File).to receive(:open).with(filename,/^r/).and_return stub_file + allow(File).to receive(:open).with(filename,/^r/).and_yield stub_file filename end @@ -293,7 +293,7 @@ let(:pass_file) do filename = "pass_file" stub_file = StringIO.new("asdf\njkl\n") - allow(File).to receive(:open).with(filename, /^r/).and_return stub_file + allow(File).to receive(:open).with(filename, /^r/).and_yield stub_file filename end @@ -465,7 +465,7 @@ let(:user_file) do filename = "foo" stub_file = StringIO.new("asdf\njkl\n") - allow(File).to receive(:open).with(filename,/^r/).and_return stub_file + allow(File).to receive(:open).with(filename,/^r/).and_yield stub_file filename end @@ -493,7 +493,7 @@ let(:pass_file) do filename = "pass_file" stub_file = StringIO.new("passfile\n") - allow(File).to receive(:open).with(filename,/^r/).and_return stub_file + allow(File).to receive(:open).with(filename,/^r/).and_yield stub_file filename end @@ -544,7 +544,7 @@ let(:user_file) do filename = "user_file" stub_file = StringIO.new("userfile") - allow(File).to receive(:open).with(filename,/^r/).and_return stub_file + allow(File).to receive(:open).with(filename,/^r/).and_yield stub_file filename end From b08a5f6ce3b6ad360f15ca1bb9b758c6a5b06f5b Mon Sep 17 00:00:00 2001 From: Mathieu Date: Wed, 12 Feb 2025 17:47:18 +0100 Subject: [PATCH 11/11] Change how ldap_login generate its specific credentials for SCHANNEL && KERBEROS auth --- .../framework/credential_collection.rb | 30 +----------- modules/auxiliary/scanner/ldap/ldap_login.rb | 37 +++++++++----- .../framework/credential_collection_spec.rb | 49 +------------------ 3 files changed, 27 insertions(+), 89 deletions(-) diff --git a/lib/metasploit/framework/credential_collection.rb b/lib/metasploit/framework/credential_collection.rb index 628b77569e15..de9135b93234 100644 --- a/lib/metasploit/framework/credential_collection.rb +++ b/lib/metasploit/framework/credential_collection.rb @@ -212,23 +212,6 @@ class CredentialCollection < PrivateCredentialCollection # @return [Boolean] attr_accessor :anonymous_login - # @!attribute ignore_private - # Whether to ignore private (password). This is usually set when Kerberos - # or Schannel authentication is requested and the credentials are - # retrieved from cache or from a file. This attribute should be true in - # these scenarios, otherwise validation will fail since the password is not - # provided. - # @return [Boolean] - attr_accessor :ignore_private - - # @!attribute ignore_public - # Whether to ignore public (username). This is usually set when Schannel - # authentication is requested and the credentials are retrieved from a - # file (certificate). This attribute should be true in this case, - # otherwise validation will fail since the password is not provided. - # @return [Boolean] - attr_accessor :ignore_public - # @option opts [Boolean] :blank_passwords See {#blank_passwords} # @option opts [String] :pass_file See {#pass_file} # @option opts [String] :password See {#password} @@ -267,15 +250,6 @@ def each_filtered alias each each_filtered def each_unfiltered(&block) - if ignore_private - if ignore_public - yield Metasploit::Framework::Credential.new(public: nil, private: nil, realm: realm) - else - yield Metasploit::Framework::Credential.new(public: username, private: nil, realm: realm) - end - return - end - prepended_creds.each { |c| yield c } if anonymous_login @@ -468,14 +442,14 @@ def empty? # # @return [Boolean] def has_users? - username.present? || user_file.present? || userpass_file.present? || !additional_publics.empty? || !!ignore_public + username.present? || user_file.present? || userpass_file.present? || !additional_publics.empty? end # Returns true when there are any private values set # # @return [Boolean] def has_privates? - super || userpass_file.present? || user_as_pass || !!ignore_private + super || userpass_file.present? || user_as_pass end end diff --git a/modules/auxiliary/scanner/ldap/ldap_login.rb b/modules/auxiliary/scanner/ldap/ldap_login.rb index 47c6dac14db9..76a17e1a4372 100644 --- a/modules/auxiliary/scanner/ldap/ldap_login.rb +++ b/modules/auxiliary/scanner/ldap/ldap_login.rb @@ -89,19 +89,9 @@ def validate_connect_options! end def run_host(ip) - ignore_public = datastore['LDAP::Auth'] == Msf::Exploit::Remote::AuthOption::SCHANNEL - ignore_private = - datastore['LDAP::Auth'] == Msf::Exploit::Remote::AuthOption::SCHANNEL || - (Msf::Exploit::Remote::AuthOption::KERBEROS && !datastore['ANONYMOUS_LOGIN'] && !datastore['PASSWORD']) - - cred_collection = build_credential_collection( - username: datastore['USERNAME'], - password: datastore['PASSWORD'], - realm: datastore['DOMAIN'], - anonymous_login: datastore['ANONYMOUS_LOGIN'], - blank_passwords: false, - ignore_public: ignore_public, - ignore_private: ignore_private + cred_collection = build_specific_credential_collection( + void_login: datastore['LDAP::Auth'] == Msf::Exploit::Remote::AuthOption::SCHANNEL, + no_password_login: datastore['LDAP::Auth'] == Msf::Exploit::Remote::AuthOption::KERBEROS && !datastore['ANONYMOUS_LOGIN'] && !datastore['PASSWORD'] ) opts = { @@ -202,4 +192,25 @@ def session_setup(result) start_session(self, nil, merge_me, false, my_session.rstream, my_session) end + + def build_specific_credential_collection(void_login:, no_password_login:) + if void_login + Metasploit::Framework::PrivateCredentialCollection.new({ + nil_password: true + }) + elsif no_password_login + Metasploit::Framework::CredentialCollection.new({ + username: datastore['USERNAME'], + nil_password: true + }) + else + build_credential_collection( + username: datastore['USERNAME'], + password: datastore['PASSWORD'], + realm: datastore['DOMAIN'], + anonymous_login: datastore['ANONYMOUS_LOGIN'], + blank_passwords: false + ) + end + end end diff --git a/spec/lib/metasploit/framework/credential_collection_spec.rb b/spec/lib/metasploit/framework/credential_collection_spec.rb index b6ef2d637631..a4d3cc365012 100644 --- a/spec/lib/metasploit/framework/credential_collection_spec.rb +++ b/spec/lib/metasploit/framework/credential_collection_spec.rb @@ -16,9 +16,7 @@ prepended_creds: prepended_creds, additional_privates: additional_privates, additional_publics: additional_publics, - password_spray: password_spray, - ignore_public: ignore_public, - ignore_private: ignore_private + password_spray: password_spray ) end @@ -41,8 +39,6 @@ let(:additional_privates) { [] } let(:additional_publics) { [] } let(:password_spray) { false } - let(:ignore_public) { nil } - let(:ignore_private) { nil } describe "#each" do specify do @@ -596,34 +592,6 @@ ) end end - - context 'when :ignore_public is true and :username is nil' do - let(:ignore_public) { true } - let(:username) { nil } - specify do - expect { |b| collection.each(&b) }.to_not yield_control - end - end - - context 'when :ignore_private is true and password is nil' do - let(:ignore_private) { true } - let(:password) { nil } - specify do - expect { |b| collection.each(&b) }.to yield_successive_args( - Metasploit::Framework::Credential.new(public: username, private: nil) - ) - end - - context 'when :ignore_public is also true and username is nil' do - let(:ignore_public) { true } - let(:username) { nil } - specify do - expect { |b| collection.each(&b) }.to yield_successive_args( - Metasploit::Framework::Credential.new(public: nil, private: nil) - ) - end - end - end end describe "#empty?" do @@ -693,21 +661,6 @@ expect(collection.empty?).to eq true end end - - context "and :ignore_public is set" do - let(:ignore_public) { true } - specify do - expect(collection.empty?).to eq true - end - - context "and :ignore_private is also set" do - let(:ignore_private) { true } - specify do - expect(collection.empty?).to eq false - end - end - end - end end end