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

Remove Licensing exchange #3143

Merged
merged 2 commits into from
Jul 15, 2024

Conversation

matt335672
Copy link
Member

@matt335672 matt335672 commented Jul 2, 2024

Fixes #3132

Replaces the existing licensing exchange with a single PDU saying the server will not issue a license.

This is necessary for clients on FIPS-compliant systems, as these are unable to decode the licensing exchange PDUs, due to outdated cyphers in the licensing PDUs.

As far as FIPS mode goes, this is tested with xfreerdp 2.11.2 on Alma 9 in FIPS mode connecting to Ubuntu with fips configured in /etc/xrdp/xrdp.ini. Command is xfreerdp +fipsmode /v:<host>. The /etc/xrdp/xrdp.ini config is actually irrelevant as in any case the connection needs to take place over TLS because of #2266.

Without this change the connection fails with:-

[14:31:38:701] [3394:3395] [DEBUG][com.freerdp.core.connection] - rdp_client_transition_to_state CONNECTION_STATE_MCS_CHANNEL_JOIN --> CONNECTION_STATE_LICENSING
[14:31:39:801] [3394:3395] [DEBUG][com.freerdp.core.rdp] - rdp_recv_callback: CONNECTION_STATE_LICENSING - rdp_client_connect_license() - -1
[14:31:39:802] [3394:3395] [DEBUG][com.freerdp.core.rdp] - transport_check_fds() - -1

@akallabeth - With FreeRDP 3.5.1 I'm getting the following warning sending an empty BLOB in the license error PDU:-

[14:34:02:508] [50170:0000c3fb] [WARN][com.freerdp.core.license] - [license_read_binary_blob_data]: license binary blob::type BB_ERROR_BLOB, length=0, skipping.

Revised code is now here:-

    /* [MS-RDPBCGR] TS_SECURITY_HEADER */
    /* A careful reading of [MS-RDPBCGR] 2.2.1.12 shows that a securityHeader
     * MUST be included, and provided the flag fields of the header does
     * not contain SEC_ENCRYPT, it is always possible to send a basic
     * security header */
    out_uint16_le(s, SEC_LICENSE_PKT | SEC_LICENSE_ENCRYPT_CS); /* flags */
    out_uint16_le(s, 0); /* flagsHi */

    /* [MS-RDPBCGR] LICENSE_VALID_CLIENT_DATA */
    /* preamble (LICENSE_PREAMBLE) */
    out_uint8(s, ERROR_ALERT);
    out_uint8(s, PREAMBLE_VERSION_3_0);
    out_uint16_le(s, 16); /* Message size, including pre-amble */

    /* validClientMessage */
    /* From [MS-RDPBCGR] 2.2.12.1, dwStateTransition must be ST_NO_TRANSITION,
     * and the bbErrorInfo field must contain an empty blob of type
     * BB_ERROR_BLOB */
    out_uint32_le(s, STATUS_VALID_CLIENT); /* dwErrorCode */
    out_uint32_le(s, ST_NO_TRANSITION); /* dwStateTransition */
    out_uint16_le(s, BB_ERROR_BLOB);    /* wBlobType */
    out_uint16_le(s, 0);                /* wBlobLen */
    s_mark_end(s);

I'm pretty sure I'm standards-compliant with this, but I've been wrong about that before. I'd appreciate your comment on that.

@matt335672 matt335672 force-pushed the remove_licensing_exchange branch 2 times, most recently from 48e6fee to 0711d9d Compare July 3, 2024 11:16
Replaces the existing licensing exchange with a single PDU
saying the user will not issue a license.

This is necessary for clients on FIPS-compliant systems, as these
are unable to decode the licensing exchange packets, due to outdated
cyphers.
This commit changes the license response PDU to be constructed rather
than simply being contained as a binary blob.

Some constants in common/ms-rdpbcgr.h are renamed with the values
from the specification.
@matt335672 matt335672 marked this pull request as ready for review July 3, 2024 13:40
@matt335672 matt335672 mentioned this pull request Jul 10, 2024
3 tasks
@jsorg71
Copy link
Contributor

jsorg71 commented Jul 13, 2024

LGTM

@matt335672 matt335672 merged commit b6407a9 into neutrinolabs:devel Jul 15, 2024
14 checks passed
@matt335672 matt335672 deleted the remove_licensing_exchange branch July 15, 2024 08:02
@metalefty metalefty mentioned this pull request Jul 30, 2024
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.

xrdp is using RDP licensing
2 participants