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

vmbus_client: don't request confidential channel support. #917

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

vmbus_client: don't request confidential channel support. #917

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b22ddb3
vmbus_client: don't request confidential channel support.
SvenGroot Feb 27, 2025
8c8e2ee
Decentralize supported feature flags.
SvenGroot Feb 27, 2025
4ce1dde
Fix confidential channel flag when restoring.
SvenGroot Feb 27, 2025
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
24 changes: 14 additions & 10 deletions vm/devices/vmbus/vmbus_client/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,11 @@ use zerocopy::KnownLayout;
const SINT: u8 = 2;
const VTL: u8 = 0;
const SUPPORTED_VERSIONS: &[Version] = &[Version::Iron, Version::Copper];
const SUPPORTED_FEATURE_FLAGS: FeatureFlags = FeatureFlags::all();
const SUPPORTED_FEATURE_FLAGS: FeatureFlags = FeatureFlags::new()
.with_guest_specified_signal_parameters(true)
.with_channel_interrupt_redirection(true)
.with_modify_connection(true)
.with_client_id(true);

/// The client interface synic events.
pub trait SynicEventClient: Send + Sync {
Expand Down Expand Up @@ -1528,13 +1532,13 @@ mod tests {
padding: 0,
selected_version_or_connection_id: 0,
},
supported_features: FeatureFlags::all().into(),
supported_features: SUPPORTED_FEATURE_FLAGS.into(),
},
));

let version = recv.await.unwrap().unwrap();
assert_eq!(version.version, Version::Copper);
assert_eq!(version.feature_flags, FeatureFlags::all());
assert_eq!(version.feature_flags, SUPPORTED_FEATURE_FLAGS);
}

async fn get_channel(&mut self, client: &mut VmbusClient) -> OfferInfo {
Expand Down Expand Up @@ -1696,7 +1700,7 @@ mod tests {
interrupt_page_or_target_info: TargetInfo::new()
.with_sint(2)
.with_vtl(0)
.with_feature_flags(FeatureFlags::all().into())
.with_feature_flags(SUPPORTED_FEATURE_FLAGS.into())
.into(),
parent_to_child_monitor_page_gpa: 0,
child_to_parent_monitor_page_gpa: 0,
Expand All @@ -1723,7 +1727,7 @@ mod tests {
interrupt_page_or_target_info: TargetInfo::new()
.with_sint(2)
.with_vtl(0)
.with_feature_flags(FeatureFlags::all().into())
.with_feature_flags(SUPPORTED_FEATURE_FLAGS.into())
.into(),
parent_to_child_monitor_page_gpa: 0,
child_to_parent_monitor_page_gpa: 0,
Expand All @@ -1741,14 +1745,14 @@ mod tests {
padding: 0,
selected_version_or_connection_id: 0,
},
supported_features: FeatureFlags::all().into_bits(),
supported_features: SUPPORTED_FEATURE_FLAGS.into_bits(),
},
));

let version = recv.await.unwrap().unwrap();

assert_eq!(version.version, Version::Copper);
assert_eq!(version.feature_flags, FeatureFlags::all());
assert_eq!(version.feature_flags, SUPPORTED_FEATURE_FLAGS);
}

