New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

start printjob docs and bug fixes #10981

Merged
merged 1 commit into from Nov 21, 2018

Conversation

Projects
None yet
4 participants
@h00die
Contributor

h00die commented Nov 18, 2018

Fixes to printjob_capture:

  1. Remove rhosts since I believe this was unintentionally added, and is similar to #10660
  2. simplify some of the spacing and newlines
  3. move the text of starting the server until after the options have validated
  4. replace raise with fail_with
  5. At two points there is a check on a print job item to be nil however it defaults to an empty string, thus making those branches in the code never reached. This means a RAW print job never has its loot stored, nor is a document title ever assigned.

I NEED HELP

Something strange is going on with RHOST. if you don't make it capital, it won't set. If you do, the datastore option still returns nil. This was not the case in Framework Version: 4.16.36-dev (what kali is currently running)

Example:

msf5 > use auxiliary/server/capture/printjob_capture 
msf5 auxiliary(server/capture/printjob_capture) > set forward true
forward => true
msf5 auxiliary(server/capture/printjob_capture) > set rhost 1.1.1.1
rhost => 1.1.1.1
msf5 auxiliary(server/capture/printjob_capture) > options

Module options (auxiliary/server/capture/printjob_capture):

   Name      Current Setting  Required  Description
   ----      ---------------  --------  -----------
   FORWARD   true             yes       Forward print jobs to another host
   METADATA  true             yes       Display Metadata from printjobs
   MODE      RAW              yes       Print mode (Accepted: RAW, LPR)
   RHOST                      no        Forward to remote host
   RPORT     9100             no        Forward to remote port (TCP)
   SRVHOST   0.0.0.0          yes       The local host to listen on. This must be an address on the local machine or 0.0.0.0
   SRVPORT   9100             yes       The local port to listen on


Auxiliary action:

   Name     Description
   ----     -----------
   Capture  


msf5 auxiliary(server/capture/printjob_capture) > set RHOST 1.1.1.1
RHOST => 1.1.1.1
msf5 auxiliary(server/capture/printjob_capture) > show options

Module options (auxiliary/server/capture/printjob_capture):

   Name      Current Setting  Required  Description
   ----      ---------------  --------  -----------
   FORWARD   true             yes       Forward print jobs to another host
   METADATA  true             yes       Display Metadata from printjobs
   MODE      RAW              yes       Print mode (Accepted: RAW, LPR)
   RHOST     1.1.1.1          no        Forward to remote host
   RPORT     9100             no        Forward to remote port (TCP)
   SRVHOST   0.0.0.0          yes       The local host to listen on. This must be an address on the local machine or 0.0.0.0
   SRVPORT   9100             yes       The local port to listen on


Auxiliary action:

   Name     Description
   ----     -----------
   Capture  


msf5 auxiliary(server/capture/printjob_capture) > run
[*] Auxiliary module running as background job 0.

[-] bad-config: Cannot forward without a valid RHOST

Can someone at r7 take a look to see whats going on with RHOST?

@h00die h00die added this to the Module documentation milestone Nov 18, 2018

@bcoles

This comment has been minimized.

Contributor

bcoles commented Nov 20, 2018

Jenkins test this please

@busterb

This comment has been minimized.

Contributor

busterb commented Nov 21, 2018

@h00die this diff should fix the RHOST setting issue:

diff --git a/lib/msf/core/data_store.rb b/lib/msf/core/data_store.rb
index bed6b66df4..939530a3d9 100644
--- a/lib/msf/core/data_store.rb
+++ b/lib/msf/core/data_store.rb
@@ -64,6 +64,7 @@ class DataStore < Hash
   # Case-insensitive wrapper around delete
   #
   def delete(k)
+    #@aliases.delete_if { |_, v| v.casecmp(k) == 0 }
     super(find_key_case(k))
   end

The problem with delete in master is that it was not deleting aliases for a key if a key was deleted. This is important because by default 'RHOST' is an alias for 'RHOSTS' (setting or referring to either one always yields the same value). Because 'RHOSTS' was registered in the datastore originally in this module, the alias was set, but when the option was unregistered, the alias was not being removed. The 'find_key_case' function, when used in the []= method would then find the alias 'rhosts' and return it for 'rhost'. That means that set rhost foo was really doing 'set rhosts foo'

Why RHOST assignment worked at all is a different story. I'm going to post a diff fixing this in a belt-and-suspenders way separately.

@busterb

This comment has been minimized.

Contributor

busterb commented Nov 21, 2018

@h00die the fix is in master now for rhost not setting properly. You can rebase or I can test and land this as-is.

@busterb busterb self-assigned this Nov 21, 2018

@busterb busterb merged commit 7ecdaa0 into rapid7:master Nov 21, 2018

3 checks passed

Metasploit Automation - Sanity Test Execution Successfully completed all tests.
Details
Metasploit Automation - Test Execution Successfully completed all tests.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

busterb added a commit that referenced this pull request Nov 21, 2018

@busterb

This comment has been minimized.

Contributor

busterb commented Nov 21, 2018

Release Notes

This modernizes the printjob_capture auxiliary module and fixes up loot storage.

msjenkins-r7 added a commit that referenced this pull request Nov 21, 2018

busterb added a commit to busterb/metasploit-framework that referenced this pull request Nov 26, 2018

remove secondary check that prevents aliases from functioning
Commit 063838f was overly aggresive about checking if an alias value exists in advance, which prevents aliases from being set altogether. da9e6ed was enought to fix the original issue reported in rapid7#10981 (comment)

@gdavidson-r7 gdavidson-r7 added the rn-fix label Dec 3, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment