vmgs: dont upgrade vmgs encryption on auto and allow fallback for gspkey

openhcl/underhill_attestation/src/lib.rs

@@ -709,6 +709,7 @@ async fn get_derived_keys(
         };
 
     // Handle various sources of Guest State Protection
+    let existing_unencrypted = !vmgs.is_encrypted() && !vmgs.was_provisioned_this_boot();
     let is_gsp_by_id = key_protector_by_id.found_id && key_protector_by_id.inner.ported != 1;
     let is_gsp = key_protector.gsp[ingress_idx].gsp_length != 0;
     tracing::info!(
@@ -722,7 +723,9 @@ async fn get_derived_keys(
     let mut requires_gsp_by_id = is_gsp_by_id;
 
     // Attempt GSP
-    let (gsp_response, no_gsp, requires_gsp) = {
+    let (gsp_response, gsp_available, no_gsp, requires_gsp) = {
+        tracing::info!(CVM_ALLOWED, "attempting GSP");
+
         let response = get_gsp_data(get, key_protector).await;
 
         tracing::info!(
@@ -734,38 +737,59 @@ async fn get_derived_keys(
             "GSP response"
         );
 
-        let no_gsp = response.extended_status_flags.no_rpc_server()
-            || response.encrypted_gsp.length == 0
+        let no_gsp_available =
+            response.extended_status_flags.no_rpc_server() || response.encrypted_gsp.length == 0;
+
+        let no_gsp = no_gsp_available
+            // disable if auto and pre-existing guest state is not encrypted or
+            // encrypted using GspById to prevent encryption changes without
+            // explicit intent
+            || (matches!(
+                guest_state_encryption_policy,
+                GuestStateEncryptionPolicy::Auto
+            ) && (is_gsp_by_id || existing_unencrypted))
+            // disable per encryption policy (first boot only, unless strict)
             || (matches!(
                 guest_state_encryption_policy,
                 GuestStateEncryptionPolicy::GspById | GuestStateEncryptionPolicy::None
             ) && (!is_gsp || strict_encryption_policy));
 
         let requires_gsp = is_gsp
             || response.extended_status_flags.requires_rpc_server()
-            || matches!(
+            || (matches!(
                 guest_state_encryption_policy,
                 GuestStateEncryptionPolicy::GspKey
-            );
+            ) && strict_encryption_policy);
 
         // If the VMGS is encrypted, but no key protection data is found,
         // assume GspById encryption is enabled, but no ID file was written.
         if is_encrypted && !requires_gsp_by_id && !requires_gsp && !found_dek {
             requires_gsp_by_id = true;
         }
 
-        (response, no_gsp, requires_gsp)
+        (response, !no_gsp_available, no_gsp, requires_gsp)
     };
 
     // Attempt GSP By Id protection if GSP is not available, when changing
     // schemes, or as requested
-    let (gsp_response_by_id, no_gsp_by_id) = if no_gsp || requires_gsp_by_id {
+    let (gsp_response_by_id, gsp_by_id_available, no_gsp_by_id) = if no_gsp || requires_gsp_by_id {
+        tracing::info!(CVM_ALLOWED, "attempting GSP By Id");
+
         let gsp_response_by_id = get
             .guest_state_protection_data_by_id()
             .await
             .map_err(GetDerivedKeysError::FetchGuestStateProtectionById)?;
 
-        let no_gsp_by_id = gsp_response_by_id.extended_status_flags.no_registry_file()
+        let no_gsp_by_id_available = gsp_response_by_id.extended_status_flags.no_registry_file();
+
+        let no_gsp_by_id = no_gsp_by_id_available
+            // disable if auto and pre-existing guest state is unencrypted
+            // to prevent encryption changes without explicit intent
+            || (matches!(
+                guest_state_encryption_policy,
+                GuestStateEncryptionPolicy::Auto
+            ) && existing_unencrypted)
+            // disable per encryption policy (first boot only, unless strict)
             || (matches!(
                 guest_state_encryption_policy,
                 GuestStateEncryptionPolicy::None
@@ -775,9 +799,13 @@ async fn get_derived_keys(
             Err(GetDerivedKeysError::GspByIdRequiredButNotFound)?
         }
 
-        (gsp_response_by_id, no_gsp_by_id)
+        (
+            gsp_response_by_id,
+            Some(!no_gsp_by_id_available),
+            no_gsp_by_id,
+        )
     } else {
-        (GuestStateProtectionById::new_zeroed(), true)
+        (GuestStateProtectionById::new_zeroed(), None, true)
     };
 
     // If sources of encryption used last are missing, attempt to unseal VMGS key with hardware key
@@ -862,7 +890,9 @@ async fn get_derived_keys(
     tracing::info!(
         CVM_ALLOWED,
         kek = !no_kek,
+        gsp_available,
         gsp = !no_gsp,
+        gsp_by_id_available = ?gsp_by_id_available,
         gsp_by_id = !no_gsp_by_id,
         "Encryption sources"
     );
@@ -952,18 +982,21 @@ async fn get_derived_keys(
         if no_kek && no_gsp {
             if matches!(
                 guest_state_encryption_policy,
-                GuestStateEncryptionPolicy::None
+                GuestStateEncryptionPolicy::GspById | GuestStateEncryptionPolicy::Auto
             ) {
+                tracing::info!(CVM_ALLOWED, "Using GspById");
+            } else {
                 // Log a warning here to indicate that the VMGS state is out of
                 // sync with the VM's configuration.
                 //
-                // This should only happen if the VM is configured to
-                // have no encryption, but it already has GspById encryption
-                // and strict encryption policy is disabled.
+                // This should only happen if strict encryption policy is
+                // disabled and one of the following is true:
+                // - The VM is configured to have no encryption, but it already
+                //   has GspById encryption.
+                // - The VM is configured to use GspKey, but GspKey is not
+                //   available and GspById is.
                 tracing::warn!(CVM_ALLOWED, "Allowing GspById");
-            } else {
-                tracing::info!(CVM_ALLOWED, "Using GspById");
-            }
+            };
 
             // Not required for Id protection
             key_protector_settings.should_write_kp = false;
@@ -1044,7 +1077,7 @@ async fn get_derived_keys(
 
             key_protector_settings.encrypt_gsp_type = GspType::GspKey;
         } else {
-            tracing::info!(CVM_ALLOWED, "Using GSP.");
+            tracing::info!(CVM_ALLOWED, "Using existing GSP.");
 
             ingress_seed = Some(
                 gsp_response.decrypted_gsp[ingress_idx].buffer
@@ -1113,17 +1146,17 @@ async fn get_derived_keys(
 
     if matches!(
         guest_state_encryption_policy,
-        GuestStateEncryptionPolicy::None | GuestStateEncryptionPolicy::GspById
+        GuestStateEncryptionPolicy::GspKey | GuestStateEncryptionPolicy::Auto
     ) {
+        tracing::info!(CVM_ALLOWED, "Using Gsp");
+    } else {
         // Log a warning here to indicate that the VMGS state is out of
         // sync with the VM's configuration.
         //
         // This should only happen if the VM is configured to have no
         // encryption or GspById encryption, but it already has GspKey
         // encryption and strict encryption policy is disabled.
         tracing::warn!(CVM_ALLOWED, "Allowing Gsp");
-    } else {
-        tracing::info!(CVM_ALLOWED, "Using Gsp");
     }
 
     Ok(DerivedKeyResult {

vm/devices/get/get_protocol/src/dps_json.rs

@@ -106,9 +106,10 @@ pub enum GuestStateLifetime {
 pub enum GuestStateEncryptionPolicy {
     /// Use the best encryption available, allowing fallback.
     ///
-    /// VMs will be created as or migrated to the best encryption available,
+    /// VMs will be created using the best encryption available,
     /// attempting GspKey, then GspById, and finally leaving the data
-    /// unencrypted if neither are available.
+    /// unencrypted if neither are available. VMs will not be migrated
+    /// to a different encryption method.
     #[default]
     Auto,
     /// Prefer (or require, if strict) no encryption.
@@ -123,11 +124,11 @@ pub enum GuestStateEncryptionPolicy {
     /// strict encryption policy is enabled. Fails if the data cannot be
     /// encrypted.
     GspById,
-    /// Require GspKey.
+    /// Prefer (or require, if strict) GspKey.
     ///
-    /// VMs will be created as or migrated to GspKey. Fails if GspKey is
-    /// not available. Strict encryption policy has no effect here since
-    /// GspKey is currently the most secure policy.
+    /// VMs will be created as or migrated to GspKey. GspById encryption will
+    /// be used if GspKey is unavailable unless strict encryption policy is
+    /// enabled. Fails if the data cannot be encrypted.
     GspKey,
     /// Use hardware sealing
     // TODO: update this doc comment once hardware sealing is implemented

vm/vmgs/vmgs/src/vmgs_impl.rs

@@ -108,6 +108,7 @@ pub struct Vmgs {
     metadata_key: VmgsDatastoreKey,
     #[cfg_attr(feature = "inspect", inspect(iter_by_index))]
     encrypted_metadata_keys: [VmgsEncryptionKey; 2],
+    provisioned_this_boot: bool,
 
     #[cfg_attr(feature = "inspect", inspect(skip))]
     logger: Option<Arc<dyn VmgsLogger>>,
@@ -206,7 +207,9 @@ impl Vmgs {
 
         let active_header = Self::format(&mut storage, VMGS_VERSION_3_0).await?;
 
-        Self::finish_open(storage, active_header, 0, logger).await
+        let mut vmgs = Self::finish_open(storage, active_header, 0, logger).await?;
+        vmgs.provisioned_this_boot = true;
+        Ok(vmgs)
     }
 
     /// Open the VMGS file.
@@ -315,6 +318,7 @@ impl Vmgs {
             datastore_keys: [VmgsDatastoreKey::new_zeroed(); 2],
             metadata_key: VmgsDatastoreKey::new_zeroed(),
             encrypted_metadata_keys,
+            provisioned_this_boot: false,
 
             #[cfg(feature = "inspect")]
             stats: Default::default(),
@@ -1468,6 +1472,11 @@ impl Vmgs {
         self.active_datastore_key_index
     }
 
+    /// Whether the VMGS file was provisioned during the most recent boot
+    pub fn was_provisioned_this_boot(&self) -> bool {
+        self.provisioned_this_boot
+    }
+
     fn prepare_new_header(&self, file_table_fcb: &ResolvedFileControlBlock) -> VmgsHeader {
         VmgsHeader {
             signature: VMGS_SIGNATURE,
@@ -1898,6 +1907,7 @@ pub mod save_restore {
                         encryption_key,
                     }
                 }),
+                provisioned_this_boot: false,
                 logger,
             }
         }
@@ -1925,6 +1935,7 @@ pub mod save_restore {
                 metadata_key,
                 encrypted_metadata_keys,
                 logger: _,
+                provisioned_this_boot: _,
             } = self;
 
             state::SavedVmgsState {