From 954dc9cf974bb34e759e40dddea5260344ea0409 Mon Sep 17 00:00:00 2001 From: Soham Das Date: Tue, 24 Mar 2026 11:20:09 -0700 Subject: [PATCH] Fix crash when user cancels hello dialog --- .../libs/sdk-host/src/userboundkey/mod.rs | 36 +++++++++++++++++-- .../src/veil_host_lib/userboundkey.vtl0.cpp | 28 +++++++++++++-- 2 files changed, 59 insertions(+), 5 deletions(-) diff --git a/rust/sdk/crates/libs/sdk-host/src/userboundkey/mod.rs b/rust/sdk/crates/libs/sdk-host/src/userboundkey/mod.rs index 01a63fcf..55b946b3 100644 --- a/rust/sdk/crates/libs/sdk-host/src/userboundkey/mod.rs +++ b/rust/sdk/crates/libs/sdk-host/src/userboundkey/mod.rs @@ -29,7 +29,11 @@ use windows::{ Security::Cryptography::CryptographicBuffer, Storage::Streams::IBuffer, Win32::{ - Foundation::{CloseHandle, E_FAIL, E_INVALIDARG, HANDLE, HLOCAL, LocalFree}, + Foundation::{ + CloseHandle, E_FAIL, E_INVALIDARG, E_UNEXPECTED, ERROR_ACCOUNT_LOCKED_OUT, + ERROR_ALREADY_EXISTS, ERROR_BAD_CONFIGURATION, ERROR_CANCELLED, ERROR_NOT_FOUND, + HANDLE, HLOCAL, LocalFree, + }, Security::Authorization::ConvertSidToStringSidW, Security::{GetTokenInformation, PSID, TOKEN_QUERY, TOKEN_USER, TokenUser}, System::Threading::{GetCurrentProcess, OpenProcessToken}, @@ -119,6 +123,32 @@ fn vec_to_kcm_buffer(data: &[u8]) -> windows_core::Result { windows_buffer.cast() } +/// Convert KeyCredentialStatus to an AbiError with a proper failure HRESULT. +/// +/// KeyCredentialStatus enum values (0..6) are not valid failure HRESULTs — they have +/// the high bit clear, so Windows considers them success codes. Passing them raw through +/// `AbiError::Hresult(status.0)` causes downstream `hr < 0` / `FAILED()` checks to +/// misinterpret the error as success. +/// +/// Uses `AbiError::Win32Error` which internally applies `HRESULT::from_win32()` to +/// produce proper failure HRESULTs. +fn key_credential_status_to_abi_error(status: KeyCredentialStatus) -> AbiError { + match status { + KeyCredentialStatus::Success => AbiError::Hresult(0), // S_OK + KeyCredentialStatus::UnknownError => AbiError::Hresult(E_FAIL.0), + KeyCredentialStatus::NotFound => AbiError::Win32Error(ERROR_NOT_FOUND.0), + KeyCredentialStatus::UserCanceled => AbiError::Win32Error(ERROR_CANCELLED.0), + KeyCredentialStatus::UserPrefersPassword => AbiError::Win32Error(ERROR_BAD_CONFIGURATION.0), + KeyCredentialStatus::CredentialAlreadyExists => { + AbiError::Win32Error(ERROR_ALREADY_EXISTS.0) + } + KeyCredentialStatus::SecurityDeviceLocked => { + AbiError::Win32Error(ERROR_ACCOUNT_LOCKED_OUT.0) + } + _ => AbiError::Hresult(E_UNEXPECTED.0), + } +} + /// Get the algorithm name string from the ECDH algorithm handle fn get_algorithm(ecdh_algorithm: u64) -> Result { // BCrypt algorithm pseudo-handle values from Windows SDK bcrypt.h @@ -308,7 +338,7 @@ pub fn userboundkey_establish_session_for_create( .Status() .map_err(|e| AbiError::Hresult(e.code().0))?; if status != KeyCredentialStatus::Success { - return Err(AbiError::Hresult(status.0)); + return Err(key_credential_status_to_abi_error(status)); } // Get credential and convert to raw pointer @@ -369,7 +399,7 @@ pub fn userboundkey_establish_session_for_load( .Status() .map_err(|e| AbiError::Hresult(e.code().0))?; if status != KeyCredentialStatus::Success { - return Err(AbiError::Hresult(status.0)); + return Err(key_credential_status_to_abi_error(status)); } // Get credential and convert to raw pointer diff --git a/src/VbsEnclaveSDK/src/veil_host_lib/userboundkey.vtl0.cpp b/src/VbsEnclaveSDK/src/veil_host_lib/userboundkey.vtl0.cpp index cecfda7e..e9690553 100644 --- a/src/VbsEnclaveSDK/src/veil_host_lib/userboundkey.vtl0.cpp +++ b/src/VbsEnclaveSDK/src/veil_host_lib/userboundkey.vtl0.cpp @@ -217,6 +217,30 @@ winrt::hstring GetAlgorithm(uintptr_t ecdhAlgorithm) THROW_HR(E_INVALIDARG); } +// Helper function to convert KeyCredentialStatus to appropriate HRESULT +HRESULT KeyCredentialStatusToHResult(KeyCredentialStatus status) +{ + switch (status) + { + case KeyCredentialStatus::Success: + return S_OK; + case KeyCredentialStatus::UnknownError: + return E_FAIL; + case KeyCredentialStatus::NotFound: + return HRESULT_FROM_WIN32(ERROR_NOT_FOUND); + case KeyCredentialStatus::UserCanceled: + return HRESULT_FROM_WIN32(ERROR_CANCELLED); + case KeyCredentialStatus::UserPrefersPassword: + return HRESULT_FROM_WIN32(ERROR_BAD_CONFIGURATION); + case KeyCredentialStatus::CredentialAlreadyExists: + return HRESULT_FROM_WIN32(ERROR_ALREADY_EXISTS); + case KeyCredentialStatus::SecurityDeviceLocked: + return HRESULT_FROM_WIN32(ERROR_ACCOUNT_LOCKED_OUT); + default: + return E_UNEXPECTED; + } +} + // Helper function to convert veil_abi::Types::keyCredentialCacheConfig to KeyCredentialCacheConfiguration KeyCredentialCacheConfiguration ConvertCacheConfig(const veil_abi::Types::keyCredentialCacheConfig& cacheConfig) { @@ -292,7 +316,7 @@ veil_abi::Types::credentialAndSessionInfo veil_abi::Untrusted::Implementation::u auto status = credentialResult.Status(); if (status != KeyCredentialStatus::Success) { - THROW_HR(static_cast(status)); + THROW_HR(KeyCredentialStatusToHResult(status)); } veil::vtl0::implementation::debug::internal::debug_wprint(L"DEBUG: Transferring credential and session ownership to VTL1"); @@ -326,7 +350,7 @@ veil_abi::Types::credentialAndSessionInfo veil_abi::Untrusted::Implementation::u auto status = credentialResult.Status(); if (status != KeyCredentialStatus::Success) { - THROW_HR(static_cast(status)); + THROW_HR(KeyCredentialStatusToHResult(status)); } veil::vtl0::implementation::debug::internal::debug_wprint(L"DEBUG: Transferring credential and session ownership to VTL1");