Security-59754.80.3.tar.gz

[apple/security.git] / securityd / src / agentquery.cpp

diff --git a/securityd/src/agentquery.cpp b/securityd/src/agentquery.cpp
index 2fe7beb845a35f16340a156be7a499ac329aea6b..3c813f4ccf6dd6d41effa20219984ad9a79d6d5f 100644 (file)
--- a/securityd/src/agentquery.cpp
+++ b/securityd/src/agentquery.cpp
@@ -46,6 +46,9 @@
 #include <xpc/private.h>
 #include "securityd_service/securityd_service/securityd_service_client.h"
 
 #include <xpc/private.h>
 #include "securityd_service/securityd_service/securityd_service_client.h"
 

+// Includes for <rdar://problem/34677969> 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"
 
 #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.
     // 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
         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 {
                        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");
                }
                        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;
 
     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());
 }
 
        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);
         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
             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;
 }
 
     return outputArray;
 }
 

-OSStatus
+void

 SecurityAgentXPCQuery::invoke() {
 SecurityAgentXPCQuery::invoke() {

-    __block OSStatus status = kAuthorizationResultUndefined;
-

     xpc_object_t hintsArray = authItemSetToXPCArray(mInHints);
     xpc_object_t contextArray = authItemSetToXPCArray(mInContext);
     xpc_object_t immutableHintsArray = authItemSetToXPCArray(mImmutableHints);
     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);
     xpc_release(contextArray);
     xpc_release(immutableHintsArray);
     xpc_release(requestObject);

-
-    return status;

 }
 
 void SecurityAgentXPCQuery::checkResult()
 }
 
 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 passphrase checking requested, save KeychainDatabase reference
        // (will quietly disable check if db isn't a keychain)

-       if (needPass)
-               mPassphraseCheck = dynamic_cast<const KeychainDatabase *>(db);
+
+    // Always require password due to <rdar://problem/34677969>
+    mPassphraseCheck = dynamic_cast<const KeychainDatabase *>(db);

 
     setTerminateOnSleep(true);
 }
 
 
     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;
 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
        AuthItemSet hints, context;
 
        // prepopulate with client hints


