feat: preserve RDP negotiation failure details in RDCleanPath error responses (#930)

* Both web and desktop clients check for X.224 negotiation failure data
in RDCleanPath error responses before falling back to generic errors
* When X.224 Connection Confirm failure is found, convert to specific
NegotiationFailure error type instead of generic RDCleanPath error
* Enable clients to show meaningful error messages like "CredSSP
authentication required" instead of generic connection failures
* Maintain backward compatibility - existing proxies sending empty
x224_connection_pdu continue working as before
* Helper for proxies creating an RDCleanPath error with server response
This commit is contained in:
Gabriel Bauman 2025-08-28 04:06:21 -07:00 committed by GitHub
parent ae99d14a69
commit ca11e338d7
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 120 additions and 5 deletions

View file

@ -410,6 +410,26 @@ where
debug!(message = ?rdcleanpath_res, "Received RDCleanPath PDU");
// Check for negotiation failure data before consuming the PDU
if let Some(x224_data) = &rdcleanpath_res.x224_connection_pdu {
if let Some(_error) = &rdcleanpath_res.error {
// Try to decode as X.224 Connection Confirm to extract negotiation failure details.
if let Ok(x224_confirm) = ironrdp_core::decode::<
ironrdp::pdu::x224::X224<ironrdp::pdu::nego::ConnectionConfirm>,
>(x224_data.as_bytes())
{
if let ironrdp::pdu::nego::ConnectionConfirm::Failure { code } = x224_confirm.0 {
// Convert to negotiation failure instead of generic RDCleanPath error.
let negotiation_failure = connector::NegotiationFailure::from(code);
return Err(connector::ConnectorError::new(
"RDP negotiation failed",
connector::ConnectorErrorKind::Negotiation(negotiation_failure),
));
}
}
}
}
let (x224_connection_response, server_cert_chain) = match rdcleanpath_res
.into_enum()
.map_err(|e| connector::custom_err!("invalid RDCleanPath PDU", e))?
@ -424,9 +444,30 @@ where
server_cert_chain,
server_addr: _,
} => (x224_connection_response, server_cert_chain),
ironrdp_rdcleanpath::RDCleanPath::Err(error) => {
ironrdp_rdcleanpath::RDCleanPath::GeneralErr(error) => {
// Fallback to generic RDCleanPath error if no negotiation failure data found
return Err(connector::custom_err!("received an RDCleanPath error", error));
}
ironrdp_rdcleanpath::RDCleanPath::NegotiationErr {
x224_connection_response,
} => {
// Try to decode as X.224 Connection Confirm to extract negotiation failure details.
if let Ok(x224_confirm) = ironrdp_core::decode::<
ironrdp::pdu::x224::X224<ironrdp::pdu::nego::ConnectionConfirm>,
>(&x224_connection_response)
{
if let ironrdp::pdu::nego::ConnectionConfirm::Failure { code } = x224_confirm.0 {
// Convert to negotiation failure instead of generic RDCleanPath error.
let negotiation_failure = connector::NegotiationFailure::from(code);
return Err(connector::ConnectorError::new(
"RDP negotiation failed",
connector::ConnectorErrorKind::Negotiation(negotiation_failure),
));
}
}
// Fallback to generic error if we can't decode the negotiation failure.
return Err(connector::general_err!("received an RDCleanPath negotiation error"));
}
};
let connector::ClientConnectorState::ConnectionInitiationWaitConfirm { .. } = connector.state else {

View file

@ -13,6 +13,7 @@ pub const BASE_VERSION: u64 = 3389;
pub const VERSION_1: u64 = BASE_VERSION + 1;
pub const GENERAL_ERROR_CODE: u16 = 1;
pub const NEGOTIATION_ERROR_CODE: u16 = 2;
#[derive(Clone, Debug, Eq, PartialEq, der::Sequence)]
#[asn1(tag_mode = "EXPLICIT")]
@ -258,6 +259,34 @@ impl RDCleanPathPdu {
}
}
/// Create a negotiation error response that includes the server's X.224 negotiation response.
///
/// This allows clients to extract specific negotiation failure details
/// (like "CredSSP required") from the server's original response.
///
/// # Example
/// ```rust
/// use ironrdp_rdcleanpath::RDCleanPathPdu;
///
/// // Server rejected connection with "CredSSP required" - preserve this info
/// let server_response = vec![/* X.224 Connection Confirm with failure code */];
/// let error_pdu = RDCleanPathPdu::new_negotiation_error(server_response)?;
/// # Ok::<(), der::Error>(())
/// ```
pub fn new_negotiation_error(server_x224_response: Vec<u8>) -> der::Result<Self> {
Ok(Self {
version: VERSION_1,
error: Some(RDCleanPathErr {
error_code: NEGOTIATION_ERROR_CODE,
http_status_code: None,
wsa_last_error: None,
tls_alert_code: None,
}),
x224_connection_pdu: Some(OctetString::new(server_x224_response)?),
..Self::default()
})
}
pub fn to_der(&self) -> der::Result<Vec<u8>> {
der::Encode::to_der(self)
}
@ -278,7 +307,10 @@ pub enum RDCleanPath {
server_cert_chain: Vec<OctetString>,
server_addr: String,
},
Err(RDCleanPathErr),
GeneralErr(RDCleanPathErr),
NegotiationErr {
x224_connection_response: Vec<u8>,
},
}
impl RDCleanPath {
@ -323,7 +355,13 @@ impl TryFrom<RDCleanPathPdu> for RDCleanPath {
server_addr,
}
} else {
Self::Err(pdu.error.ok_or(MissingRDCleanPathField("error"))?)
let error = pdu.error.ok_or(MissingRDCleanPathField("error"))?;
match (error.error_code, pdu.x224_connection_pdu) {
(NEGOTIATION_ERROR_CODE, Some(x224_pdu)) => Self::NegotiationErr {
x224_connection_response: x224_pdu.as_bytes().to_vec(),
},
_ => Self::GeneralErr(error),
}
};
Ok(rdcleanpath)
@ -359,11 +397,24 @@ impl From<RDCleanPath> for RDCleanPathPdu {
server_addr: Some(server_addr),
..Default::default()
},
RDCleanPath::Err(error) => Self {
RDCleanPath::GeneralErr(error) => Self {
version: VERSION_1,
error: Some(error),
..Default::default()
},
RDCleanPath::NegotiationErr {
x224_connection_response,
} => Self {
version: VERSION_1,
error: Some(RDCleanPathErr {
error_code: NEGOTIATION_ERROR_CODE,
http_status_code: None,
wsa_last_error: None,
tls_alert_code: None,
}),
x224_connection_pdu: Some(OctetString::new(x224_connection_response).unwrap()),
..Default::default()
},
}
}
}

