X-Git-Url: https://git.saurik.com/apple/security.git/blobdiff_plain/6b200bc335dc93c5516ccb52f14bd896d8c7fad7..refs/heads/master:/securityd/src/agentquery.cpp?ds=sidebyside diff --git a/securityd/src/agentquery.cpp b/securityd/src/agentquery.cpp index 2fe7beb8..3c813f4c 100644 --- a/securityd/src/agentquery.cpp +++ b/securityd/src/agentquery.cpp @@ -46,6 +46,9 @@ #include #include "securityd_service/securityd_service/securityd_service_client.h" +// Includes for Always require the user's password on keychain approval dialogs +#include "server.h" + #define SECURITYAGENT_BOOTSTRAP_NAME_BASE "com.apple.security.agent" #define SECURITYAGENT_LOGINWINDOW_BOOTSTRAP_NAME_BASE "com.apple.security.agent.login" @@ -106,7 +109,7 @@ SecurityAgentXPCConnection::~SecurityAgentXPCConnection() // If a connection has been established, we need to tear it down. if (NULL != mXPCConnection) { // Tearing this down is a multi-step process. First, request a cancellation. - // This is safe even if the connection is already in the cancelled state. + // This is safe even if the connection is already in the canceled state. xpc_connection_cancel(mXPCConnection); // Then release the XPC connection @@ -147,7 +150,8 @@ SecurityAgentXPCConnection::activate(bool ignoreUid) xpc_connection_set_target_uid(mXPCConnection, targetUid); secnotice("SecurityAgentXPCConnection", "Creating a standard security agent"); } else { - mXPCConnection = xpc_connection_create_mach_service(SECURITYAGENT_LOGINWINDOW_BOOTSTRAP_NAME_BASE, NULL, 0); + mXPCConnection = xpc_connection_create_mach_service(SECURITYAGENT_LOGINWINDOW_BOOTSTRAP_NAME_BASE, NULL, + XPC_CONNECTION_MACH_SERVICE_PRIVILEGED); xpc_connection_set_instance(mXPCConnection, sessionUUID); secnotice("SecurityAgentXPCConnection", "Creating a loginwindow security agent"); } @@ -247,7 +251,7 @@ SecurityAgentXPCQuery::inferHints(Process &thisProcess) bool validSignature = thisProcess.checkAppleSigned(); AuthItemSet clientImmutableHints; - clientImmutableHints.insert(AuthItemRef(AGENT_HINT_PROCESS_SIGNED, AuthValueOverlay(sizeof(validSignature), &validSignature))); + clientImmutableHints.insert(AuthItemRef(AGENT_HINT_CLIENT_SIGNED, AuthValueOverlay(sizeof(validSignature), &validSignature))); mImmutableHints.insert(clientImmutableHints.begin(), clientImmutableHints.end()); } @@ -318,6 +322,10 @@ static void xpcArrayToAuthItemSet(AuthItemSet *setToBuild, xpc_object_t input) { bool sensitive = xpc_dictionary_get_value(item, AUTH_XPC_ITEM_SENSITIVE_VALUE_LENGTH); if (sensitive) { size_t sensitiveLength = (size_t)xpc_dictionary_get_uint64(item, AUTH_XPC_ITEM_SENSITIVE_VALUE_LENGTH); + if (sensitiveLength > length) { + secnotice("SecurityAgentXPCQuery", "Sensitive data len %zu is not valid", sensitiveLength); + return true; + } dataCopy = malloc(sensitiveLength); memcpy(dataCopy, data, sensitiveLength); memset_s((void *)data, length, 0, sensitiveLength); // clear the sensitive data, memset_s is never optimized away @@ -427,10 +435,8 @@ static xpc_object_t authItemSetToXPCArray(AuthItemSet input) { return outputArray; } -OSStatus +void SecurityAgentXPCQuery::invoke() { - __block OSStatus status = kAuthorizationResultUndefined; - xpc_object_t hintsArray = authItemSetToXPCArray(mInHints); xpc_object_t contextArray = authItemSetToXPCArray(mInContext); xpc_object_t immutableHintsArray = authItemSetToXPCArray(mImmutableHints); @@ -467,8 +473,6 @@ SecurityAgentXPCQuery::invoke() { xpc_release(contextArray); xpc_release(immutableHintsArray); xpc_release(requestObject); - - return status; } void SecurityAgentXPCQuery::checkResult() @@ -490,17 +494,18 @@ QueryKeychainUse::QueryKeychainUse(bool needPass, const Database *db) { // if passphrase checking requested, save KeychainDatabase reference // (will quietly disable check if db isn't a keychain) - if (needPass) - mPassphraseCheck = dynamic_cast(db); + + // Always require password due to + mPassphraseCheck = dynamic_cast(db); setTerminateOnSleep(true); } +// Callers to this function must hold the common lock Reason QueryKeychainUse::queryUser (const char *database, const char *description, AclAuthorization action) { Reason reason = SecurityAgent::noReason; - uint32_t retryCount = 0; - OSStatus status; + uint32_t retryCount = 0; AuthItemSet hints, context; // prepopulate with client hints @@ -511,7 +516,7 @@ Reason QueryKeychainUse::queryUser (const char *database, const char *descriptio // item name into hints - hints.insert(AuthItemRef(AGENT_HINT_KEYCHAIN_ITEM_NAME, AuthValueOverlay(description ? (uint32_t)strlen(description) : 0, const_cast(description)))); + hints.insert(AuthItemRef(AGENT_HINT_KEYCHAIN_ITEM_NAME, AuthValueOverlay(description ? (uint32_t)strlen(description) : 0, const_cast(description)))); // keychain name into hints hints.insert(AuthItemRef(AGENT_HINT_KEYCHAIN_PATH, AuthValueOverlay(database ? (uint32_t)strlen(database) : 0, const_cast(database)))); @@ -537,7 +542,12 @@ Reason QueryKeychainUse::queryUser (const char *database, const char *descriptio hints.erase(retryHint); hints.insert(retryHint); // replace setInput(hints, context); - status = invoke(); + + { + // Must drop the common lock while showing UI. + StSyncLock syncLock(const_cast(mPassphraseCheck)->common().uiLock(), const_cast(mPassphraseCheck)->common()); + invoke(); + } if (retryCount > kMaximumAuthorizationTries) { @@ -551,22 +561,39 @@ Reason QueryKeychainUse::queryUser (const char *database, const char *descriptio continue; passwordItem->getCssmData(data); + + // decode() replaces the master key, so do this only if we know the passphrase is correct. + // I suspect decode() is redundant but something might rely on its side effects so let's keep it. + if (const_cast(mPassphraseCheck)->validatePassphrase(data) && const_cast(mPassphraseCheck)->decode(data)) { + reason = SecurityAgent::noReason; + } else { + reason = SecurityAgent::invalidPassphrase; + } } - while ((reason = (const_cast(mPassphraseCheck)->decode(data) ? SecurityAgent::noReason : SecurityAgent::invalidPassphrase))); + while (reason != SecurityAgent::noReason); + + readChoice(); } else { - create("builtin", "confirm-access"); - setInput(hints, context); - invoke(); +// create("builtin", "confirm-access"); +// setInput(hints, context); +// invoke(); + + // This is a hack to support , we can never simply prompt for confirmation + secerror("ACL validation fallback case! Must ask user for account password because we have no database"); + Session &session = Server::session(); + try{ + session.verifyKeyStorePassphrase(1, true, description); + } catch (...) { + return SecurityAgent::invalidPassphrase; + } + SecurityAgentXPCQuery::allow = true; } - readChoice(); - return reason; } - // // Obtain passphrases and submit them to the accept() method until it is accepted // or we can't get another passphrase. Accept() should consume the passphrase @@ -575,7 +602,6 @@ Reason QueryKeychainUse::queryUser (const char *database, const char *descriptio Reason QueryOld::query() { Reason reason = SecurityAgent::noReason; - OSStatus status; AuthItemSet hints, context; CssmAutoData passphrase(Allocator::standard(Allocator::sensitive)); int retryCount = 0; @@ -605,7 +631,7 @@ Reason QueryOld::query() hints.erase(retryHint); hints.insert(retryHint); // replace setInput(hints, context); - status = invoke(); + invoke(); if (retryCount > maxTries) { @@ -641,6 +667,11 @@ Reason QueryOld::operator () () // Reason QueryUnlock::accept(CssmManagedData &passphrase) { + // Must hold the 'common' lock to call decode; otherwise there's a data corruption issue + StLock _(safer_cast(database).common()); + + // Calling validatePassphrase here throws when trying to constitute a key. + // Unsure why but since this is for the KC unlock path and not a validation path the wrong password won't make things worse. if (safer_cast(database).decode(passphrase)) return SecurityAgent::noReason; else @@ -670,7 +701,6 @@ QueryKeybagPassphrase::QueryKeybagPassphrase(Session & session, int32_t tries) : Reason QueryKeybagPassphrase::query() { Reason reason = SecurityAgent::noReason; - OSStatus status; AuthItemSet hints, context; CssmAutoData passphrase(Allocator::standard(Allocator::sensitive)); int retryCount = 0; @@ -701,7 +731,7 @@ Reason QueryKeybagPassphrase::query() hints.erase(retryHint); hints.insert(retryHint); // replace setInput(hints, context); - status = invoke(); + invoke(); checkResult(); @@ -732,7 +762,6 @@ Reason QueryKeybagNewPassphrase::query(CssmOwnedData &oldPassphrase, CssmOwnedDa CssmAutoData pass(Allocator::standard(Allocator::sensitive)); CssmAutoData oldPass(Allocator::standard(Allocator::sensitive)); Reason reason = SecurityAgent::noReason; - OSStatus status; AuthItemSet hints, context; int retryCount = 0; @@ -766,7 +795,7 @@ Reason QueryKeybagNewPassphrase::query(CssmOwnedData &oldPassphrase, CssmOwnedDa hints.erase(retryHint); hints.insert(retryHint); // replace setInput(hints, context); - status = invoke(); + invoke(); checkResult(); @@ -823,7 +852,6 @@ Reason QueryNewPassphrase::query() CssmAutoData passphrase(Allocator::standard(Allocator::sensitive)); CssmAutoData oldPassphrase(Allocator::standard(Allocator::sensitive)); - OSStatus status; AuthItemSet hints, context; int retryCount = 0; @@ -860,7 +888,7 @@ Reason QueryNewPassphrase::query() hints.erase(retryHint); hints.insert(retryHint); // replace setInput(hints, context); - status = invoke(); + invoke(); if (retryCount > maxTries) { @@ -940,7 +968,6 @@ Reason QueryGenericPassphrase::query(const CssmData *prompt, bool verify, string &passphrase) { Reason reason = SecurityAgent::noReason; - OSStatus status; // not really used; remove? AuthItemSet hints, context; hints.insert(mClientHints.begin(), mClientHints.end()); @@ -959,7 +986,7 @@ Reason QueryGenericPassphrase::query(const CssmData *prompt, bool verify, do { setInput(hints, context); - status = invoke(); + invoke(); checkResult(); passwordItem = mOutContext.find(AGENT_PASSWORD); @@ -983,7 +1010,6 @@ Reason QueryDBBlobSecret::query(DbHandle *dbHandleArray, uint8 dbHandleArrayCoun { Reason reason = SecurityAgent::noReason; CssmAutoData passphrase(Allocator::standard(Allocator::sensitive)); - OSStatus status; // not really used; remove? AuthItemSet hints/*NUKEME*/, context; hints.insert(mClientHints.begin(), mClientHints.end()); @@ -1006,7 +1032,7 @@ Reason QueryDBBlobSecret::query(DbHandle *dbHandleArray, uint8 dbHandleArrayCoun hints.erase(retryHint); hints.insert(retryHint); // replace setInput(hints, context); - status = invoke(); + invoke(); checkResult(); secretItem = mOutContext.find(AGENT_PASSWORD); if (!secretItem) @@ -1046,7 +1072,7 @@ Reason QueryDBBlobSecret::accept(CssmManagedData &passphrase, // @@@ no pluggable authentication possible! Reason -QueryKeychainAuth::operator () (const char *database, const char *description, AclAuthorization action, const char *prompt) +QueryKeychainAuth::performQuery(const KeychainDatabase& db, const char *description, AclAuthorization action, const char *prompt) { Reason reason = SecurityAgent::noReason; AuthItemSet hints, context; @@ -1055,7 +1081,7 @@ QueryKeychainAuth::operator () (const char *database, const char *description, A string password; using CommonCriteria::Securityd::KeychainAuthLogger; - KeychainAuthLogger logger(mAuditToken, AUE_ssauthint, database, description); + KeychainAuthLogger logger(mAuditToken, (short)AUE_ssauthint, db.dbName(), description); hints.insert(mClientHints.begin(), mClientHints.end()); @@ -1068,15 +1094,16 @@ QueryKeychainAuth::operator () (const char *database, const char *description, A hints.insert(AuthItemRef(AGENT_HINT_KEYCHAIN_ITEM_NAME, AuthValueOverlay(description ? (uint32_t)strlen(description) : 0, const_cast(description)))); // keychain name into hints - hints.insert(AuthItemRef(AGENT_HINT_KEYCHAIN_PATH, AuthValueOverlay(database ? (uint32_t)strlen(database) : 0, const_cast(database)))); + hints.insert(AuthItemRef(AGENT_HINT_KEYCHAIN_PATH, AuthValueOverlay(db.dbName() ? (uint32_t)strlen(db.dbName()) : 0, const_cast(db.dbName())))); create("builtin", "confirm-access-user-password"); AuthItem *usernameItem; AuthItem *passwordItem; + // This entire do..while requires the UI lock because we do accept() in the condition, which in some cases is reentrant + StSyncLock syncLock(db.common().uiLock(), db.common()); do { - AuthItemRef triesHint(AGENT_HINT_TRIES, AuthValueOverlay(sizeof(retryCount), &retryCount)); hints.erase(triesHint); hints.insert(triesHint); // replace @@ -1112,6 +1139,7 @@ QueryKeychainAuth::operator () (const char *database, const char *description, A usernameItem->getString(username); passwordItem->getString(password); } while ((reason = accept(username, password))); + syncLock.unlock(); if (SecurityAgent::noReason == reason) logger.logSuccess();