Skip to content
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

Multiple fixes and query_service_status example script #162

Merged
merged 5 commits into from
Aug 14, 2020

Conversation

cdelafuente-r7
Copy link
Contributor

@cdelafuente-r7 cdelafuente-r7 commented Aug 13, 2020

This PR add the following fixes and improvements:

  • Add the missing Credit Charge logic to SMB2
  • Fix NBSS header
  • Add missing SMB1 Process ID
  • Refactor send_recv to avoid code duplication

It also updates the specs accordingly.

Verification

  • Try to read a file bigger than 64KB with: ruby examples/read_file.rb <host ip> <username> <password> <share> <file name>
  • Verify the file is read without error
  • Run the query service example script against Windows XP: ruby examples/query_service_status.rb <host ip> <username> <password> RemoteRegistry
  • Verify there is no error and the service status is returned

Scenarios

Credit Charge fix

First, check the file size:

$ ruby examples/list_directory.rb 192.168.3.22 msfuser mypasswd ADMIN$ test 2
SMB2 : (0x00000000) STATUS_SUCCESS: The operation completed successfully.
Connected to \\192.168.3.22\ADMIN$ successfully!
FILE: .
	SIZE(BYTES):0
	SIZE_ON_DISK(BYTES):0
	CREATED:2020-08-13T18:12:05+02:00
	ACCESSED:2020-08-13T18:12:26+02:00
	CHANGED:2020-08-13T18:12:26+02:00

FILE: ..
	SIZE(BYTES):0
	SIZE_ON_DISK(BYTES):0
	CREATED:2020-08-13T18:12:05+02:00
	ACCESSED:2020-08-13T18:12:26+02:00
	CHANGED:2020-08-13T18:12:26+02:00

FILE: 1lzbtVcI.tmp
	SIZE(BYTES):258048
	SIZE_ON_DISK(BYTES):258048
	CREATED:2020-08-10T20:56:00+02:00
	ACCESSED:2020-08-13T18:09:33+02:00
	CHANGED:2020-08-13T18:12:26+02:00

1lzbtVcI.tmp size is 258048 bytes (>64KB).

Then try to read it:

  • Before this fix
$ ruby examples/read_file.rb 192.168.3.22 msfuser mypasswd ADMIN$ test\\1lzbtVcI.tmp
SMB2 : (0x00000000) STATUS_SUCCESS: The operation completed successfully.
Connected to \\192.168.3.22\ADMIN$ successfully!
Traceback (most recent call last):
	1: from examples/read_file.rb:39:in `<main>'
/opt/ruby_smb/lib/ruby_smb/smb2/file.rb:136:in `read': The server responded with an unexpected status code: STATUS_INVALID_PARAMETER (RubySMB::Error::UnexpectedStatusCode)

The read returns an STATUS_INVALID_PARAMETER status. this is due to a wrong Credit Charge value in the request.

  • After this fix
$ ruby examples/read_file.rb 192.168.3.22 msfuser mypasswd ADMIN$ test\\1lzbtVcI.tmp
SMB2 : (0x00000000) STATUS_SUCCESS: The operation completed successfully.
Connected to \\192.168.3.22\ADMIN$ successfully!
...[file content]...

Missing SMB1 Process ID fix

Tested against Windows XP and the example script provided in this PR (examples/query_service_status.rb)

  • Before this fix
$ ruby examples/query_service_status.rb 192.168.3.25 Administrator mypasswd RemoteRegistry
SMB1 : (0x00000000) STATUS_SUCCESS: The operation completed successfully.
Binding to \svcctl...
Bound to \svcctl
Opening Service Control Manager
Traceback (most recent call last):
	1: from examples/query_service_status.rb:34:in `<main>'
/opt/ruby_smb/lib/ruby_smb/dcerpc/svcctl.rb:285:in `open_sc_manager_w': Error returned when opening Service Control Manager (SCM): (0x000006e4) RPC_S_CANNOT_SUPPORT: The requested operation is not supported. (RubySMB::Dcerpc::Error::SvcctlError)
  • After this fix
$ ruby examples/query_service_status.rb 192.168.3.25 Administrator mypasswd RemoteRegistry
SMB1 : (0x00000000) STATUS_SUCCESS: The operation completed successfully.
Binding to \svcctl...
Bound to \svcctl
Opening Service Control Manager
Service RemoteRegistry is running
Service RemoteRegistry starts automatically during system startup

- Add the missing Credit Charge logic to SMB2
- Fix NBSS header
- Add missing SMB1 process ID
- Refactor #send_recv to avoid code duplication
@coveralls
Copy link

coveralls commented Aug 13, 2020

Coverage Status

Coverage decreased (-0.01%) to 97.799% when pulling 9de3882 on cdelafuente-r7:fix_credit_and_nbss_header into f429ebb on rapid7:master.

Copy link
Contributor

@smcintyre-r7 smcintyre-r7 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes look good, just left a few comments. I'll take a look at testing this now.

lib/ruby_smb/client.rb Outdated Show resolved Hide resolved
lib/ruby_smb/client/negotiation.rb Outdated Show resolved Hide resolved
lib/ruby_smb/smb2/file.rb Outdated Show resolved Hide resolved
lib/ruby_smb/smb2/file.rb Outdated Show resolved Hide resolved
@smcintyre-r7
Copy link
Contributor

I successfully ran through the examples/read_file.rb test with a file that was 249K and compared the hashes. The target system was Windows 10 and it correctly negotiated SMB3. That test is looking good, I'll be testing the XP case now.

@cdelafuente-r7
Copy link
Contributor Author

Great! Thanks for testing! I just pushed some updates and I believe it addresses your comments.

@smcintyre-r7
Copy link
Contributor

Changes look great, all my comments have been addressed and I just wrapped successful testing of the XP case. I'll merge these changes in, thanks @cdelafuente-r7 !

@smcintyre-r7 smcintyre-r7 merged commit ced3870 into rapid7:master Aug 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants