mirror of
https://github.com/openai/codex.git
synced 2026-10-19 16:17:49 +00:00
Fix cancellation of macOS user-verification prompts (#53790)
## Why Keychain signing reconstructs its own authentication context, so invalidating the caller's context does not reliably cancel the context that owns the biometric prompt. ## What changed Evaluate the stored key's access-control policy explicitly with `LAContext` before signing. On cancellation, invalidate that context and wait for its authorization callback before releasing the lifecycle lock, rejecting any late success. Reload the key with the authorized context and disable further interaction so signing cannot open another prompt. ## Testing Update unit tests to cover waiting for cancellation completion and rejecting completed authorization after cancellation. Add an ignored macOS hardware test and setup instructions covering persisted Secure Enclave key reuse, signature verification, cancellation, retry signing, and cleanup. GitOrigin-RevId: 4ea9756430c5649654e95e047d1666617503072c
This commit is contained in:
Generated
+1
File diff suppressed because one or more lines are too long
Generated
+14
@@ -4935,6 +4935,7 @@ name = "codex-user-verification"
|
||||
version = "0.0.0"
|
||||
dependencies = [
|
||||
"base64 0.22.1",
|
||||
"block2",
|
||||
"core-foundation 0.10.1",
|
||||
"dirs",
|
||||
"objc2",
|
||||
@@ -10961,8 +10962,10 @@ version = "0.3.2"
|
||||
source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "e48e0b8b339e0d9d2ed4416b7f93f9d4daadff7d4dd797f89867cde11aeac607"
|
||||
dependencies = [
|
||||
"block2",
|
||||
"objc2",
|
||||
"objc2-foundation",
|
||||
"objc2-security",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
@@ -10977,6 +10980,17 @@ dependencies = [
|
||||
"objc2-foundation",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
name = "objc2-security"
|
||||
version = "0.3.2"
|
||||
source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "709fe137109bd1e8b5a99390f77a7d8b2961dafc1a1c5db8f2e60329ad6d895a"
|
||||
dependencies = [
|
||||
"bitflags 2.13.1",
|
||||
"objc2",
|
||||
"objc2-core-foundation",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
name = "objc2-ui-kit"
|
||||
version = "0.3.2"
|
||||
|
||||
@@ -15,11 +15,12 @@ thiserror = { workspace = true }
|
||||
tracing = { workspace = true }
|
||||
|
||||
[target.'cfg(target_os = "macos")'.dependencies]
|
||||
block2 = "0.6.2"
|
||||
core-foundation = "0.10"
|
||||
dirs = { workspace = true }
|
||||
objc2 = "0.6.3"
|
||||
objc2-foundation = { version = "0.3.2", default-features = false, features = ["std", "NSString"] }
|
||||
objc2-local-authentication = { version = "0.3.2", default-features = false, features = ["std", "LAContext", "LABiometryType"] }
|
||||
objc2-local-authentication = { version = "0.3.2", default-features = false, features = ["std", "LAContext", "LABiometryType", "block2", "objc2-security"] }
|
||||
security-framework = { version = "3.5", features = ["OSX_10_15"] }
|
||||
security-framework-sys = "2.15"
|
||||
|
||||
|
||||
@@ -0,0 +1,59 @@
|
||||
# Native hardware test
|
||||
|
||||
`tests/hardware.rs` exercises the macOS provider with a real persisted Secure
|
||||
Enclave key: create and reload it, verify a signature, cancel an authentication
|
||||
request, and sign a new challenge with the same key. It is ignored by default.
|
||||
|
||||
Run on a physical Touch ID Mac with fingerprints enrolled and the lid open. Use
|
||||
an existing **isolated development app** with a valid provisioning profile and
|
||||
Keychain Sharing entitlements, plus its Apple Development signing identity.
|
||||
The test does not use a Wallet account or submit a purchase. Run only one copy at
|
||||
a time; it reserves the `codex-user-verification-hardware-test` key namespace and
|
||||
removes that namespace's key before and after the test.
|
||||
|
||||
From the repository root, set `TEMPLATE_APP` to that signed development `.app`
|
||||
and `SIGNING_IDENTITY` to its signing identity, then run in Bash. Signing the
|
||||
runner is insufficient: the test executable itself needs the profile and
|
||||
entitlements.
|
||||
|
||||
```bash
|
||||
set -euo pipefail
|
||||
: "${TEMPLATE_APP:?Set the path to the isolated signed development app}"
|
||||
: "${SIGNING_IDENTITY:?Set its Apple Development identity}"
|
||||
run_dir=$(mktemp -d)
|
||||
app="$run_dir/HardwareTest.app"
|
||||
(cd codex-rs && cargo build --locked -p codex-user-verification --test hardware \
|
||||
--message-format=json) > "$run_dir/build.jsonl"
|
||||
binary=$(jq -r 'select(.reason == "compiler-artifact" and .target.name == "hardware" and .executable != null) | .executable' "$run_dir/build.jsonl")
|
||||
mkdir -p "$app/Contents/MacOS"
|
||||
cp "$binary" "$app/Contents/MacOS/hardware"
|
||||
cp "$TEMPLATE_APP/Contents/Info.plist" "$app/Contents/Info.plist"
|
||||
cp "$TEMPLATE_APP/Contents/embedded.provisionprofile" "$app/Contents/embedded.provisionprofile"
|
||||
/usr/libexec/PlistBuddy -c 'Set :CFBundleExecutable hardware' "$app/Contents/Info.plist"
|
||||
codesign -d --xml --entitlements - "$TEMPLATE_APP" > "$run_dir/entitlements.plist"
|
||||
codesign --force --options runtime --entitlements "$run_dir/entitlements.plist" \
|
||||
--sign "$SIGNING_IDENTITY" "$app"
|
||||
codesign --verify --strict "$app"
|
||||
echo "Test bundle: $app"
|
||||
"$app/Contents/MacOS/hardware" --ignored --exact \
|
||||
persisted_key_signing_cancellation_and_retry --nocapture --test-threads=1
|
||||
```
|
||||
|
||||
Use Touch ID for the first prompt, then remove your finger. Leave the
|
||||
**cancellation** prompt untouched and observe that it appears and closes on its
|
||||
own. Authenticate the **retry signing** prompt once. Passing assertions verify
|
||||
both signatures, the cancellation result, and key removal; confirming visible
|
||||
prompt dismissal is the operator's part of the test. An early interruption fails
|
||||
the test rather than counting as cancellation.
|
||||
|
||||
If cancellation has not returned ten seconds after it is issued, the watchdog
|
||||
exits with a failure instead of hanging. After a forced exit, stop any remaining
|
||||
test process and remove the reserved key with the same signed bundle:
|
||||
|
||||
```bash
|
||||
CODEX_HARDWARE_TEST_CLEANUP=1 "$app/Contents/MacOS/hardware" --ignored --exact \
|
||||
persisted_key_signing_cancellation_and_retry --nocapture --test-threads=1
|
||||
```
|
||||
|
||||
This test covers the native provider. It does not exercise app-server routing,
|
||||
UI state, closed-lid recovery, or hosted Wallet completion.
|
||||
@@ -1,4 +1,4 @@
|
||||
//! Keeps the authentication context on its owner thread while a native operation blocks.
|
||||
//! Drains asynchronous authentication before releasing its context and lifecycle lock.
|
||||
|
||||
use crate::UserVerificationError;
|
||||
use crate::UserVerificationFailureReason;
|
||||
@@ -6,46 +6,35 @@ use crate::UserVerificationRequestGuard;
|
||||
use std::sync::mpsc;
|
||||
use std::time::Duration;
|
||||
|
||||
pub(crate) fn run_with_cancellation<T: Send>(
|
||||
pub(crate) fn wait_with_cancellation<T>(
|
||||
guard: &UserVerificationRequestGuard,
|
||||
operation: impl FnOnce() -> Result<T, UserVerificationError> + Send,
|
||||
completion: mpsc::Receiver<Result<T, UserVerificationError>>,
|
||||
cancel: impl FnOnce(),
|
||||
) -> Result<T, UserVerificationError> {
|
||||
guard.check()?;
|
||||
let (done_tx, done_rx) = mpsc::sync_channel(1);
|
||||
std::thread::scope(|scope| {
|
||||
let worker = std::thread::Builder::new()
|
||||
.name("user-verification-sign".to_string())
|
||||
.spawn_scoped(scope, move || {
|
||||
let _ = done_tx.send(guard.check().and_then(|()| operation()));
|
||||
})
|
||||
.map_err(|error| {
|
||||
tracing::warn!(%error, "could not start user-verification signer");
|
||||
worker_error()
|
||||
})?;
|
||||
let result = loop {
|
||||
match done_rx.recv_timeout(Duration::from_millis(/*millis*/ 50)) {
|
||||
Ok(result) => break Some(result),
|
||||
Err(mpsc::RecvTimeoutError::Timeout) => {
|
||||
if !guard.is_active() {
|
||||
cancel();
|
||||
break None;
|
||||
}
|
||||
let result = loop {
|
||||
match completion.recv_timeout(Duration::from_millis(/*millis*/ 50)) {
|
||||
Ok(result) => break result,
|
||||
Err(mpsc::RecvTimeoutError::Timeout) => {
|
||||
if !guard.is_active() {
|
||||
cancel();
|
||||
// Invalidation is asynchronous. Keep the lifecycle lock until the
|
||||
// authentication callback completes, then reject any late success.
|
||||
break completion
|
||||
.recv()
|
||||
.unwrap_or_else(|_| Err(completion_error()));
|
||||
}
|
||||
Err(mpsc::RecvTimeoutError::Disconnected) => break Some(Err(worker_error())),
|
||||
}
|
||||
};
|
||||
// Join before returning so no signer outlives the context or its lifecycle lock.
|
||||
worker.join().map_err(|_| worker_error())?;
|
||||
guard.check()?;
|
||||
result.unwrap_or_else(|| Err(worker_error()))
|
||||
})
|
||||
Err(mpsc::RecvTimeoutError::Disconnected) => break Err(completion_error()),
|
||||
}
|
||||
};
|
||||
guard.check()?;
|
||||
result
|
||||
}
|
||||
|
||||
fn worker_error() -> UserVerificationError {
|
||||
fn completion_error() -> UserVerificationError {
|
||||
UserVerificationError::Failed {
|
||||
reason: UserVerificationFailureReason::ProviderError,
|
||||
message: "the user-verification signer could not complete".to_string(),
|
||||
message: "the user-verification authorization could not complete".to_string(),
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
//! Exercises prompt teardown and late-result rejection without requiring biometric hardware.
|
||||
//! Exercises cancellation completion and late-result rejection without biometric hardware.
|
||||
|
||||
use super::*;
|
||||
use crate::UserVerificationCancellationReason;
|
||||
@@ -7,34 +7,26 @@ use std::sync::atomic::AtomicBool;
|
||||
use std::sync::atomic::Ordering;
|
||||
|
||||
#[test]
|
||||
fn cancellation_tears_down_the_prompt_on_the_context_owner_thread() {
|
||||
fn cancellation_waits_for_completion_on_the_context_owner_thread() {
|
||||
let guard = UserVerificationRequestGuard::default();
|
||||
let queued = guard.clone();
|
||||
let (started_tx, started_rx) = mpsc::channel();
|
||||
let (dismissed_tx, dismissed_rx) = mpsc::channel();
|
||||
let (exited_tx, exited_rx) = mpsc::channel();
|
||||
let (done_tx, done_rx) = mpsc::sync_channel(0);
|
||||
let (cancelled_tx, cancelled_rx) = mpsc::channel();
|
||||
let owner = std::thread::spawn(move || {
|
||||
let owner_thread = std::thread::current().id();
|
||||
run_with_cancellation(
|
||||
&queued,
|
||||
move || {
|
||||
started_tx.send(()).expect("authentication started");
|
||||
dismissed_rx
|
||||
.recv_timeout(Duration::from_secs(/*secs*/ 5))
|
||||
.expect("prompt dismissed before the deadline");
|
||||
exited_tx.send(()).expect("signer exited");
|
||||
Ok("late signature")
|
||||
},
|
||||
|| {
|
||||
assert_eq!(std::thread::current().id(), owner_thread);
|
||||
dismissed_tx.send(()).expect("dismiss authentication");
|
||||
},
|
||||
)
|
||||
wait_with_cancellation(&queued, done_rx, || {
|
||||
assert_eq!(std::thread::current().id(), owner_thread);
|
||||
cancelled_tx.send(()).expect("dismiss authentication");
|
||||
})
|
||||
});
|
||||
started_rx
|
||||
.recv_timeout(Duration::from_secs(/*secs*/ 5))
|
||||
.expect("signer is waiting for authentication");
|
||||
guard.cancel();
|
||||
cancelled_rx
|
||||
.recv_timeout(Duration::from_secs(/*secs*/ 5))
|
||||
.expect("authentication invalidated");
|
||||
assert!(!owner.is_finished());
|
||||
done_tx
|
||||
.send(Ok("late authorization"))
|
||||
.expect("complete authentication");
|
||||
assert_eq!(
|
||||
owner.join().expect("owner returned"),
|
||||
Err(UserVerificationError::Cancelled {
|
||||
@@ -42,42 +34,37 @@ fn cancellation_tears_down_the_prompt_on_the_context_owner_thread() {
|
||||
message: "the verification operation is no longer active".to_string(),
|
||||
})
|
||||
);
|
||||
exited_rx.try_recv().expect("signer exited before return");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn completed_operation_does_not_cancel_its_context() {
|
||||
let cancelled = AtomicBool::new(/*v*/ false);
|
||||
let (done_tx, done_rx) = mpsc::channel();
|
||||
done_tx
|
||||
.send(Ok("authorization"))
|
||||
.expect("complete authentication");
|
||||
assert_eq!(
|
||||
run_with_cancellation(
|
||||
&UserVerificationRequestGuard::default(),
|
||||
|| Ok("signature"),
|
||||
|| cancelled.store(/*val*/ true, Ordering::Release),
|
||||
),
|
||||
Ok("signature")
|
||||
wait_with_cancellation(&UserVerificationRequestGuard::default(), done_rx, || {
|
||||
cancelled.store(/*val*/ true, Ordering::Release)
|
||||
},),
|
||||
Ok("authorization")
|
||||
);
|
||||
assert!(!cancelled.load(Ordering::Acquire));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn already_cancelled_operation_never_starts_a_signer() {
|
||||
fn cancellation_rejects_already_completed_authorization() {
|
||||
let guard = UserVerificationRequestGuard::default();
|
||||
let (done_tx, done_rx) = mpsc::channel();
|
||||
done_tx.send(Ok(())).expect("complete authentication");
|
||||
guard.cancel();
|
||||
let started = AtomicBool::new(/*v*/ false);
|
||||
let result = run_with_cancellation(
|
||||
&guard,
|
||||
|| {
|
||||
started.store(/*val*/ true, Ordering::Release);
|
||||
Ok(())
|
||||
},
|
||||
|| panic!("no context needs cancellation"),
|
||||
);
|
||||
assert_eq!(
|
||||
result,
|
||||
wait_with_cancellation(&guard, done_rx, || panic!(
|
||||
"authentication already completed"
|
||||
)),
|
||||
Err(UserVerificationError::Cancelled {
|
||||
reason: UserVerificationCancellationReason::Interrupted,
|
||||
message: "the verification operation is no longer active".to_string(),
|
||||
})
|
||||
);
|
||||
assert!(!started.load(Ordering::Acquire));
|
||||
}
|
||||
|
||||
@@ -9,6 +9,7 @@ use core_foundation::dictionary::CFDictionary;
|
||||
use core_foundation::number::CFNumber;
|
||||
use security_framework::access_control::ProtectionMode;
|
||||
use security_framework::access_control::SecAccessControl;
|
||||
use security_framework::key::SecKey;
|
||||
use security_framework_sys::access_control::kSecAccessControlBiometryAny;
|
||||
use security_framework_sys::access_control::kSecAccessControlPrivateKeyUsage;
|
||||
use security_framework_sys::item::kSecAttrKeyClass;
|
||||
@@ -27,7 +28,11 @@ pub(super) fn access_control() -> Result<SecAccessControl, UserVerificationError
|
||||
.map_err(|err| error::classify("NSOSStatusErrorDomain", i64::from(err.code())))
|
||||
}
|
||||
|
||||
pub(super) fn validate(attributes: &CFDictionary) -> Result<(), UserVerificationError> {
|
||||
pub(super) fn validate(key: &SecKey) -> Result<(), UserVerificationError> {
|
||||
validate_attributes(&key.attributes())
|
||||
}
|
||||
|
||||
fn validate_attributes(attributes: &CFDictionary) -> Result<(), UserVerificationError> {
|
||||
let size = CFNumber::from(/*value*/ 256);
|
||||
// Do not compare opaque SecAccessControl objects: macOS can change their persisted
|
||||
// representation. Biometric signing is enforced by the policy set at key creation.
|
||||
|
||||
@@ -32,7 +32,10 @@ fn protected_attributes() -> CFMutableDictionary<CFString, CFType> {
|
||||
#[test]
|
||||
fn reuse_requires_secure_enclave_key_attributes() {
|
||||
let attributes = protected_attributes();
|
||||
assert_eq!(validate(&attributes.to_immutable().to_untyped()), Ok(()));
|
||||
assert_eq!(
|
||||
validate_attributes(&attributes.to_immutable().to_untyped()),
|
||||
Ok(())
|
||||
);
|
||||
let alternatives = unsafe {
|
||||
[
|
||||
(kSecAttrTokenID, CFString::new("software").as_CFType()),
|
||||
@@ -48,7 +51,7 @@ fn reuse_requires_secure_enclave_key_attributes() {
|
||||
let mut attributes = protected_attributes();
|
||||
attributes.set(unsafe { CFString::wrap_under_get_rule(attribute) }, value);
|
||||
assert_eq!(
|
||||
validate(&attributes.to_immutable().to_untyped()),
|
||||
validate_attributes(&attributes.to_immutable().to_untyped()),
|
||||
Err(UserVerificationError::Unavailable {
|
||||
reason: UserVerificationUnavailableReason::ProviderUnavailable,
|
||||
message: "the local credential has incompatible key protection; delete it before enrolling again"
|
||||
|
||||
@@ -15,16 +15,21 @@ use crate::UserVerificationRequestGuard;
|
||||
use crate::UserVerificationStatus;
|
||||
use crate::UserVerificationUnavailableReason;
|
||||
use crate::lifecycle_lock::LifecycleLock;
|
||||
use crate::native_operation::run_with_cancellation;
|
||||
use crate::native_operation::wait_with_cancellation;
|
||||
use base64::Engine as _;
|
||||
use base64::engine::general_purpose::URL_SAFE_NO_PAD;
|
||||
use block2::RcBlock;
|
||||
use core_foundation::base::CFType;
|
||||
use core_foundation::base::TCFType as _;
|
||||
use objc2::rc::Retained;
|
||||
use objc2::runtime::Bool;
|
||||
use objc2_foundation::NSError;
|
||||
use objc2_foundation::NSString;
|
||||
use objc2_local_authentication::LAAccessControlOperation;
|
||||
use objc2_local_authentication::LABiometryType;
|
||||
use objc2_local_authentication::LAContext;
|
||||
use objc2_local_authentication::LAPolicy;
|
||||
use security_framework::access_control::SecAccessControl;
|
||||
use security_framework::base::Error as SecurityError;
|
||||
use security_framework::item::ItemClass;
|
||||
use security_framework::item::ItemSearchOptions;
|
||||
@@ -39,6 +44,9 @@ use security_framework::key::KeyType;
|
||||
use security_framework::key::SecKey;
|
||||
use security_framework::key::Token;
|
||||
use security_framework_sys::base::errSecItemNotFound;
|
||||
use security_framework_sys::item::kSecAttrAccessControl;
|
||||
use security_framework_sys::item::kSecValueRef;
|
||||
use std::sync::mpsc;
|
||||
|
||||
pub(crate) struct NativeProvider {
|
||||
pub(crate) namespace: UserVerificationKeyNamespace,
|
||||
@@ -168,13 +176,85 @@ impl UserVerificationProvider for NativeProvider {
|
||||
} else {
|
||||
format!("{}\n\n{}", request.title, request.description)
|
||||
};
|
||||
// The keychain query retains this context and uses its text for the signing prompt.
|
||||
unsafe {
|
||||
context.setLocalizedFallbackTitle(Some(&NSString::from_str("")));
|
||||
context.setLocalizedReason(&NSString::from_str(&reason));
|
||||
}
|
||||
check_biometrics(&context)?;
|
||||
guard.check()?;
|
||||
// SecKeyCopyAttributes does not guarantee the stored item's access-control attribute.
|
||||
let attributes = match key_query(&self.namespace.label, &noninteractive_context())
|
||||
.load_refs(true)
|
||||
.load_attributes(true)
|
||||
.search()
|
||||
{
|
||||
Ok(results) => results.into_iter().find_map(|result| match result {
|
||||
SearchResult::Dict(attributes) => Some(attributes),
|
||||
_ => None,
|
||||
}),
|
||||
Err(error) if error.code() == errSecItemNotFound => {
|
||||
return Err(UserVerificationError::Unavailable {
|
||||
reason: UserVerificationUnavailableReason::CredentialMissing,
|
||||
message: "no user-verification credential has been created".to_string(),
|
||||
});
|
||||
}
|
||||
Err(error) => return Err(keychain_error(error)),
|
||||
}
|
||||
.ok_or_else(|| UserVerificationError::Failed {
|
||||
reason: UserVerificationFailureReason::ProviderError,
|
||||
message: "the user-verification key has no attributes".to_string(),
|
||||
})?;
|
||||
let key = attributes
|
||||
.find(unsafe { kSecValueRef.cast() })
|
||||
.and_then(|value| unsafe { CFType::wrap_under_get_rule(*value) }.downcast::<SecKey>())
|
||||
.ok_or_else(|| UserVerificationError::Failed {
|
||||
reason: UserVerificationFailureReason::ProviderError,
|
||||
message: "the user-verification item has no key reference".to_string(),
|
||||
})?;
|
||||
key_protection::validate(&key)?;
|
||||
let access = attributes
|
||||
.find(unsafe { kSecAttrAccessControl.cast() })
|
||||
.and_then(|value| {
|
||||
unsafe { CFType::wrap_under_get_rule(*value) }.downcast::<SecAccessControl>()
|
||||
})
|
||||
.ok_or_else(|| UserVerificationError::Failed {
|
||||
reason: UserVerificationFailureReason::ProviderError,
|
||||
message: "the user-verification key has no access control".to_string(),
|
||||
})?;
|
||||
let (done_tx, done_rx) = mpsc::sync_channel(1);
|
||||
let reply = RcBlock::new(move |success: Bool, error: *mut NSError| {
|
||||
let result = if success.as_bool() {
|
||||
Ok(())
|
||||
} else if let Some(error) = unsafe { error.as_ref() } {
|
||||
Err(error::classify(
|
||||
&error.domain().to_string(),
|
||||
error.code() as i64,
|
||||
))
|
||||
} else {
|
||||
Err(UserVerificationError::Failed {
|
||||
reason: UserVerificationFailureReason::ProviderError,
|
||||
message: "biometric authorization did not complete".to_string(),
|
||||
})
|
||||
};
|
||||
let _ = done_tx.send(result);
|
||||
});
|
||||
guard.check()?;
|
||||
// Keychain signing reconstructs its own context. Evaluate the key's policy here
|
||||
// so cancellation invalidates the context that actually owns the prompt.
|
||||
unsafe {
|
||||
context.evaluateAccessControl_operation_localizedReason_reply(
|
||||
&*access.as_concrete_TypeRef().cast(),
|
||||
LAAccessControlOperation::UseKeySign,
|
||||
&NSString::from_str(&reason),
|
||||
&reply,
|
||||
);
|
||||
}
|
||||
wait_with_cancellation(guard, done_rx, || unsafe { context.invalidate() })?;
|
||||
// Fetch a new key reference with the authorized, noninteractive context. Signing
|
||||
// must fail instead of opening another prompt if that authorization is unusable.
|
||||
unsafe {
|
||||
context.setInteractionNotAllowed(/*interaction_not_allowed*/ true)
|
||||
};
|
||||
let key =
|
||||
find_key(&self.namespace.label, &context, KeyUse::Protected)?.ok_or_else(|| {
|
||||
UserVerificationError::Unavailable {
|
||||
@@ -184,21 +264,12 @@ impl UserVerificationProvider for NativeProvider {
|
||||
})?;
|
||||
let credential = key_info(&key)?;
|
||||
guard.check()?;
|
||||
let signature = run_with_cancellation(
|
||||
guard,
|
||||
|| {
|
||||
key.create_signature(
|
||||
Algorithm::ECDSASignatureMessageX962SHA256,
|
||||
&request.challenge,
|
||||
)
|
||||
.map_err(|error| error::classify(&error.domain().to_string(), error.code() as i64))
|
||||
},
|
||||
|| {
|
||||
// LAContext remains on this thread. Invalidation cancels its pending
|
||||
// keychain authentication, allowing the signer and lifecycle lock to exit.
|
||||
unsafe { context.invalidate() };
|
||||
},
|
||||
);
|
||||
let signature = key
|
||||
.create_signature(
|
||||
Algorithm::ECDSASignatureMessageX962SHA256,
|
||||
&request.challenge,
|
||||
)
|
||||
.map_err(|error| error::classify(&error.domain().to_string(), error.code() as i64));
|
||||
guard.check()?;
|
||||
Ok(UserVerificationProof {
|
||||
credential_id: credential.credential_id,
|
||||
@@ -217,7 +288,7 @@ fn noninteractive_context() -> Retained<LAContext> {
|
||||
}
|
||||
|
||||
fn check_biometrics(context: &LAContext) -> Result<(), UserVerificationError> {
|
||||
// canEvaluatePolicy only checks availability; the protected key triggers authentication.
|
||||
// canEvaluatePolicy only checks availability; evaluating the key's policy authenticates.
|
||||
unsafe { context.canEvaluatePolicy_error(LAPolicy::DeviceOwnerAuthenticationWithBiometrics) }
|
||||
.map_err(|error| error::classify(&error.domain().to_string(), error.code() as i64))
|
||||
}
|
||||
@@ -232,6 +303,25 @@ fn find_key(
|
||||
context: &LAContext,
|
||||
key_use: KeyUse,
|
||||
) -> Result<Option<SecKey>, UserVerificationError> {
|
||||
match key_query(label, context).load_refs(true).search() {
|
||||
Ok(results) => {
|
||||
let key = results.into_iter().find_map(|result| match result {
|
||||
SearchResult::Ref(Reference::Key(key)) => Some(key),
|
||||
_ => None,
|
||||
});
|
||||
if let Some(key) = &key
|
||||
&& matches!(key_use, KeyUse::Protected)
|
||||
{
|
||||
key_protection::validate(key)?;
|
||||
}
|
||||
Ok(key)
|
||||
}
|
||||
Err(error) if error.code() == errSecItemNotFound => Ok(None),
|
||||
Err(error) => Err(keychain_error(error)),
|
||||
}
|
||||
}
|
||||
|
||||
fn key_query(label: &str, context: &LAContext) -> ItemSearchOptions {
|
||||
// Security accepts an LAContext object as kSecUseAuthenticationContext. Wrapping under
|
||||
// the get rule retains it, and ItemSearchOptions owns that retain for the query lifetime.
|
||||
let authentication = unsafe { CFType::wrap_under_get_rule(std::ptr::from_ref(context).cast()) };
|
||||
@@ -241,24 +331,8 @@ fn find_key(
|
||||
.key_class(KeyClass::private())
|
||||
.label(label)
|
||||
.local_authentication_context(Some(authentication))
|
||||
.load_refs(true)
|
||||
.limit(Limit::Max(1));
|
||||
match options.search() {
|
||||
Ok(results) => {
|
||||
let key = results.into_iter().find_map(|result| match result {
|
||||
SearchResult::Ref(Reference::Key(key)) => Some(key),
|
||||
_ => None,
|
||||
});
|
||||
if let Some(key) = &key
|
||||
&& matches!(key_use, KeyUse::Protected)
|
||||
{
|
||||
key_protection::validate(&key.attributes())?;
|
||||
}
|
||||
Ok(key)
|
||||
}
|
||||
Err(error) if error.code() == errSecItemNotFound => Ok(None),
|
||||
Err(error) => Err(keychain_error(error)),
|
||||
}
|
||||
options
|
||||
}
|
||||
|
||||
fn create_key(label: &str) -> Result<SecKey, UserVerificationError> {
|
||||
@@ -273,7 +347,7 @@ fn create_key(label: &str) -> Result<SecKey, UserVerificationError> {
|
||||
.set_access_control(access);
|
||||
let key = SecKey::new(&options)
|
||||
.map_err(|error| error::classify(&error.domain().to_string(), error.code() as i64))?;
|
||||
key_protection::validate(&key.attributes())?;
|
||||
key_protection::validate(&key)?;
|
||||
Ok(key)
|
||||
}
|
||||
|
||||
|
||||
@@ -0,0 +1,134 @@
|
||||
//! Opt-in checks of the persisted Secure Enclave signing and cancellation path.
|
||||
#![cfg(target_os = "macos")]
|
||||
|
||||
use base64::Engine as _;
|
||||
use base64::engine::general_purpose::URL_SAFE_NO_PAD;
|
||||
use codex_user_verification::UserVerificationCancellationReason;
|
||||
use codex_user_verification::UserVerificationError;
|
||||
use codex_user_verification::UserVerificationKeyNamespace;
|
||||
use codex_user_verification::UserVerificationProvider;
|
||||
use codex_user_verification::UserVerificationRequest;
|
||||
use codex_user_verification::UserVerificationRequestGuard;
|
||||
use codex_user_verification::platform_provider;
|
||||
use p256::ecdsa::Signature;
|
||||
use p256::ecdsa::VerifyingKey;
|
||||
use p256::ecdsa::signature::Verifier as _;
|
||||
use p256::pkcs8::DecodePublicKey as _;
|
||||
use pretty_assertions::assert_eq;
|
||||
use std::sync::Arc;
|
||||
use std::sync::mpsc;
|
||||
use std::time::Duration;
|
||||
|
||||
const TEST_IDENTITY: &str = "codex-user-verification-hardware-test";
|
||||
|
||||
struct TestKey(Arc<dyn UserVerificationProvider>);
|
||||
|
||||
impl Drop for TestKey {
|
||||
fn drop(&mut self) {
|
||||
let deleted = self.0.delete(&UserVerificationRequestGuard::default());
|
||||
if !std::thread::panicking() {
|
||||
assert!(deleted.is_ok(), "delete the test credential: {deleted:?}");
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
#[ignore = "requires a signed test app, a physical Touch ID Mac, and user interaction; see README.md"]
|
||||
fn persisted_key_signing_cancellation_and_retry() {
|
||||
let provider = platform_provider(UserVerificationKeyNamespace::new(TEST_IDENTITY));
|
||||
// This namespace is reserved for this test, including cleanup after a killed run.
|
||||
provider
|
||||
.delete(&UserVerificationRequestGuard::default())
|
||||
.expect("clear any previous test credential");
|
||||
if std::env::var_os("CODEX_HARDWARE_TEST_CLEANUP").is_some() {
|
||||
return;
|
||||
}
|
||||
let cleanup = TestKey(Arc::clone(&provider));
|
||||
let created = provider
|
||||
.ensure_key(&UserVerificationRequestGuard::default())
|
||||
.expect("create a persisted Secure Enclave key");
|
||||
assert!(created.created);
|
||||
let reloaded = platform_provider(UserVerificationKeyNamespace::new(TEST_IDENTITY))
|
||||
.ensure_key(&UserVerificationRequestGuard::default())
|
||||
.expect("reload the persisted key");
|
||||
assert!(!reloaded.created);
|
||||
assert_eq!(reloaded.credential, created.credential);
|
||||
let public_key = VerifyingKey::from_public_key_der(
|
||||
&URL_SAFE_NO_PAD
|
||||
.decode(&created.credential.public_key)
|
||||
.unwrap(),
|
||||
)
|
||||
.unwrap();
|
||||
let sign = |request: &UserVerificationRequest| {
|
||||
let proof = provider
|
||||
.verify(request, &UserVerificationRequestGuard::default())
|
||||
.expect("sign with the persisted key");
|
||||
assert_eq!(proof.credential_id, created.credential.credential_id);
|
||||
let signature =
|
||||
Signature::from_der(&URL_SAFE_NO_PAD.decode(proof.signature).unwrap()).unwrap();
|
||||
public_key.verify(&request.challenge, &signature).unwrap();
|
||||
};
|
||||
let mut request = UserVerificationRequest {
|
||||
challenge: b"Local hardware test. No purchase.".to_vec(),
|
||||
title: "Hardware test: authenticate".into(),
|
||||
description: "Use Touch ID once, then remove your finger.".into(),
|
||||
};
|
||||
eprintln!("Use Touch ID for the first signature.");
|
||||
sign(&request);
|
||||
|
||||
request.title = "Hardware test: cancellation".into();
|
||||
request.description =
|
||||
"Leave Touch ID untouched; this prompt should close automatically.".into();
|
||||
eprintln!("Leave the next prompt untouched; cancellation follows in three seconds.");
|
||||
let guard = UserVerificationRequestGuard::default();
|
||||
let cancellation = guard.clone();
|
||||
let (finished, completion) = mpsc::channel();
|
||||
let timer = std::thread::spawn(move || {
|
||||
if completion.recv_timeout(Duration::from_secs(/* secs */ 3))
|
||||
== Err(mpsc::RecvTimeoutError::Timeout)
|
||||
{
|
||||
cancellation.cancel();
|
||||
if completion.recv_timeout(Duration::from_secs(/* secs */ 10))
|
||||
== Err(mpsc::RecvTimeoutError::Timeout)
|
||||
{
|
||||
eprintln!(
|
||||
"Cancellation did not finish. Run with CODEX_HARDWARE_TEST_CLEANUP=1 to remove the test key."
|
||||
);
|
||||
std::process::exit(/* code */ 1);
|
||||
}
|
||||
true
|
||||
} else {
|
||||
false
|
||||
}
|
||||
});
|
||||
let result = provider.verify(&request, &guard);
|
||||
finished.send(()).unwrap();
|
||||
assert!(
|
||||
timer.join().unwrap(),
|
||||
"verification finished before cancellation"
|
||||
);
|
||||
assert!(
|
||||
matches!(
|
||||
result,
|
||||
Err(UserVerificationError::Cancelled {
|
||||
reason: UserVerificationCancellationReason::Interrupted,
|
||||
..
|
||||
})
|
||||
),
|
||||
"unexpected cancellation result: {result:?}"
|
||||
);
|
||||
|
||||
request.challenge = b"Retry signing after cancellation.".to_vec();
|
||||
request.title = "Hardware test: retry signing".into();
|
||||
request.description = "Use Touch ID to sign again with the same stored key.".into();
|
||||
eprintln!("Use Touch ID for the retry.");
|
||||
sign(&request);
|
||||
drop(cleanup);
|
||||
assert!(
|
||||
provider
|
||||
.status(&UserVerificationRequestGuard::default())
|
||||
.unwrap()
|
||||
.credential
|
||||
.is_none()
|
||||
);
|
||||
}
|
||||
Reference in New Issue
Block a user