mirror of
https://github.com/openai/codex.git
synced 2026-10-21 14:23:14 +00:00
Repair expired Windows sandbox account passwords during setup (#46043)
## Why An ACL refresh cannot rotate expired sandbox account passwords or update stored credentials. Accounts marked with `UF_PASSWORD_EXPIRED` need full setup before logon. ## What changed - Trigger full setup when either sandbox account has an expired password, then reload credentials and report an error if either password remains expired. - Treat expired passwords as incomplete setup in service provisioning and reject them during registration-only refreshes. ## Testing Add coverage for expiration on either account, a single repair attempt, credential reload, and unsuccessful repair. Add an ignored Windows test requiring administrator rights to verify that resetting an expired password preserves other account flags. GitOrigin-RevId: cd6e86f226bea52243d37bd7c7becf7378ba8721
This commit is contained in:
@@ -31,6 +31,7 @@ use std::os::windows::io::OwnedHandle;
|
||||
use std::path::Path;
|
||||
use std::path::PathBuf;
|
||||
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;
|
||||
use windows_sys::Win32::Security::LOGON32_PROVIDER_DEFAULT;
|
||||
use windows_sys::Win32::Security::LogonUserW;
|
||||
@@ -339,15 +340,17 @@ fn require_sandbox_account_with_setup(
|
||||
|
||||
if identity.is_some() {
|
||||
// Cleanup may also have removed the group, so repair missing or disabled accounts before ACL
|
||||
// refresh can fail, not only after a later logon reports ERROR_ACCOUNT_DISABLED.
|
||||
// refresh can fail. Expired passwords also require full setup, since an ACL refresh
|
||||
// cannot rotate the account passwords and update the stored DPAPI credentials.
|
||||
for username in [OFFLINE_USERNAME, ONLINE_USERNAME] {
|
||||
let needs_repair = match read_local_user_flags(username) {
|
||||
Ok(Some(flags)) => flags & UF_ACCOUNTDISABLE != 0,
|
||||
Ok(Some(flags)) => flags & (UF_ACCOUNTDISABLE | UF_PASSWORD_EXPIRED) != 0,
|
||||
Ok(None) => true,
|
||||
Err(_) => false,
|
||||
};
|
||||
if needs_repair {
|
||||
setup_reason = Some("sandbox account is missing or disabled".to_string());
|
||||
setup_reason =
|
||||
Some("sandbox account is missing, disabled, or password expired".to_string());
|
||||
identity = None;
|
||||
break;
|
||||
}
|
||||
@@ -373,6 +376,14 @@ fn require_sandbox_account_with_setup(
|
||||
},
|
||||
&desired_offline_proxy_settings,
|
||||
)?;
|
||||
for username in [OFFLINE_USERNAME, ONLINE_USERNAME] {
|
||||
if let Ok(Some(flags)) = read_local_user_flags(username) {
|
||||
anyhow::ensure!(
|
||||
flags & UF_PASSWORD_EXPIRED == 0,
|
||||
"Windows sandbox account password is still expired after setup"
|
||||
);
|
||||
}
|
||||
}
|
||||
identity = select_identity(network_identity, codex_home)?;
|
||||
}
|
||||
let identity = identity.ok_or_else(|| {
|
||||
|
||||
@@ -6,6 +6,8 @@
|
||||
use super::require_sandbox_account_with_setup;
|
||||
use crate::WindowsSandboxProxySettingsMode;
|
||||
use crate::resolved_permissions::ResolvedWindowsSandboxPermissions;
|
||||
use crate::setup::OFFLINE_USERNAME;
|
||||
use crate::setup::ONLINE_USERNAME;
|
||||
use crate::setup::SETUP_VERSION;
|
||||
use crate::setup::SandboxSetupRequest;
|
||||
use crate::setup::SandboxUserRecord;
|
||||
@@ -22,6 +24,99 @@ use std::cell::Cell;
|
||||
use std::collections::HashMap;
|
||||
use std::fs;
|
||||
use windows_sys::Win32::NetworkManagement::NetManagement::UF_NORMAL_ACCOUNT;
|
||||
use windows_sys::Win32::NetworkManagement::NetManagement::UF_PASSWORD_EXPIRED;
|
||||
|
||||
#[test]
|
||||
fn credential_setup_repairs_expired_accounts_once_and_reloads_credentials() -> Result<()> {
|
||||
let permissions = ResolvedWindowsSandboxPermissions::try_from_permission_profile(
|
||||
&PermissionProfile::read_only(),
|
||||
)?;
|
||||
for (expired_username, repair_succeeds) in [
|
||||
(None, true),
|
||||
(Some(OFFLINE_USERNAME), true),
|
||||
(Some(ONLINE_USERNAME), true),
|
||||
(Some(OFFLINE_USERNAME), false),
|
||||
(Some(ONLINE_USERNAME), false),
|
||||
] {
|
||||
let home = tempfile::tempdir()?;
|
||||
let marker = SetupMarker {
|
||||
version: SETUP_VERSION,
|
||||
offline_username: OFFLINE_USERNAME.into(),
|
||||
online_username: ONLINE_USERNAME.into(),
|
||||
created_at: None,
|
||||
proxy_ports: vec![],
|
||||
allow_local_binding: false,
|
||||
};
|
||||
let mut users = SandboxUsersFile {
|
||||
version: SETUP_VERSION,
|
||||
offline: SandboxUserRecord {
|
||||
username: OFFLINE_USERNAME.into(),
|
||||
password: BASE64.encode(crate::dpapi::protect(b"old-test-password")?),
|
||||
},
|
||||
online: SandboxUserRecord {
|
||||
username: ONLINE_USERNAME.into(),
|
||||
password: BASE64.encode(crate::dpapi::protect(b"old-test-password")?),
|
||||
},
|
||||
};
|
||||
for (path, bytes) in [
|
||||
(setup_marker_path(home.path()), serde_json::to_vec(&marker)?),
|
||||
(sandbox_users_path(home.path()), serde_json::to_vec(&users)?),
|
||||
] {
|
||||
fs::create_dir_all(path.parent().unwrap())?;
|
||||
fs::write(path, bytes)?;
|
||||
}
|
||||
let setups = Cell::new(/*value*/ 0);
|
||||
let env = HashMap::new();
|
||||
let result = require_sandbox_account_with_setup(
|
||||
&SandboxSetupRequest {
|
||||
permissions: &permissions,
|
||||
command_cwd: home.path(),
|
||||
env_map: &env,
|
||||
codex_home: home.path(),
|
||||
proxy_enforced: false,
|
||||
},
|
||||
WindowsSandboxProxySettingsMode::Preserve,
|
||||
|_, _| {
|
||||
setups.set(setups.get() + 1);
|
||||
users.offline.password =
|
||||
BASE64.encode(crate::dpapi::protect(b"new-test-password")?);
|
||||
users.online.password = users.offline.password.clone();
|
||||
fs::write(sandbox_users_path(home.path()), serde_json::to_vec(&users)?)?;
|
||||
Ok(())
|
||||
},
|
||||
|username| {
|
||||
Ok(Some(
|
||||
if expired_username == Some(username) && (setups.get() == 0 || !repair_succeeds)
|
||||
{
|
||||
UF_NORMAL_ACCOUNT | UF_PASSWORD_EXPIRED
|
||||
} else {
|
||||
UF_NORMAL_ACCOUNT
|
||||
},
|
||||
))
|
||||
},
|
||||
);
|
||||
assert_eq!(setups.get(), usize::from(expired_username.is_some()));
|
||||
assert_eq!(
|
||||
result
|
||||
.map(|(creds, _)| (creds.username, creds.password))
|
||||
.map_err(|err| err.to_string()),
|
||||
if !repair_succeeds {
|
||||
Err("Windows sandbox account password is still expired after setup".into())
|
||||
} else {
|
||||
Ok((
|
||||
OFFLINE_USERNAME.into(),
|
||||
if expired_username.is_some() {
|
||||
"new-test-password"
|
||||
} else {
|
||||
"old-test-password"
|
||||
}
|
||||
.into(),
|
||||
))
|
||||
}
|
||||
);
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn credential_setup_reconciles_effective_firewall_policy() -> Result<()> {
|
||||
|
||||
@@ -53,6 +53,10 @@ use crate::write_file_atomically;
|
||||
|
||||
use super::SetupMode;
|
||||
|
||||
#[cfg(test)]
|
||||
#[path = "sandbox_users_tests.rs"]
|
||||
mod tests;
|
||||
|
||||
const SID_USERS: &str = "S-1-5-32-545";
|
||||
|
||||
pub fn resolve_sandbox_users_group_sid() -> Result<Vec<u8>> {
|
||||
@@ -131,7 +135,8 @@ pub fn ensure_local_user(
|
||||
std::ptr::null_mut(),
|
||||
);
|
||||
if status != NERR_Success {
|
||||
// Try update password via level 1003.
|
||||
// Reset only the password, preserving account flags owned by enterprise policy.
|
||||
// A separate flag update could fail after the reset but before credentials are saved.
|
||||
let pw_info = USER_INFO_1003 {
|
||||
usri1003_password: pwd_w.as_ptr() as *mut u16,
|
||||
};
|
||||
|
||||
@@ -0,0 +1,82 @@
|
||||
//! Native account repair regression; uses a disposable account and requires elevation.
|
||||
|
||||
use super::ensure_local_user;
|
||||
use crate::local_user_flags;
|
||||
use crate::set_local_user_flags;
|
||||
use crate::to_wide;
|
||||
use anyhow::Result;
|
||||
use pretty_assertions::assert_eq;
|
||||
use windows_sys::Win32::NetworkManagement::NetManagement::NetApiBufferFree;
|
||||
use windows_sys::Win32::NetworkManagement::NetManagement::NetUserDel;
|
||||
use windows_sys::Win32::NetworkManagement::NetManagement::NetUserGetInfo;
|
||||
use windows_sys::Win32::NetworkManagement::NetManagement::NetUserSetInfo;
|
||||
use windows_sys::Win32::NetworkManagement::NetManagement::UF_ACCOUNTDISABLE;
|
||||
use windows_sys::Win32::NetworkManagement::NetManagement::UF_DONT_EXPIRE_PASSWD;
|
||||
use windows_sys::Win32::NetworkManagement::NetManagement::UF_PASSWORD_EXPIRED;
|
||||
use windows_sys::Win32::NetworkManagement::NetManagement::USER_INFO_4;
|
||||
|
||||
struct TestAccount(String);
|
||||
|
||||
impl Drop for TestAccount {
|
||||
fn drop(&mut self) {
|
||||
unsafe { NetUserDel(std::ptr::null(), to_wide(&self.0).as_ptr()) };
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
#[ignore = "requires administrator rights to create and remove a disposable local account"]
|
||||
fn repairs_existing_account_password_without_changing_account_policy() -> Result<()> {
|
||||
let name = format!("CdxExpiry{}", std::process::id());
|
||||
anyhow::ensure!(
|
||||
local_user_flags(&name)?.is_none(),
|
||||
"test account already exists"
|
||||
);
|
||||
// Setup can create the account before returning an error.
|
||||
let account = TestAccount(name.clone());
|
||||
let mut log = Vec::new();
|
||||
ensure_local_user(&name, "Old-Test-Password-93!", UF_ACCOUNTDISABLE, &mut log)?;
|
||||
let initial_flags = local_user_flags(&account.0)?.unwrap();
|
||||
set_local_user_flags(&account.0, initial_flags & !UF_DONT_EXPIRE_PASSWD)?;
|
||||
// A level-1008 flags write does not set the dedicated must-change field.
|
||||
// https://learn.microsoft.com/windows/win32/api/lmaccess/ns-lmaccess-user_info_4
|
||||
let mut buffer = std::ptr::null_mut();
|
||||
let status = unsafe {
|
||||
NetUserGetInfo(
|
||||
std::ptr::null(),
|
||||
to_wide(&account.0).as_ptr(),
|
||||
/*level*/ 4,
|
||||
&mut buffer,
|
||||
)
|
||||
};
|
||||
anyhow::ensure!(status == 0, "read USER_INFO_4 failed: {status}");
|
||||
let status = unsafe {
|
||||
(*buffer.cast::<USER_INFO_4>()).usri4_password_expired = 1;
|
||||
let status = NetUserSetInfo(
|
||||
std::ptr::null(),
|
||||
to_wide(&account.0).as_ptr(),
|
||||
/*level*/ 4,
|
||||
buffer,
|
||||
std::ptr::null_mut(),
|
||||
);
|
||||
NetApiBufferFree(buffer.cast());
|
||||
status
|
||||
};
|
||||
anyhow::ensure!(status == 0, "expire test account password failed: {status}");
|
||||
let expired_flags = local_user_flags(&account.0)?.unwrap();
|
||||
assert_eq!(expired_flags & UF_PASSWORD_EXPIRED, UF_PASSWORD_EXPIRED);
|
||||
|
||||
ensure_local_user(
|
||||
&account.0,
|
||||
"New-Test-Password-94!",
|
||||
/*new_user_flags*/ 0,
|
||||
&mut log,
|
||||
)?;
|
||||
|
||||
assert_eq!(
|
||||
local_user_flags(&account.0)?.unwrap(),
|
||||
expired_flags & !UF_PASSWORD_EXPIRED
|
||||
);
|
||||
drop(account);
|
||||
assert_eq!(local_user_flags(&name)?, None);
|
||||
Ok(())
|
||||
}
|
||||
@@ -11,6 +11,7 @@ use crate::ipc::ProvisioningRequest;
|
||||
use codex_windows_sandbox::string_from_sid_bytes;
|
||||
use windows::ApplicationModel::Package;
|
||||
use windows_sys::Win32::NetworkManagement::NetManagement::UF_ACCOUNTDISABLE;
|
||||
use windows_sys::Win32::NetworkManagement::NetManagement::UF_PASSWORD_EXPIRED;
|
||||
|
||||
use crate::installation_record::InstallationRecord;
|
||||
use crate::installation_record::RuntimeRegistration;
|
||||
@@ -57,11 +58,11 @@ pub(super) fn run(
|
||||
let account = entry.account.username();
|
||||
ensure!(
|
||||
codex_windows_sandbox::local_user_flags(account)?
|
||||
.is_some_and(|flags| flags & UF_ACCOUNTDISABLE == 0)
|
||||
.is_some_and(|flags| flags & (UF_ACCOUNTDISABLE | UF_PASSWORD_EXPIRED) == 0)
|
||||
&& string_from_sid_bytes(&codex_windows_sandbox::resolve_sid(account)?)
|
||||
.map_err(anyhow::Error::msg)?
|
||||
== entry.user_sid,
|
||||
"registration refresh cannot replace a sandbox account"
|
||||
"registration refresh cannot repair or replace a sandbox account"
|
||||
);
|
||||
}
|
||||
}
|
||||
@@ -77,12 +78,13 @@ pub(super) fn run(
|
||||
});
|
||||
// Persist the authenticated owner before setup can rotate shared credentials.
|
||||
crate::installation_record::save_runtime(&installation)?;
|
||||
// Expired passwords require full setup to rotate and persist credentials before logon.
|
||||
for account in [
|
||||
codex_windows_sandbox::OFFLINE_USERNAME,
|
||||
codex_windows_sandbox::ONLINE_USERNAME,
|
||||
] {
|
||||
setup_complete &= codex_windows_sandbox::local_user_flags(account)?
|
||||
.is_some_and(|flags| flags & UF_ACCOUNTDISABLE == 0);
|
||||
.is_some_and(|flags| flags & (UF_ACCOUNTDISABLE | UF_PASSWORD_EXPIRED) == 0);
|
||||
}
|
||||
if !setup_complete {
|
||||
// Account state can change outside our setup lock; refresh must never repair it.
|
||||
|
||||
Reference in New Issue
Block a user