Skip to content

feat(mstsgu): add NTLM RPC association setup - #1838

Merged
Marc-André Moreau (mamoreau-devolutions) merged 2 commits into
masterfrom
copilot/rpch-ntlm-bind
Aug 30, 2026
Merged

feat(mstsgu): add NTLM RPC association setup#1838
Marc-André Moreau (mamoreau-devolutions) merged 2 commits into
masterfrom
copilot/rpch-ntlm-bind

Conversation

@mamoreau-devolutions

@mamoreau-devolutions Marc-André Moreau (mamoreau-devolutions) commented Aug 29, 2026

Copy link
Copy Markdown
Contributor

Add DCE/RPC NTLM association setup for RD Gateway, including header-signing negotiation.
Keep its SSPI context independent from HTTP authentication contexts.

This excludes live RPC-over-HTTP transport and protected RPC traffic.

Add the DCE/RPC NTLM handshake needed for RD Gateway association setup.
Keep its SSPI context independent from HTTP authentication contexts.

This excludes live RPC-over-HTTP transport and protected RPC traffic.
Copilot AI balanced review requested due to automatic review settings August 29, 2026 23:44
@github-actions github-actions Bot added maintainer-required Maintainer review or intervention is required risk/unknown Risk could not be determined automatically; needs maintainer-level scrutiny size/L Size: up to 899 counted lines and 20 files; exceeds M in either measure labels Aug 29, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Adds NTLM-authenticated DCE/RPC association setup for RD Gateway while keeping HTTP authentication contexts separate.

Changes:

  • Adds NTLM bind, bind-ack, and rpc_auth_3 codecs.
  • Introduces independent SSPI association state.
  • Adds wire-format and state-transition tests and documentation.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
crates/ironrdp-mstsgu/src/rpc.rs Implements NTLM association state and authenticated RPC codecs.
crates/ironrdp-mstsgu/tests/rpc_pdu.rs Tests wire layouts, validation, and NTLM sequencing.
crates/ironrdp-mstsgu/tests/rpc_tsgu_stubs.rs Adds error stubs required by RPC tests.
crates/ironrdp-mstsgu/README.md Documents the new association support.
crates/ironrdp-mstsgu/CHANGELOG.md Records the added feature.
Suppressed comments (3)

crates/ironrdp-mstsgu/src/rpc.rs:1699

  • The security trailer is required to start on a 16-byte boundary relative to the PDU body (MS-RPCE 2.2.2.11), but the decoder only subtracts the declared padding and never checks the trailer offset. A packet can therefore use a self-consistent auth_pad_length while placing the trailer at a nonconforming offset and still be accepted. Validate body_with_padding.len() % 16 == 0 before decoding the body.
    let body_with_padding = &source[RPC_COMMON_HEADER_SIZE..trailer_offset];
    let padding_length = usize::from(trailer[2]);
    // The gateway profile accepts canonical zero-filled padding and uses
    // auth_pad_length rather than a fixed bind_ack layout.
    let body_length = body_with_padding
        .len()
        .checked_sub(padding_length)
        .ok_or(RpcPduError::InvalidAuthenticationPadding { actual: trailer[2] })?;

crates/ironrdp-mstsgu/src/rpc.rs:1682

  • MS-RPCE 2.2.2.11 says auth_reserved should be ignored on read. Rejecting a nonzero value makes the bind acknowledgement decoder unnecessarily incompatible with peers that populate this reserved byte. Ignore it during decoding; retaining zero on encode is correct.
    if trailer[3] != 0 {
        return Err(RpcPduError::NonZeroAuthenticationReserved { actual: trailer[3] });

crates/ironrdp-mstsgu/src/rpc.rs:1701

  • The authentication padding is alignment-only; MS-RPCE 2.2.2.11 specifies its length and placement but does not require its octets to be zero. Rejecting nonzero padding imposes an extra wire constraint and can reject otherwise conforming bind acknowledgements. Strip the declared padding without inspecting its contents.
    if body_with_padding[body_length..].iter().any(|&byte| byte != 0) {
        return Err(RpcPduError::NonZeroAuthenticationPadding);

Comment thread crates/ironrdp-mstsgu/src/rpc.rs Outdated
Accept a bind acknowledgement that clears header-signing support.
Expose the negotiated value for later protected RPC traffic.
@mamoreau-devolutions
Marc-André Moreau (mamoreau-devolutions) merged commit 2b40f47 into master Aug 30, 2026
46 of 60 checks passed
@mamoreau-devolutions
Marc-André Moreau (mamoreau-devolutions) deleted the copilot/rpch-ntlm-bind branch August 30, 2026 02:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

maintainer-required Maintainer review or intervention is required risk/unknown Risk could not be determined automatically; needs maintainer-level scrutiny size/L Size: up to 899 counted lines and 20 files; exceeds M in either measure

Development

Successfully merging this pull request may close these issues.

2 participants