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.

Sign up for GitHub

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

fix: make license parsing and protocol more resilient #436

Merged
merged 7 commits into from
Apr 25, 2024
Merged

fix: make license parsing and protocol more resilient #436

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
e9b2f3a
converts BlobType to resilient parsing style
Apr 6, 2024
becad4a
Makes the Licensing parsing and handling more resilient in order to m…
Apr 12, 2024
a494270
fixing xtask checks
Apr 15, 2024
f420d2a
updates Cargo.lock
Apr 16, 2024
e5bd2be
Merge branch 'master' into fix/resilient-blobtype
Apr 16, 2024
49dab7d
Merge branch 'master' into fix/resilient-blobtype
Apr 24, 2024
b6bdae7
Code Review
Apr 24, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 4 additions & 1 deletion crates/ironrdp-acceptor/src/connection.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ use ironrdp_pdu as pdu;
use ironrdp_svc::{StaticChannelSet, SvcServerProcessor};
use pdu::rdp::capability_sets::CapabilitySet;
use pdu::rdp::headers::ShareControlPdu;
use pdu::rdp::server_license::{LicensePdu, LicensingErrorMessage};
use pdu::write_buf::WriteBuf;
use pdu::{decode, gcc, mcs, nego, rdp};

Expand Down Expand Up @@ -392,7 +393,9 @@ impl Sequence for Acceptor {
early_capability,
channels,
} => {
let license = rdp::server_license::InitialServerLicenseMessage::new_status_valid_client_message();
let license: LicensePdu = LicensingErrorMessage::new_valid_client()
.map_err(ConnectorError::pdu)?
.into();

debug!(message = ?license, "Send");

Expand Down
7 changes: 5 additions & 2 deletions crates/ironrdp-connector/src/connection.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -700,13 +700,16 @@ fn create_client_info_pdu(config: &Config, routing_addr: &SocketAddr) -> rdp::Cl
};

// Default flags for all sessions
let mut flags = ClientInfoFlags::UNICODE
let mut flags = ClientInfoFlags::MOUSE
| ClientInfoFlags::MOUSE_HAS_WHEEL
| ClientInfoFlags::UNICODE
| ClientInfoFlags::DISABLE_CTRL_ALT_DEL
| ClientInfoFlags::LOGON_NOTIFY
| ClientInfoFlags::LOGON_ERRORS
| ClientInfoFlags::NO_AUDIO_PLAYBACK
| ClientInfoFlags::VIDEO_DISABLE
| ClientInfoFlags::ENABLE_WINDOWS_KEY;
| ClientInfoFlags::ENABLE_WINDOWS_KEY
| ClientInfoFlags::MAXIMIZE_SHELL;