@@ -511,7 +516,7 @@ Reason QueryKeychainUse::queryUser (const char *database, const char *descriptio
 
        // item name into hints
 
 
        // item name into hints
 

-       hints.insert(AuthItemRef(AGENT_HINT_KEYCHAIN_ITEM_NAME, AuthValueOverlay(description ? (uint32_t)strlen(description) : 0, const_cast<char*>(description))));
+    hints.insert(AuthItemRef(AGENT_HINT_KEYCHAIN_ITEM_NAME, AuthValueOverlay(description ? (uint32_t)strlen(description) : 0, const_cast<char*>(description))));

 
        // keychain name into hints
        hints.insert(AuthItemRef(AGENT_HINT_KEYCHAIN_PATH, AuthValueOverlay(database ? (uint32_t)strlen(database) : 0, const_cast<char*>(database))));
 
        // keychain name into hints
        hints.insert(AuthItemRef(AGENT_HINT_KEYCHAIN_PATH, AuthValueOverlay(database ? (uint32_t)strlen(database) : 0, const_cast<char*>(database))));


@@ -537,7 +542,12 @@ Reason QueryKeychainUse::queryUser (const char *database, const char *descriptio
             hints.erase(retryHint); hints.insert(retryHint); // replace
 
             setInput(hints, context);
             hints.erase(retryHint); hints.insert(retryHint); // replace
 
             setInput(hints, context);

-                       status = invoke();
+
+            {
+                // Must drop the common lock while showing UI.
+                StSyncLock<Mutex, Mutex> syncLock(const_cast<KeychainDatabase*>(mPassphraseCheck)->common().uiLock(), const_cast<KeychainDatabase*>(mPassphraseCheck)->common());
+                invoke();
+            }

 
             if (retryCount > kMaximumAuthorizationTries)
                        {
 
             if (retryCount > kMaximumAuthorizationTries)
                        {


@@ -551,22 +561,39 @@ Reason QueryKeychainUse::queryUser (const char *database, const char *descriptio
                                continue;
 
                        passwordItem->getCssmData(data);
                                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<KeychainDatabase*>(mPassphraseCheck)->validatePassphrase(data) && const_cast<KeychainDatabase*>(mPassphraseCheck)->decode(data)) {
+                reason = SecurityAgent::noReason;
+            } else {
+                reason = SecurityAgent::invalidPassphrase;
+            }

                }
                }

-               while ((reason = (const_cast<KeychainDatabase*>(mPassphraseCheck)->decode(data) ? SecurityAgent::noReason : SecurityAgent::invalidPassphrase)));
+        while (reason != SecurityAgent::noReason);
+        
+        readChoice();

        }
        else
        {
        }
        else
        {

-               create("builtin", "confirm-access");
-        setInput(hints, context);
-               invoke();
+//        create("builtin", "confirm-access");
+//        setInput(hints, context);
+//        invoke();
+        
+        // This is a hack to support <rdar://problem/34677969>, 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;
 }
 
        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
 //
 // 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;
 Reason QueryOld::query()
 {
        Reason reason = SecurityAgent::noReason;

-       OSStatus status;

        AuthItemSet hints, context;
        CssmAutoData passphrase(Allocator::standard(Allocator::sensitive));
        int retryCount = 0;
        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);
         hints.erase(retryHint); hints.insert(retryHint); // replace
 
         setInput(hints, context);

-        status = invoke();
+        invoke();

 
         if (retryCount > maxTries)
         {
 
         if (retryCount > maxTries)
         {


@@ -641,6 +667,11 @@ Reason QueryOld::operator () ()
 //
 Reason QueryUnlock::accept(CssmManagedData &passphrase)
 {
 //
 Reason QueryUnlock::accept(CssmManagedData &passphrase)
 {

+    // Must hold the 'common' lock to call decode; otherwise there's a data corruption issue
+    StLock<Mutex> _(safer_cast<KeychainDatabase &>(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<KeychainDatabase &>(database).decode(passphrase))
                return SecurityAgent::noReason;
        else
        if (safer_cast<KeychainDatabase &>(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;
 Reason QueryKeybagPassphrase::query()
 {
        Reason reason = SecurityAgent::noReason;

-       OSStatus status;

        AuthItemSet hints, context;
        CssmAutoData passphrase(Allocator::standard(Allocator::sensitive));
        int retryCount = 0;
        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);
         hints.erase(retryHint); hints.insert(retryHint); // replace
 
         setInput(hints, context);

-        status = invoke();
+        invoke();

 
         checkResult();
 
 
         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;
     CssmAutoData pass(Allocator::standard(Allocator::sensitive));
     CssmAutoData oldPass(Allocator::standard(Allocator::sensitive));
     Reason reason = SecurityAgent::noReason;

-       OSStatus status;

        AuthItemSet hints, context;
        int retryCount = 0;
 
        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);
         hints.erase(retryHint); hints.insert(retryHint); // replace
 
         setInput(hints, context);

-        status = invoke();
+        invoke();

 
         checkResult();
 
 
         checkResult();
 


@@ -823,7 +852,6 @@ Reason QueryNewPassphrase::query()
        CssmAutoData passphrase(Allocator::standard(Allocator::sensitive));
        CssmAutoData oldPassphrase(Allocator::standard(Allocator::sensitive));
 
        CssmAutoData passphrase(Allocator::standard(Allocator::sensitive));
        CssmAutoData oldPassphrase(Allocator::standard(Allocator::sensitive));
 

-    OSStatus status;

        AuthItemSet hints, context;
 
        int retryCount = 0;
        AuthItemSet hints, context;
 
        int retryCount = 0;


@@ -860,7 +888,7 @@ Reason QueryNewPassphrase::query()
         hints.erase(retryHint); hints.insert(retryHint); // replace
 
         setInput(hints, context);
         hints.erase(retryHint); hints.insert(retryHint); // replace
 
         setInput(hints, context);

-               status = invoke();
+               invoke();

 
                if (retryCount > maxTries)
                {
 
                if (retryCount > maxTries)
                {


@@ -940,7 +968,6 @@ Reason QueryGenericPassphrase::query(const CssmData *prompt, bool verify,
                                      string &passphrase)
 {
     Reason reason = SecurityAgent::noReason;
                                      string &passphrase)
 {
     Reason reason = SecurityAgent::noReason;

-    OSStatus status;    // not really used; remove?

     AuthItemSet hints, context;
 
     hints.insert(mClientHints.begin(), mClientHints.end());
     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);
 
     do {
         setInput(hints, context);

-               status = invoke();
+               invoke();

                checkResult();
                passwordItem = mOutContext.find(AGENT_PASSWORD);
 
                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));
 {
     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());
     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);
         hints.erase(retryHint); hints.insert(retryHint); // replace
 
         setInput(hints, context);

-               status = invoke();
+               invoke();

                checkResult();
                secretItem = mOutContext.find(AGENT_PASSWORD);
                if (!secretItem)
                checkResult();
                secretItem = mOutContext.find(AGENT_PASSWORD);
                if (!secretItem)


@@ -1046,7 +1072,7 @@ Reason QueryDBBlobSecret::accept(CssmManagedData &passphrase,
 
 // @@@  no pluggable authentication possible!
 Reason
 
 // @@@  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;
 {
     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;
        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());
 
 
     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<char*>(description))));
 
        // keychain name into hints
        hints.insert(AuthItemRef(AGENT_HINT_KEYCHAIN_ITEM_NAME, AuthValueOverlay(description ? (uint32_t)strlen(description) : 0, const_cast<char*>(description))));
 
        // keychain name into hints

-       hints.insert(AuthItemRef(AGENT_HINT_KEYCHAIN_PATH, AuthValueOverlay(database ? (uint32_t)strlen(database) : 0, const_cast<char*>(database))));
+       hints.insert(AuthItemRef(AGENT_HINT_KEYCHAIN_PATH, AuthValueOverlay(db.dbName() ? (uint32_t)strlen(db.dbName()) : 0, const_cast<char*>(db.dbName()))));

 
     create("builtin", "confirm-access-user-password");
 
     AuthItem *usernameItem;
     AuthItem *passwordItem;
 
 
     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<Mutex, Mutex> syncLock(db.common().uiLock(), db.common());

     do {
     do {

-

         AuthItemRef triesHint(AGENT_HINT_TRIES, AuthValueOverlay(sizeof(retryCount), &retryCount));
         hints.erase(triesHint); hints.insert(triesHint); // replace
 
         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)));
         usernameItem->getString(username);
         passwordItem->getString(password);
     } while ((reason = accept(username, password)));

+    syncLock.unlock();

 
     if (SecurityAgent::noReason == reason)
         logger.logSuccess();
 
     if (SecurityAgent::noReason == reason)
         logger.logSuccess();