Skip to content

fix(ironrdp-pdu): tolerate unknown security header flags in BasicSecurityHeader#1458

Open
Anton Isaiev (totoshko88) wants to merge 1 commit into
Devolutions:masterfrom
totoshko88:fix/tolerate-unknown-security-header-flags
Open

fix(ironrdp-pdu): tolerate unknown security header flags in BasicSecurityHeader#1458
Anton Isaiev (totoshko88) wants to merge 1 commit into
Devolutions:masterfrom
totoshko88:fix/tolerate-unknown-security-header-flags

Conversation

@totoshko88

Copy link
Copy Markdown
Contributor

Summary

Use from_bits_truncate() instead of from_bits() when decoding BasicSecurityHeader flags. Some servers (e.g., Windows Server 2019 with RDS licensing / RD Connection Broker) send security header flag combinations that include bits not defined in the current bitflags enum. The strict from_bits() rejected these as invalid, causing connection failure during the UpgradeLicense license exchange phase.

This matches FreeRDP behavior which masks for known flags without rejecting the PDU when unrecognized bits are present.

What was tested

  • Existing unit tests pass (cargo xtask check tests -v)
  • Lints pass (cargo xtask check lints -v)

Closes #1457

…rityHeader

Use from_bits_truncate() instead of from_bits() when decoding
BasicSecurityHeader flags. Some servers (e.g., Windows Server 2019
with RDS licensing / RD Connection Broker) send security header flag
combinations that include bits not defined in the current bitflags
enum. The strict from_bits() rejected these as invalid, causing
connection failure during the UpgradeLicense license exchange phase.

This matches FreeRDP behavior which masks for known flags without
rejecting the PDU when unrecognized bits are present.

Closes Devolutions#1457
@glamberson

Copy link
Copy Markdown
Contributor

Thanks for chasing this down, Anton Isaiev (@totoshko88), and for the detailed repro writeup. I traced the decode path against current master and I think the fix is landing in the wrong spot, so let me lay out what I found before it goes further.

The PR's premise is that the server sets flag bits not defined in BasicSecurityHeaderFlags. That turns out not to be possible: the bitflags cover all 16 bits of the u16 (EXCHANGE_PKT 0x0001 through FLAGSHI_VALID 0x8000, defined since 2019), so from_bits() can never return None for any u16. With every bit defined, from_bits_truncate() is identical to from_bits(), so this change is a no-op for decode behavior. (Also worth flagging: #1144 deliberately moved the crate off from_bits_truncate to from_bits_retain, so reintroducing truncate would cut against that.)

So BasicSecurityHeader::decode isn't what's rejecting your server. The error you posted:

field: "securityHeaderFlags", reason: "invalid security header flags"

comes from LicenseHeader::decode, one line below the header decode, at the LICENSE_PKT presence check:

let security_header = BasicSecurityHeader::decode(src)?;   // succeeds
if !security_header.flags.contains(BasicSecurityHeaderFlags::LICENSE_PKT) {
    return Err(invalid_field_err!("securityHeaderFlags", "invalid security header flags"));
}

And LicensePdu::decode runs every license PDU through LicenseHeader::decode first, so this gate fires before any message-type routing. The real signal is that your Win2019 + RDS/Broker server's license-stage PDU doesn't have LICENSE_PKT (0x0080) set in the bytes we read as the header, which fits the "different flags with RDS licensing" you observed.

Could you grab a hex dump of the failing PDU, the first ~8 bytes at the point LicenseHeader::decode runs (the basic security header is flags: u16 + flagsHi: u16)? That settles in one shot whether:

  • LICENSE_PKT is genuinely absent, in which case the fix is to relax that hard requirement in LicenseHeader::decode to match FreeRDP's leniency and let the existing LicensingErrorMessage / StatusValidClient path handle it, or
  • the header is offset and we're reading the wrong two bytes, a framing fix similar in shape to what you found in fix(displaycontrol): decode the full headered DISPLAYCONTROL_CAPS_PDU #1442.

Happy to help land whichever it turns out to be. Enterprise RDS/Broker interop is exactly the kind of gap worth closing.

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

Labels

None yet

Development

Successfully merging this pull request may close these issues.

LicenseHeader decode rejects valid security header flags from Windows Server 2019 with RDS licensing (UpgradeLicense)

2 participants