mirror of
https://github.com/openai/codex.git
synced 2026-10-20 00:18:49 +00:00
Repair rejected Windows sandbox credentials during provisioning (#47695)
## Why Registered Windows sandbox setup could appear complete even when Windows rejected the stored account passwords. Checking account flags alone did not detect these failures before runtime logon. ## What changed - Check stored credentials for both offline and online accounts before reusing completed setup during non-refresh provisioning. - Classify logon failure, expired-password, and password-change-required errors as `SandboxAccountCredentialMismatch`, preserving the underlying Windows error. - Trigger full setup to rotate credentials only after verifying that recorded account SIDs still match. Propagate credential-file and policy errors without triggering password repair, and skip credential checks for refresh-only requests. ## Testing Add tests for native error classification, missing credential files, healthy credentials, password failures on either account, policy failures, and rejection of repair when account ownership validation fails. GitOrigin-RevId: b3aa02f82841dd2605b0c3585ec93144727721e6
This commit is contained in:
@@ -29,10 +29,14 @@ use base64::Engine;
|
||||
use base64::engine::general_purpose::STANDARD as BASE64_STANDARD;
|
||||
use std::collections::HashMap;
|
||||
use std::fs;
|
||||
use std::io;
|
||||
use std::os::windows::io::FromRawHandle;
|
||||
use std::os::windows::io::OwnedHandle;
|
||||
use std::path::Path;
|
||||
use std::path::PathBuf;
|
||||
use windows_sys::Win32::Foundation::ERROR_LOGON_FAILURE;
|
||||
use windows_sys::Win32::Foundation::ERROR_PASSWORD_EXPIRED;
|
||||
use windows_sys::Win32::Foundation::ERROR_PASSWORD_MUST_CHANGE;
|
||||
use windows_sys::Win32::NetworkManagement::NetManagement::UF_ACCOUNTDISABLE;
|
||||
use windows_sys::Win32::NetworkManagement::NetManagement::UF_PASSWORD_EXPIRED;
|
||||
use windows_sys::Win32::Security::LOGON32_LOGON_INTERACTIVE;
|
||||
@@ -55,6 +59,18 @@ pub struct SandboxCreds {
|
||||
pub password: String,
|
||||
}
|
||||
|
||||
/// Windows rejected the stored sandbox password; credential-file and policy errors are distinct.
|
||||
#[derive(Debug)]
|
||||
pub struct SandboxAccountCredentialMismatch;
|
||||
|
||||
impl std::fmt::Display for SandboxAccountCredentialMismatch {
|
||||
fn fmt(&self, formatter: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
|
||||
formatter.write_str("Windows rejected the stored sandbox account password")
|
||||
}
|
||||
}
|
||||
|
||||
impl std::error::Error for SandboxAccountCredentialMismatch {}
|
||||
|
||||
/// Returns true when the on-disk setup artifacts exist and match the current
|
||||
/// setup version.
|
||||
///
|
||||
@@ -206,11 +222,24 @@ pub fn logon_existing_sandbox_account(
|
||||
)
|
||||
} == 0
|
||||
{
|
||||
return Err(std::io::Error::last_os_error()).context("log on existing sandbox account");
|
||||
return Err(account_logon_error(io::Error::last_os_error()));
|
||||
}
|
||||
Ok(unsafe { OwnedHandle::from_raw_handle(token as _) })
|
||||
}
|
||||
|
||||
fn account_logon_error(error: io::Error) -> anyhow::Error {
|
||||
let bad_password = matches!(
|
||||
error.raw_os_error().map(|code| code as u32),
|
||||
Some(ERROR_LOGON_FAILURE | ERROR_PASSWORD_EXPIRED | ERROR_PASSWORD_MUST_CHANGE)
|
||||
);
|
||||
let error = anyhow::Error::new(error);
|
||||
if bad_password {
|
||||
error.context(SandboxAccountCredentialMismatch)
|
||||
} else {
|
||||
error.context("log on existing sandbox account")
|
||||
}
|
||||
}
|
||||
|
||||
fn select_identity(
|
||||
network_identity: SandboxNetworkIdentity,
|
||||
codex_home: &Path,
|
||||
|
||||
@@ -3,8 +3,12 @@
|
||||
//! the public wrapper, native account queries, payload serialization, singleflight,
|
||||
//! or helper launches.
|
||||
|
||||
use super::SandboxAccountCredentialMismatch;
|
||||
use super::account_logon_error;
|
||||
use super::logon_existing_sandbox_account;
|
||||
use super::require_sandbox_account_with_setup;
|
||||
use super::sandbox_setup_is_complete_with_settings;
|
||||
use crate::SandboxRuntimeAccount;
|
||||
use crate::WindowsSandboxProvisioningSettings;
|
||||
use crate::WindowsSandboxProxySettingsMode;
|
||||
use crate::resolved_permissions::ResolvedWindowsSandboxPermissions;
|
||||
@@ -25,10 +29,41 @@ use pretty_assertions::assert_eq;
|
||||
use std::cell::Cell;
|
||||
use std::collections::HashMap;
|
||||
use std::fs;
|
||||
use std::io;
|
||||
use windows_sys::Win32::Foundation as win32;
|
||||
use windows_sys::Win32::NetworkManagement::NetManagement::UF_ACCOUNTDISABLE;
|
||||
use windows_sys::Win32::NetworkManagement::NetManagement::UF_NORMAL_ACCOUNT;
|
||||
use windows_sys::Win32::NetworkManagement::NetManagement::UF_PASSWORD_EXPIRED;
|
||||
|
||||
#[test]
|
||||
fn only_native_bad_or_expired_passwords_trigger_credential_repair() -> Result<()> {
|
||||
for code in [
|
||||
win32::ERROR_LOGON_FAILURE,
|
||||
win32::ERROR_PASSWORD_EXPIRED,
|
||||
win32::ERROR_PASSWORD_MUST_CHANGE,
|
||||
] {
|
||||
let error = account_logon_error(io::Error::from_raw_os_error(code as i32));
|
||||
assert!(error.is::<SandboxAccountCredentialMismatch>());
|
||||
assert_eq!(
|
||||
error.downcast_ref::<io::Error>().unwrap().raw_os_error(),
|
||||
Some(code as i32)
|
||||
);
|
||||
}
|
||||
for code in [
|
||||
win32::ERROR_ACCESS_DENIED,
|
||||
win32::ERROR_ACCOUNT_RESTRICTION,
|
||||
win32::ERROR_LOGON_TYPE_NOT_GRANTED,
|
||||
] {
|
||||
let error = account_logon_error(io::Error::from_raw_os_error(code as i32));
|
||||
assert!(!error.is::<SandboxAccountCredentialMismatch>());
|
||||
}
|
||||
let home = tempfile::tempdir()?;
|
||||
let error =
|
||||
logon_existing_sandbox_account(home.path(), SandboxRuntimeAccount::Offline).unwrap_err();
|
||||
assert!(!error.is::<SandboxAccountCredentialMismatch>());
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn credential_setup_repairs_expired_accounts_once_and_reloads_credentials() -> Result<()> {
|
||||
let permissions = ResolvedWindowsSandboxPermissions::try_from_permission_profile(
|
||||
|
||||
@@ -279,6 +279,8 @@ pub use hide_users::hide_current_user_profile_dir;
|
||||
#[cfg(target_os = "windows")]
|
||||
pub use hide_users::hide_newly_created_users;
|
||||
#[cfg(target_os = "windows")]
|
||||
pub use identity::SandboxAccountCredentialMismatch;
|
||||
#[cfg(target_os = "windows")]
|
||||
pub use identity::logon_existing_sandbox_account;
|
||||
#[cfg(target_os = "windows")]
|
||||
pub use identity::require_logon_sandbox_creds;
|
||||
|
||||
@@ -5,7 +5,11 @@ use std::sync::atomic::AtomicBool;
|
||||
use anyhow::Context;
|
||||
use anyhow::Result;
|
||||
use anyhow::ensure;
|
||||
use codex_windows_sandbox::SandboxAccountCredentialMismatch;
|
||||
use codex_windows_sandbox::SandboxProvisioningResponse;
|
||||
use codex_windows_sandbox::SandboxRuntimeAccount;
|
||||
use codex_windows_sandbox::logon_existing_sandbox_account;
|
||||
use codex_windows_sandbox::sandbox_secrets_dir;
|
||||
|
||||
use crate::ipc::ProvisioningRequest;
|
||||
use codex_windows_sandbox::string_from_sid_bytes;
|
||||
@@ -18,6 +22,10 @@ use crate::installation_record::RuntimeRegistration;
|
||||
use crate::ipc::ClientIdentity;
|
||||
use crate::ipc::OwnedHandle;
|
||||
|
||||
#[cfg(test)]
|
||||
#[path = "registered_tests.rs"]
|
||||
mod tests;
|
||||
|
||||
pub(super) fn run(
|
||||
identity: &ClientIdentity,
|
||||
request: ProvisioningRequest,
|
||||
@@ -79,7 +87,7 @@ pub(super) fn run(
|
||||
// Persist the authenticated owner before setup can rotate shared credentials.
|
||||
crate::installation_record::save_runtime(&installation)
|
||||
.context("persist registered sandbox owner")?;
|
||||
// Expired passwords require full setup to rotate and persist credentials before logon.
|
||||
// Missing, disabled, or expired accounts require full setup before runtime logon.
|
||||
for account in [
|
||||
codex_windows_sandbox::OFFLINE_USERNAME,
|
||||
codex_windows_sandbox::ONLINE_USERNAME,
|
||||
@@ -88,6 +96,32 @@ pub(super) fn run(
|
||||
.with_context(|| format!("inspect registered sandbox account {account}"))?
|
||||
.is_some_and(|flags| flags & (UF_ACCOUNTDISABLE | UF_PASSWORD_EXPIRED) == 0);
|
||||
}
|
||||
if setup_complete && !request.refresh_only {
|
||||
setup_complete = !credentials_need_repair(
|
||||
|account| {
|
||||
crate::package_lifecycle::with_owner_impersonation(identity.token.0, || {
|
||||
let mut pins = Vec::new();
|
||||
crate::ipc::pin_existing_ancestors(
|
||||
&sandbox_secrets_dir(&identity.codex_home),
|
||||
&mut pins,
|
||||
)?;
|
||||
logon_existing_sandbox_account(&identity.codex_home, account).map(drop)
|
||||
})
|
||||
.with_context(|| format!("check registered sandbox account {account:?}"))
|
||||
},
|
||||
|| {
|
||||
for entry in &installation.runtime()?.accounts {
|
||||
let sid = codex_windows_sandbox::resolve_sid(entry.account.username())?;
|
||||
ensure!(
|
||||
string_from_sid_bytes(&sid).map_err(anyhow::Error::msg)?
|
||||
== entry.user_sid,
|
||||
"managed runtime account was replaced"
|
||||
);
|
||||
}
|
||||
Ok(())
|
||||
},
|
||||
)?;
|
||||
}
|
||||
if !setup_complete {
|
||||
// Account state can change outside our setup lock; refresh must never repair it.
|
||||
ensure!(
|
||||
@@ -134,3 +168,25 @@ pub(super) fn run(
|
||||
);
|
||||
Ok(SandboxProvisioningResponse::Ok)
|
||||
}
|
||||
|
||||
fn credentials_need_repair(
|
||||
mut logon: impl FnMut(SandboxRuntimeAccount) -> Result<()>,
|
||||
validate_ownership: impl FnOnce() -> Result<()>,
|
||||
) -> Result<bool> {
|
||||
let mut needs_repair = false;
|
||||
for account in [
|
||||
SandboxRuntimeAccount::Offline,
|
||||
SandboxRuntimeAccount::Online,
|
||||
] {
|
||||
match logon(account) {
|
||||
Ok(()) => {}
|
||||
Err(error) if error.is::<SandboxAccountCredentialMismatch>() => needs_repair = true,
|
||||
Err(error) => return Err(error),
|
||||
}
|
||||
}
|
||||
if needs_repair {
|
||||
// Full setup rotates both passwords; never rotate a recreated local account.
|
||||
validate_ownership()?;
|
||||
}
|
||||
Ok(needs_repair)
|
||||
}
|
||||
|
||||
@@ -0,0 +1,46 @@
|
||||
//! Checks which credential failures may trigger a full setup repair.
|
||||
|
||||
use super::credentials_need_repair;
|
||||
use anyhow::anyhow;
|
||||
use codex_windows_sandbox::SandboxAccountCredentialMismatch;
|
||||
use pretty_assertions::assert_eq;
|
||||
|
||||
#[test]
|
||||
fn only_password_failures_on_owned_accounts_allow_repair() {
|
||||
enum Logon {
|
||||
Healthy,
|
||||
Password,
|
||||
Policy,
|
||||
}
|
||||
use Logon::Healthy;
|
||||
use Logon::Password;
|
||||
use Logon::Policy;
|
||||
for (errors, ownership_ok, expected, ownership_checked) in [
|
||||
([Healthy, Healthy], true, Ok(false), false),
|
||||
([Password, Healthy], true, Ok(true), true),
|
||||
([Healthy, Password], true, Ok(true), true),
|
||||
([Password, Policy], true, Err("policy"), false),
|
||||
([Policy, Password], true, Err("policy"), false),
|
||||
([Password, Healthy], false, Err("replacement"), true),
|
||||
] {
|
||||
let mut errors = errors.into_iter();
|
||||
let mut checked = false;
|
||||
let result = credentials_need_repair(
|
||||
|_| match errors.next().unwrap() {
|
||||
Healthy => Ok(()),
|
||||
Password => Err(anyhow!(SandboxAccountCredentialMismatch)),
|
||||
Policy => Err(anyhow!("policy")),
|
||||
},
|
||||
|| {
|
||||
checked = true;
|
||||
anyhow::ensure!(ownership_ok, "replacement");
|
||||
Ok(())
|
||||
},
|
||||
)
|
||||
.map_err(|error| error.to_string());
|
||||
assert_eq!(
|
||||
(result, checked),
|
||||
(expected.map_err(str::to_owned), ownership_checked)
|
||||
);
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user