if config.autologon {
flags |= ClientInfoFlags::AUTOLOGON;
Expand Down
49 changes: 26 additions & 23 deletions crates/ironrdp-connector/src/connection_finalization.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,34 +160,37 @@ impl Sequence for ConnectionFinalizationSequence {
debug!("Server Synchronize");
ConnectionFinalizationState::WaitForResponse
}
ShareDataPdu::Control(control_pdu) => match control_pdu.action {
finalization_messages::ControlAction::Cooperate => {
if control_pdu.grant_id == 0 && control_pdu.control_id == 0 {
debug!("Server Control (Cooperate)");
ShareDataPdu::Control(control_pdu) => {
match control_pdu.action {
finalization_messages::ControlAction::Cooperate => {
if control_pdu.grant_id == 0 && control_pdu.control_id == 0 {
debug!("Server Control (Cooperate)");
} else {
warn!("Server Control (Cooperate) had invalid grant_id or control_id, expected 0,0 but got {},{}", control_pdu.grant_id, control_pdu.control_id);
ibeckermayer marked this conversation as resolved.
Show resolved Hide resolved
}
ConnectionFinalizationState::WaitForResponse
} else {
return Err(general_err!("invalid Control Cooperate PDU"));
}
}
finalization_messages::ControlAction::GrantedControl => {
debug!(
control_pdu.grant_id,
control_pdu.control_id,
user_channel_id = self.user_channel_id,
SERVER_CHANNEL_ID
);

if control_pdu.grant_id == self.user_channel_id
&& control_pdu.control_id == u32::from(SERVER_CHANNEL_ID)
{
debug!("Server Control (Granted Control)");
finalization_messages::ControlAction::GrantedControl => {
debug!(
control_pdu.grant_id,
control_pdu.control_id,
user_channel_id = self.user_channel_id,
SERVER_CHANNEL_ID
);

if control_pdu.grant_id != self.user_channel_id {
warn!("Server Control (Granted Control) had invalid grant_id, expected {}, but got {}", self.user_channel_id, control_pdu.grant_id);
}

if control_pdu.control_id != u32::from(SERVER_CHANNEL_ID) {
warn!("Server Control (Granted Control) had invalid control_id, expected {}, but got {}", SERVER_CHANNEL_ID, control_pdu.control_id);
}

ConnectionFinalizationState::WaitForResponse
} else {
return Err(general_err!("invalid Granted Control PDU"));
}
_ => return Err(general_err!("unexpected control action")),
}
_ => return Err(general_err!("unexpected control action")),
},
}
ShareDataPdu::ServerSetErrorInfo(server_error_info::ServerSetErrorInfoPdu(error_info)) => {
match error_info {
server_error_info::ErrorInfo::ProtocolIndependentCode(
Expand Down
122 changes: 65 additions & 57 deletions crates/ironrdp-connector/src/license_exchange.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,13 @@
use core::fmt;
use std::mem;

use ironrdp_pdu::rdp::server_license;
use ironrdp_pdu::rdp::server_license::{self, LicensePdu, ServerLicenseError};
use ironrdp_pdu::write_buf::WriteBuf;
use ironrdp_pdu::PduHint;
use rand_core::{OsRng, RngCore as _};

use super::legacy;
use crate::{
encode_send_data_request, ConnectorError, ConnectorResult, ConnectorResultExt as _, Sequence, State, Written,
};
use crate::{encode_send_data_request, ConnectorResult, ConnectorResultExt as _, Sequence, State, Written};

#[derive(Default, Debug)]
#[non_exhaustive]
Expand Down Expand Up @@ -98,14 +96,12 @@ impl Sequence for LicenseExchangeSequence {

LicenseExchangeState::NewLicenseRequest => {
let send_data_indication_ctx = legacy::decode_send_data_indication(input)?;
let initial_server_license = send_data_indication_ctx
.decode_user_data::<server_license::InitialServerLicenseMessage>()
.with_context("decode initial server license PDU")?;
let license_pdu = send_data_indication_ctx
.decode_user_data::<LicensePdu>()
.with_context("decode during LicenseExchangeState::NewLicenseRequest")?;

debug!(message = ?initial_server_license, "Received");

match initial_server_license.message_type {
server_license::InitialMessageType::LicenseRequest(license_request) => {
match license_pdu {
LicensePdu::ServerLicenseRequest(license_request) => {
let mut client_random = [0u8; server_license::RANDOM_NUMBER_SIZE];
OsRng.fill_bytes(&mut client_random);

Expand All @@ -123,10 +119,10 @@ impl Sequence for LicenseExchangeSequence {
trace!(?encryption_data, "Successfully generated Client New License Request");
info!(message = ?new_license_request, "Send");

let written = encode_send_data_request(
let written = encode_send_data_request::<LicensePdu>(
send_data_indication_ctx.initiator_id,
send_data_indication_ctx.channel_id,
&new_license_request,
&new_license_request.into(),
output,
)?;

Expand Down Expand Up @@ -161,21 +157,34 @@ impl Sequence for LicenseExchangeSequence {
}
}
}
server_license::InitialMessageType::StatusValidClient(_) => {
info!("Server did not initiate license exchange");
(Written::Nothing, LicenseExchangeState::LicenseExchanged)
LicensePdu::LicensingErrorMessage(error_message) => {
if error_message.error_code == server_license::LicenseErrorCode::StatusValidClient {
info!("Server did not initiate license exchange");
(Written::Nothing, LicenseExchangeState::LicenseExchanged)
} else {
return Err(custom_err!(
"LicensingErrorMessage",
ServerLicenseError::from(error_message)
));
}
ibeckermayer marked this conversation as resolved.
Show resolved Hide resolved
}
_ => {
return Err(general_err!(
"unexpected PDU received during LicenseExchangeState::NewLicenseRequest"
));
}
}
}

LicenseExchangeState::PlatformChallenge { encryption_data } => {
let send_data_indication_ctx = legacy::decode_send_data_indication(input)?;

match send_data_indication_ctx
.decode_user_data::<server_license::ServerPlatformChallenge>()
.with_context("decode SERVER_PLATFORM_CHALLENGE")
{
Ok(challenge) => {
let license_pdu = send_data_indication_ctx
.decode_user_data::<LicensePdu>()
.with_context("decode during LicenseExchangeState::PlatformChallenge")?;

match license_pdu {
LicensePdu::ServerPlatformChallenge(challenge) => {
debug!(message = ?challenge, "Received");

let challenge_response =
Expand All @@ -188,10 +197,10 @@ impl Sequence for LicenseExchangeSequence {

debug!(message = ?challenge_response, "Send");

let written = encode_send_data_request(
let written = encode_send_data_request::<LicensePdu>(
send_data_indication_ctx.initiator_id,
send_data_indication_ctx.channel_id,
&challenge_response,
&challenge_response.into(),
output,
)?;

Expand All @@ -200,28 +209,35 @@ impl Sequence for LicenseExchangeSequence {
LicenseExchangeState::UpgradeLicense { encryption_data },
)
}
Err(error) => {
// FIXME(#269): weird control flow pattern
downcast_if_status_valid_client(error, |licensing_error_message| {
debug!(message = ?licensing_error_message, "Received");
LicensePdu::LicensingErrorMessage(error_message) => {
if error_message.error_code == server_license::LicenseErrorCode::StatusValidClient {
debug!(message = ?error_message, "Received");
info!("Client licensing completed");
(Written::Nothing, LicenseExchangeState::LicenseExchanged)
})?
} else {
return Err(custom_err!(
"LicensingErrorMessage",
ServerLicenseError::from(error_message)
));
}
}
ibeckermayer marked this conversation as resolved.
Show resolved Hide resolved
_ => {
return Err(general_err!(
"unexpected PDU received during LicenseExchangeState::PlatformChallenge"
));
}
}
}

LicenseExchangeState::UpgradeLicense { encryption_data } => {
let send_data_indication_ctx = legacy::decode_send_data_indication(input)?;

// FIXME: The ServerUpgradeLicense type is handling both SERVER_UPGRADE_LICENSE and SERVER_NEW_LICENSE PDUs.
// It’s expected that fixing #263 will also lead to a better alternative here.
let license_pdu = send_data_indication_ctx
.decode_user_data::<LicensePdu>()
.with_context("decode during SERVER_NEW_LICENSE/LicenseExchangeState::UpgradeLicense")?;

match send_data_indication_ctx
.decode_user_data::<server_license::ServerUpgradeLicense>()
.with_context("decode SERVER_NEW_LICENSE/SERVER_UPGRADE_LICENSE")
{
Ok(upgrade_license) => {
match license_pdu {
LicensePdu::ServerUpgradeLicense(upgrade_license) => {
debug!(message = ?upgrade_license, "Received");

upgrade_license
Expand All @@ -230,12 +246,21 @@ impl Sequence for LicenseExchangeSequence {

debug!("License verified with success");
}
Err(error) => {
// FIXME(#269): weird control flow pattern
downcast_if_status_valid_client(error, |licensing_error_message| {
debug!(message = ?licensing_error_message, "Received");
LicensePdu::LicensingErrorMessage(error_message) => {
if error_message.error_code == server_license::LicenseErrorCode::StatusValidClient {
debug!(message = ?error_message, "Received");
info!("Client licensing completed");
})?;
} else {
return Err(custom_err!(
"LicensingErrorMessage",
ServerLicenseError::from(error_message)
));
}
ibeckermayer marked this conversation as resolved.
Show resolved Hide resolved
}
_ => {
return Err(general_err!(
"unexpected PDU received during LicenseExchangeState::UpgradeLicense"
));
}
}

Expand All @@ -250,20 +275,3 @@ impl Sequence for LicenseExchangeSequence {
Ok(written)
}
}

// FIXME(#269): server_license::ServerLicenseError should not be retrieved from an error type.
fn downcast_if_status_valid_client<T, Fn>(error: ConnectorError, op: Fn) -> ConnectorResult<T>
where
Fn: FnOnce(&server_license::LicensingErrorMessage) -> T,
{
match std::error::Error::source(&error)
.and_then(|source| source.downcast_ref::<server_license::ServerLicenseError>())
{
Some(server_license::ServerLicenseError::ValidClientStatus(licensing_error_message))
if licensing_error_message.error_code == server_license::LicenseErrorCode::StatusValidClient =>
{
Ok(op(licensing_error_message))
}
_ => Err(error),
}
}
10 changes: 5 additions & 5 deletions crates/ironrdp-fuzzing/src/oracles/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,11 +35,11 @@ pub fn pdu_decode(data: &[u8]) {
let _ = decode::<gcc::ConferenceCreateRequest>(data);
let _ = decode::<gcc::ConferenceCreateResponse>(data);

let _ = decode::<server_license::ClientNewLicenseRequest>(data);
let _ = decode::<server_license::ClientPlatformChallengeResponse>(data);
let _ = decode::<server_license::InitialServerLicenseMessage>(data);
let _ = decode::<server_license::ServerLicenseRequest>(data);
let _ = decode::<server_license::ServerPlatformChallenge>(data);
let _ = decode::<server_license::LicensePdu>(data);
let _ = decode::<server_license::LicensePdu>(data);
let _ = decode::<server_license::LicensePdu>(data);
let _ = decode::<server_license::LicensePdu>(data);
let _ = decode::<server_license::LicensePdu>(data);
ibeckermayer marked this conversation as resolved.
Show resolved Hide resolved

let _ = decode::<vc::ChannelPduHeader>(data);

Expand Down
Loading