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
Standard RDP Security Layer Levels/Method Overhaul #2273
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Test FAILed. |
[MS-RDPBCGR] Section 5.3 describes the encryption level and method values for standard RDP security. Looking at the current usage of these values in the FreeRDP code gives me reason to believe that there is a certain lack of understanding of how these values should be handled. The encryption level is only configured on the server side in the "Encryption Level" setting found in the Remote Desktop Session Host Configuration RDP-Tcp properties dialog and this value is never transferred from the client to the server over the wire. The possible options are "None", "Low", "Client Compatible", "High" and "FIPS Compliant". The client receices this value in the Server Security Data block (TS_UD_SC_SEC1), probably only for informational purposes and maybe to give the client the possibility to verify if the server's decision for the encryption method confirms to the server's encryption level. The possible encryption methods are "NONE", "40BIT", "56BIT", "128BIT" and "FIPS" and the RDP client advertises the ones it supports to the server in the Client Security Data block (TS_UD_CS_SEC). The server's configured encryption level value restricts the possible final encryption method. Something that I was not able to find in the documentation is the priority level of the individual encryption methods based on which the server makes its final method decision if there are several options. My analysis with Windows Servers reveiled that the order is 128, 56, 40, FIPS. The server only chooses FIPS if the level is "FIPS Comliant" or if it is the only method advertised by the client. Bottom line: * FreeRDP's client side does not need to set settings->EncryptionLevel (which was done quite frequently). * FreeRDP's server side does not have to set the supported encryption methods list in settings->EncryptionMethods Changes in this commit: Removed unnecessary/confusing changes of EncryptionLevel/Methods settings Refactor settings->DisableEncryption * This value actually means "Advanced RDP Encryption (NLA/TLS) is NOT used" * The old name caused lots of confusion among developers * Renamed it to "UseRdpSecurityLayer" (the compare logic stays untouched) Any client's setting of settings->EncryptionMethods were annihilated * All clients "want" to set all supported methods * Some clients forgot 56bit because 56bit was not supported at the time the code was written * settings->EncryptionMethods was overwritten anyways in nego_connect() * Removed all client side settings of settings->EncryptionMethods The default is "None" (0) * Changed nego_connect() to advertise all supported methods if settings->EncryptionMethods is 0 (None) * Added a commandline option /encryption-methods:comma separated list of the values "40", "56", "128", "FIPS". E.g. /encryption-methods:56,128 * Print warning if server chooses non-advertised method Verify received level and method in client's gcc_read_server_security_data * Only accept valid/known encryption methods * Verify encryption level/method combinations according to MS-RDPBCGR 5.3.2 Server implementations can now set settings->EncryptionLevel * The default for settings->EncryptionLevel is 0 (None) * nego_send_negotiation_response() changes it to ClientCompatible in that case * default to ClientCompatible if the server implementation set an invalid level Fix server's gcc_write_server_security_data * Verify server encryption level value set by server implementations * Choose rdp encryption method based on level and supported client methods * Moved FIPS to the lowest priority (only used if other methods are possible) Updated sample server * Support RDP Security (RdpKeyFile was not set) * Added commented sample code for setting the security level
nfedera
force-pushed
the
fix-2014-12-12-01
branch
from
December 12, 2014 01:17
6b2a300
to
939f1c6
Compare
Test PASSed. |
@nfedera that's some real good code you've got there :) I like your pull requests |
awakecoding
added a commit
that referenced
this pull request
Dec 12, 2014
Standard RDP Security Layer Levels/Method Overhaul
@nfedera really nice! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
[MS-RDPBCGR] Section 5.3 describes the encryption level and method values for standard RDP security.
Looking at the current usage of these values in the FreeRDP code gives me reason to believe that there is a certain lack of understanding of how these values should be handled.
The encryption level is only configured on the server side in the "Encryption Level" setting found in the Remote Desktop Session Host Configuration RDP-Tcp properties dialog.
This value is never transferred from the client to the server over the wire!
The possible options are "None", "Low", "Client Compatible", "High" and "FIPS Compliant".
The client receices this value in the Server Security Data block (TS_UD_SC_SEC1), probably only for informational purposes and maybe to give the client the possibility to verify if the server's decision for the encryption method confirms to the server's encryption level.
The possible encryption methods are "NONE", "40BIT", "56BIT", "128BIT" and "FIPS" and the RDP client advertises the ones it supports to the server in the Client Security Data block (TS_UD_CS_SEC).
The server's configured encryption level value restricts the possible final encryption method.
Something that I was not able to find in the documentation is the priority level of the individual encryption methods based on which the server makes its final method decision if there are several options.
My analysis with Windows Servers reveiled that the order is 128, 56, 40, FIPS.
The server only chooses FIPS if the level is "FIPS Comliant" or if it is the only method advertised by the client.
Bottom line:
Changes in this commit:
Removed unnecessary/confusing changes of EncryptionLevel/Methods settings
Refactor settings->DisableEncryption
Any client's setting of settings->EncryptionMethods were annihilated
Verify received level and method in client's gcc_read_server_security_data
Server implementations can now set settings->EncryptionLevel
Fix server's gcc_write_server_security_data
Updated sample server