View file

@ -1089,12 +1089,35 @@ where
server_cert_chain,
server_addr: _,
} => (x224_connection_response, server_cert_chain),
ironrdp_rdcleanpath::RDCleanPath::Err(error) => {
ironrdp_rdcleanpath::RDCleanPath::GeneralErr(error) => {
return Err(
IronError::from(anyhow::Error::new(error).context("received an RDCleanPath error"))
.with_kind(IronErrorKind::RDCleanPath),
);
}
ironrdp_rdcleanpath::RDCleanPath::NegotiationErr {
x224_connection_response,
} => {
// Try to decode as X.224 Connection Confirm to extract negotiation failure details.
if let Ok(x224_confirm) = ironrdp_core::decode::<
ironrdp::pdu::x224::X224<ironrdp::pdu::nego::ConnectionConfirm>,
>(&x224_connection_response)
{
if let ironrdp::pdu::nego::ConnectionConfirm::Failure { code } = x224_confirm.0 {
// Convert to negotiation failure instead of generic RDCleanPath error.
let negotiation_failure = connector::NegotiationFailure::from(code);
return Err(IronError::from(
anyhow::Error::new(negotiation_failure).context("RDP negotiation failed"),
)
.with_kind(IronErrorKind::NegotiationFailure));
}
}
// Fallback to generic error if we can't decode the negotiation failure.
return Err(
IronError::from(anyhow::Error::msg("received an RDCleanPath negotiation error"))
.with_kind(IronErrorKind::RDCleanPath),
);
}
};
let connector::ClientConnectorState::ConnectionInitiationWaitConfirm { .. } = connector.state else {