#[async_test]
Expand All @@ -1768,7 +1772,7 @@ mod tests {
interrupt_page_or_target_info: TargetInfo::new()
.with_sint(2)
.with_vtl(0)
.with_feature_flags(FeatureFlags::all().into())
.with_feature_flags(SUPPORTED_FEATURE_FLAGS.into())
.into(),
parent_to_child_monitor_page_gpa: 0,
child_to_parent_monitor_page_gpa: 0,
Expand Down Expand Up @@ -1822,7 +1826,7 @@ mod tests {
interrupt_page_or_target_info: TargetInfo::new()
.with_sint(2)
.with_vtl(0)
.with_feature_flags(FeatureFlags::all().into())
.with_feature_flags(SUPPORTED_FEATURE_FLAGS.into())
.into(),
parent_to_child_monitor_page_gpa: 0,
child_to_parent_monitor_page_gpa: 0,
Expand All @@ -1849,7 +1853,7 @@ mod tests {
interrupt_page_or_target_info: TargetInfo::new()
.with_sint(2)
.with_vtl(0)
.with_feature_flags(FeatureFlags::all().into())
.with_feature_flags(SUPPORTED_FEATURE_FLAGS.into())
.into(),
parent_to_child_monitor_page_gpa: 0,
child_to_parent_monitor_page_gpa: 0,
Expand Down
3 changes: 2 additions & 1 deletion vm/devices/vmbus/vmbus_client/src/saved_state.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
use crate::OfferInfo;
use crate::RestoreError;
use crate::RestoredChannel;
use crate::SUPPORTED_FEATURE_FLAGS;
use guid::Guid;
use mesh::payload::Protobuf;
use vmbus_channel::bus::OfferKey;
Expand Down Expand Up @@ -198,7 +199,7 @@ impl TryFrom<ClientState> for super::ClientState {
.ok_or(RestoreError::UnsupportedVersion(version))?;

let feature_flags = FeatureFlags::from(feature_flags);
if feature_flags.contains_unsupported_bits() {
if !SUPPORTED_FEATURE_FLAGS.contains(feature_flags) {
return Err(RestoreError::UnsupportedFeatureFlags(feature_flags.into()));
}

Expand Down
14 changes: 3 additions & 11 deletions vm/devices/vmbus/vmbus_core/src/protocol.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -175,17 +175,9 @@ pub struct FeatureFlags {
}

impl FeatureFlags {
pub const fn all() -> Self {
Self::new()
.with_guest_specified_signal_parameters(true)
.with_channel_interrupt_redirection(true)
.with_modify_connection(true)
.with_client_id(true)
.with_confidential_channels(true)
}

pub fn contains_unsupported_bits(&self) -> bool {
u32::from(*self) & !u32::from(Self::all()) != 0
/// Returns true if `other` contains only flags that are also set in `self`.
pub fn contains(&self, other: FeatureFlags) -> bool {
self.into_bits() & other.into_bits() == other.into_bits()
}
}

Expand Down
33 changes: 21 additions & 12 deletions vm/devices/vmbus/vmbus_server/src/channels.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1196,6 +1196,14 @@ static SUPPORTED_VERSIONS: &[Version] = &[
Version::Copper,
];

// Feature flags that are always supported.
// N.B. Confidential channels are conditionally supported if running in the paravisor.
const SUPPORTED_FEATURE_FLAGS: FeatureFlags = FeatureFlags::new()
.with_guest_specified_signal_parameters(true)
.with_channel_interrupt_redirection(true)
.with_modify_connection(true)
.with_client_id(true);

/// An error that occurred while mapping the interrupt page.
#[derive(Error, Debug)]
pub enum InterruptPageError {
Expand Down Expand Up @@ -2247,16 +2255,17 @@ impl<'a, N: 'a + Notifier> ServerWithNotifier<'a, N> {
// The max version and features may be limited in order to test older protocol versions.
//
// N.B. Confidential channels should only be enabled if the connection is trusted.
let max_supported_flags =
SUPPORTED_FEATURE_FLAGS.with_confidential_channels(request.trusted);

if let Some(max_version) = self.inner.max_version {
if version as u32 > max_version.version {
return None;
}

max_version.feature_flags.with_confidential_channels(
max_version.feature_flags.confidential_channels() && request.trusted,
)
max_supported_flags & max_version.feature_flags
} else {
FeatureFlags::all().with_confidential_channels(request.trusted)
max_supported_flags
}
} else {
FeatureFlags::new()
Expand Down Expand Up @@ -3931,7 +3940,7 @@ mod tests {
server.with_notifier(notifier).complete_initiate_contact(
ModifyConnectionResponse::Supported(
protocol::ConnectionState::SUCCESSFUL,
FeatureFlags::all(),
SUPPORTED_FEATURE_FLAGS,
),
);

Expand Down Expand Up @@ -4196,7 +4205,7 @@ mod tests {
.with_notifier(&mut notifier)
.complete_initiate_contact(ModifyConnectionResponse::Supported(
protocol::ConnectionState::SUCCESSFUL,
FeatureFlags::all(),
SUPPORTED_FEATURE_FLAGS,
));

let version_response = protocol::VersionResponse {
Expand Down Expand Up @@ -4393,7 +4402,7 @@ mod tests {
.with_notifier(&mut notifier)
.complete_initiate_contact(ModifyConnectionResponse::Supported(
protocol::ConnectionState::SUCCESSFUL,
FeatureFlags::all(),
SUPPORTED_FEATURE_FLAGS,
));

// Discard the version response message.
Expand Down Expand Up @@ -4479,7 +4488,7 @@ mod tests {
self.c()
.complete_modify_connection(ModifyConnectionResponse::Supported(
protocol::ConnectionState::SUCCESSFUL,
FeatureFlags::all(),
SUPPORTED_FEATURE_FLAGS,
));
}

Expand Down Expand Up @@ -4676,7 +4685,7 @@ mod tests {
self.c()
.complete_initiate_contact(ModifyConnectionResponse::Supported(
protocol::ConnectionState::SUCCESSFUL,
FeatureFlags::all(),
SUPPORTED_FEATURE_FLAGS,
));

let version = self.version.unwrap();
Expand Down Expand Up @@ -4727,7 +4736,7 @@ mod tests {
env.c()
.complete_initiate_contact(ModifyConnectionResponse::Supported(
protocol::ConnectionState::SUCCESSFUL,
FeatureFlags::all(),
SUPPORTED_FEATURE_FLAGS,
));
let offer_id3 = env.offer(3);
env.c().handle_request_offers().unwrap();
Expand Down Expand Up @@ -5008,7 +5017,7 @@ mod tests {
env.c()
.complete_modify_connection(ModifyConnectionResponse::Supported(
protocol::ConnectionState::SUCCESSFUL,
FeatureFlags::all(),
SUPPORTED_FEATURE_FLAGS,
));

env.notifier
Expand Down Expand Up @@ -5166,7 +5175,7 @@ mod tests {
env.c()
.complete_modify_connection(ModifyConnectionResponse::Supported(
protocol::ConnectionState::FAILED_UNKNOWN_FAILURE,
FeatureFlags::all(),
SUPPORTED_FEATURE_FLAGS,
));

env.notifier
Expand Down
14 changes: 9 additions & 5 deletions vm/devices/vmbus/vmbus_server/src/channels/saved_state.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use super::OfferError;
use super::OfferParamsInternal;
use super::OfferedInfo;
use super::RestoreState;
use super::SUPPORTED_FEATURE_FLAGS;
use guid::Guid;
use mesh::payload::Protobuf;
use std::fmt::Display;
Expand Down Expand Up @@ -315,15 +316,16 @@ impl VersionInfo {
}
}

fn restore(self) -> Result<vmbus_core::VersionInfo, RestoreError> {
fn restore(self, trusted: bool) -> Result<vmbus_core::VersionInfo, RestoreError> {
let version = super::SUPPORTED_VERSIONS
.iter()
.find(|v| self.version == **v as u32)
.copied()
.ok_or(RestoreError::UnsupportedVersion(self.version))?;

let feature_flags = FeatureFlags::from(self.feature_flags);
if feature_flags.contains_unsupported_bits() {
let supported_flags = SUPPORTED_FEATURE_FLAGS.with_confidential_channels(trusted);
if !supported_flags.contains(feature_flags) {
return Err(RestoreError::UnsupportedFeatureFlags(feature_flags.into()));
}

Expand Down Expand Up @@ -429,7 +431,7 @@ impl Connection {
trusted,
} => super::ConnectionState::Connecting {
info: super::ConnectionInfo {
version: version.restore()?,
version: version.restore(trusted)?,
trusted,
interrupt_page,
monitor_page: monitor_page.map(MonitorPageGpas::restore),
Expand All @@ -450,7 +452,7 @@ impl Connection {
client_id,
trusted,
} => super::ConnectionState::Connected(super::ConnectionInfo {
version: version.restore()?,
version: version.restore(trusted)?,
trusted,
offers_sent,
interrupt_page,
Expand Down Expand Up @@ -805,7 +807,9 @@ impl ReservedState {
}

fn restore(&self) -> Result<super::ReservedState, RestoreError> {
let version = self.version.clone().restore().map_err(|e| match e {
// We don't know if the connection when the channel was reserved was trusted, so assume it
// was for what feature flags are accepted here; it doesn't affect any actual behavior.
let version = self.version.clone().restore(true).map_err(|e| match e {
RestoreError::UnsupportedVersion(v) => RestoreError::UnsupportedReserveVersion(v),
RestoreError::UnsupportedFeatureFlags(f) => {
RestoreError::UnsupportedReserveFeatureFlags(f)
Expand Down
2 changes: 1 addition & 1 deletion vm/devices/vmbus/vmbus_server/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -447,7 +447,7 @@ impl<'a, T: Spawn> VmbusServerBuilder<'a, T> {
.map(|_| {
ModifyConnectionResponse::Supported(
protocol::ConnectionState::SUCCESSFUL,
protocol::FeatureFlags::all(),
protocol::FeatureFlags::from_bits(u32::MAX),
)
})
.boxed()
Expand Down