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

Address exception and decoding issue in rdp-enum-encryption #1611

Closed
wants to merge 5 commits into from

Conversation

TomSellers
Copy link

@TomSellers TomSellers commented May 26, 2019

This PR addresses a few bugs in RDP protocol parsing in scripts/rdp-enum-encryption.nse and nselib/rdp.lua.

It should also address the following issue:

Feature Request: rdp-enum-encryption should check if NLA is required #174

Exception vs Windows XP

There was an exception caused by rdp-enum-encryption.nse attempting to unpack data that doesn't exist. This was noticed when running the script against Windows XP SP3 which doesn't return a certain layer in the protocol during the initial negotiation packet exchange.

Testing

sudo nmap -sSVC -p 3389 --script=rdp-enum-encryption -vvvv  -d2 <target>

Before fix

<snip>

NSE: rdp-enum-encryption M:559f23dc78f8 against 192.168.200.196:3389 threw an error!
.../local/bin/../share/nmap/scripts/rdp-enum-encryption.nse:74: bad argument #2 to 'unpack' (data string too short)
stack traceback:
	[C]: in function 'string.unpack'
	.../local/bin/../share/nmap/scripts/rdp-enum-encryption.nse:74: in upvalue 'enum_protocols'
	.../local/bin/../share/nmap/scripts/rdp-enum-encryption.nse:152: in function <.../local/bin/../share/nmap/scripts/rdp-enum-encryption.nse:149>
	(...tail calls...)

<snip>
PORT     STATE SERVICE       REASON          VERSION
3389/tcp open  ms-wbt-server syn-ack ttl 128 Microsoft Terminal Services

After fix

PORT     STATE SERVICE       REASON          VERSION
3389/tcp open  ms-wbt-server syn-ack ttl 128 Microsoft Terminal Services
| rdp-enum-encryption: 
|   Security layer
|     CredSSP: Unknown
|     Native RDP: Unknown
|     SSL: Unknown
|   RDP Encryption level: Unknown
|     128-bit RC4: SUCCESS
|_    FIPS 140-1: FAILURE

Incorrectly decoding ServerData

The existing code was incorrectly decoding the ServerData section of the ConferenceCreateResponse packet. This was due to assuming a certain block was a fixed size. This broke when certain optional fields were included. The impact was that certain encryption level options were not being detected. This primarily impacted Windows XP.

The fix was adding decoding of the type and lengths for the sections in ServerData.

Before fix

Technically, before the fix this section was empty due to the first bug fixed above. But after fixing that bug here is what the output looked like.

3389/tcp open  ms-wbt-server syn-ack ttl 128
| rdp-enum-encryption: 
|   RDP Encryption level: Unknown
|     128-bit RC4: SUCCESS
|_    FIPS 140-1: FAILURE

After fix

PORT     STATE SERVICE       REASON
3389/tcp open  ms-wbt-server syn-ack ttl 128
| rdp-enum-encryption: 
|   Security layer
|     CredSSP: Unknown
|     Native RDP: Unknown
|     SSL: Unknown
|   RDP Encryption level: Client Compatible
|     40-bit RC4: SUCCESS
|     56-bit RC4: SUCCESS
|     128-bit RC4: SUCCESS
|_    FIPS 140-1: FAILURE

Added output

  • Now that ServerData is being decoded we can determine which RDP protocol is being advertised. When detected this will now show up in the RDP Protocol Version: section. Since the library doesn't currently support TLS negotiation this will be missing for hosts that require TLS until this feature is added. The same applies for CredSSP.

  • In the Security layer section of the output

    • CredSSP has been changed to CredSSP (NLA) to clarify that this is NLA for those who are using the script to audit networks for things like CVE-2019-0708.
    • CredSSP with Early User Auth and RDSTLS have been added.

Output

Windows XP SP3

3389/tcp open  ms-wbt-server syn-ack ttl 128
| rdp-enum-encryption: 
|   Security layer
|     CredSSP (NLA): Unknown
|     CredSSP with Early User Auth: Unknown
|     Native RDP: Unknown
|     RDSTLS: Unknown
|     SSL: Unknown
|   RDP Encryption level: Client Compatible
|     40-bit RC4: SUCCESS
|     56-bit RC4: SUCCESS
|     128-bit RC4: SUCCESS
|     FIPS 140-1: FAILURE
|_  RDP Protocol Version:  RDP 5.x, 6.x, 7.x, or 8.x server

Windows 2008 with NLA optional and configured for client compatible ciphers

3389/tcp open  ms-wbt-server syn-ack ttl 128
| rdp-enum-encryption: 
|   Security layer
|     CredSSP (NLA): SUCCESS
|     CredSSP with Early User Auth: SUCCESS
|     Native RDP: SUCCESS
|     RDSTLS: SUCCESS
|     SSL: SUCCESS
|   RDP Encryption level: Client Compatible
|     40-bit RC4: SUCCESS
|     56-bit RC4: SUCCESS
|     128-bit RC4: SUCCESS
|     FIPS 140-1: SUCCESS
|_  RDP Protocol Version:  RDP 5.x, 6.x, 7.x, or 8.x server

Windows 2019

3389/tcp open  ms-wbt-server syn-ack ttl 128
| rdp-enum-encryption: 
|   Security layer
|     CredSSP (NLA): SUCCESS
|     CredSSP with Early User Auth: SUCCESS
|     Native RDP: FAILED (HYBRID_REQUIRED_BY_SERVER)
|     RDSTLS: SUCCESS
|_    SSL: FAILED (HYBRID_REQUIRED_BY_SERVER)

@TomSellers TomSellers changed the title Address exception in rdp-enum-encryption Address exception and decoding issue in rdp-enum-encryption May 28, 2019
Copy link

@dmiller-nmap dmiller-nmap left a comment

Choose a reason for hiding this comment

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

Looks great! Just the one comment regarding endianness in string.unpack formats to address. Commit if you have access, or let us know if you need to get your SVN access restored.

nselib/rdp.lua Outdated Show resolved Hide resolved
@nmap-bot nmap-bot closed this in 95f9e2c May 28, 2019
@c22
Copy link

c22 commented Aug 16, 2019

For the purposes of determining vulnerability to certain RDP attacks, I don't think this is sufficient. This seems to tell you if NLA is available but does not tell you if NLA is being enforced. Which is to say that a client could still attempt to create a non-NLA session even if NLA is supported by the server.

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