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 bbc7eefe9f457bacee0d1d4aea1413c94acd3488..3c813f4ccf6dd6d41effa20219984ad9a79d6d5f 100644 (file)
--- a/securityd/src/agentquery.cpp
+++ b/securityd/src/agentquery.cpp
@@ -24,8 +24,10 @@
 //
 // passphrases - canonical code to obtain passphrases
 //
 //
 // passphrases - canonical code to obtain passphrases
 //

+#define __STDC_WANT_LIB_EXT1__ 1
+#include <string.h>
+

 #include "agentquery.h"
 #include "agentquery.h"

-#include "authority.h"

 #include "ccaudit_extensions.h"
 
 #include <Security/AuthorizationTags.h>
 #include "ccaudit_extensions.h"
 
 #include <Security/AuthorizationTags.h>


@@ -44,14 +46,17 @@
 #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"

-#define AUTHORIZATIONHOST_BOOTSTRAP_NAME_BASE           "com.apple.security.authhost"

 
 #define AUTH_XPC_ITEM_NAME  "_item_name"
 #define AUTH_XPC_ITEM_FLAGS "_item_flags"
 #define AUTH_XPC_ITEM_VALUE "_item_value"
 #define AUTH_XPC_ITEM_TYPE  "_item_type"
 
 #define AUTH_XPC_ITEM_NAME  "_item_name"
 #define AUTH_XPC_ITEM_FLAGS "_item_flags"
 #define AUTH_XPC_ITEM_VALUE "_item_value"
 #define AUTH_XPC_ITEM_TYPE  "_item_type"

+#define AUTH_XPC_ITEM_SENSITIVE_VALUE_LENGTH "_item_sensitive_value_length"

 
 #define AUTH_XPC_REQUEST_METHOD_KEY "_agent_request_key"
 #define AUTH_XPC_REQUEST_METHOD_CREATE "_agent_request_create"
 
 #define AUTH_XPC_REQUEST_METHOD_KEY "_agent_request_key"
 #define AUTH_XPC_REQUEST_METHOD_CREATE "_agent_request_create"


@@ -77,52 +82,17 @@
 { 0,0,0,0, 0,0,0,0, 0,0,0,0, (unsigned char)((0xff000000 & (sessionid))>>24), (unsigned char)((0x00ff0000 & (sessionid))>>16), (unsigned char)((0x0000ff00 & (sessionid))>>8),  (unsigned char)((0x000000ff & (sessionid))) }
 
 
 { 0,0,0,0, 0,0,0,0, 0,0,0,0, (unsigned char)((0xff000000 & (sessionid))>>24), (unsigned char)((0x00ff0000 & (sessionid))>>16), (unsigned char)((0x0000ff00 & (sessionid))>>8),  (unsigned char)((0x000000ff & (sessionid))) }
 
 

-// NOSA support functions. This is a test mode where the SecurityAgent
-// is simulated via stdio in the client. Good for running automated tests
-// of client programs. Only available if -DNOSA when compiling.
-
-#if defined(NOSA)
-#include <cstdarg>
-
-static void getNoSA(char *buffer, size_t bufferSize, const char *fmt, ...)
-{
-       // write prompt
-       va_list args;
-       va_start(args, fmt);
-       vfprintf(stdout, fmt, args);
-       va_end(args);
-
-       // read reply
-       memset(buffer, 0, bufferSize);
-       const char *nosa = getenv("NOSA");
-       if (!strcmp(nosa, "-")) {
-               if (fgets(buffer, bufferSize-1, stdin) == NULL)
-                       CssmError::throwMe(CSSM_ERRCODE_NO_USER_INTERACTION);
-               buffer[strlen(buffer)-1] = '\0';        // remove trailing newline
-               if (!isatty(fileno(stdin)))
-                       printf("%s\n", buffer);                 // echo to output if input not terminal
-       } else {
-               strncpy(buffer, nosa, bufferSize-1);
-               printf("%s\n", buffer);
-       }
-       if (buffer[0] == '\0')                          // empty input -> cancellation
-               CssmError::throwMe(CSSM_ERRCODE_USER_CANCELED);
-}
-#endif // NOSA
-
-

 // SecurityAgentXPCConnection
 
 // SecurityAgentXPCConnection
 

-SecurityAgentXPCConnection::SecurityAgentXPCConnection(const AuthHostType type, Session &session)
-: mAuthHostType(type),
-mHostInstance(session.authhost(mAuthHostType)),
+SecurityAgentXPCConnection::SecurityAgentXPCConnection(Session &session)
+: mHostInstance(session.authhost()),

 mSession(session),
 mConnection(&Server::connection()),
 mAuditToken(Server::connection().auditToken())
 {
        // this may take a while
        Server::active().longTermActivity();
 mSession(session),
 mConnection(&Server::connection()),
 mAuditToken(Server::connection().auditToken())
 {
        // this may take a while
        Server::active().longTermActivity();

-    secdebug("SecurityAgentConnection", "new SecurityAgentConnection(%p)", this);
+    secnotice("SecurityAgentConnection", "new SecurityAgentConnection(%p)", this);

     mXPCConnection = NULL;
     mNobodyUID = -2;
     struct passwd *pw = getpwnam("nobody");
     mXPCConnection = NULL;
     mNobodyUID = -2;
     struct passwd *pw = getpwnam("nobody");


@@ -133,13 +103,13 @@ mAuditToken(Server::connection().auditToken())
 
 SecurityAgentXPCConnection::~SecurityAgentXPCConnection()
 {
 
 SecurityAgentXPCConnection::~SecurityAgentXPCConnection()
 {

-    secdebug("SecurityAgentConnection", "SecurityAgentConnection(%p) dying", this);
+    secnotice("SecurityAgentConnection", "SecurityAgentConnection(%p) dying", this);

        mConnection->useAgent(NULL);
 
     // 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.
        mConnection->useAgent(NULL);
 
     // 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


@@ -156,7 +126,7 @@ bool SecurityAgentXPCConnection::inDarkWake()
 void
 SecurityAgentXPCConnection::activate(bool ignoreUid)
 {
 void
 SecurityAgentXPCConnection::activate(bool ignoreUid)
 {

-    secdebug("SecurityAgentConnection", "activate(%p)", this);
+    secnotice("SecurityAgentConnection", "activate(%p)", this);

 
        mConnection->useAgent(this);
     if (mXPCConnection != NULL) {
 
        mConnection->useAgent(this);
     if (mXPCConnection != NULL) {


@@ -167,53 +137,40 @@ SecurityAgentXPCConnection::activate(bool ignoreUid)
        try {
                uuid_t sessionUUID = UUID_INITIALIZER_FROM_SESSIONID(mSession.sessionId());
 
        try {
                uuid_t sessionUUID = UUID_INITIALIZER_FROM_SESSIONID(mSession.sessionId());
 

-        if (mAuthHostType == securityAgent) {
-            // Yes, these need to be throws, as we're still in securityd, and thus still have to do flow control with exceptions.
-            if (!(mSession.attributes() & sessionHasGraphicAccess))
-                CssmError::throwMe(CSSM_ERRCODE_NO_USER_INTERACTION);
-            if (inDarkWake())
-                CssmError::throwMe(CSSM_ERRCODE_IN_DARK_WAKE);
-            uid_t targetUid = mHostInstance->session().originatorUid();
-
-            secdebug("SecurityAgentXPCConnection","Retrieved UID %d for this session", targetUid);
-                       if (!ignoreUid && targetUid != 0 && targetUid != mNobodyUID) {
-                               mXPCConnection = xpc_connection_create_mach_service(SECURITYAGENT_BOOTSTRAP_NAME_BASE, NULL, 0);
-                               xpc_connection_set_target_uid(mXPCConnection, targetUid);
-                               secdebug("SecurityAgentXPCConnection", "Creating a standard security agent");
-                       } else {
-                               mXPCConnection = xpc_connection_create_mach_service(SECURITYAGENT_LOGINWINDOW_BOOTSTRAP_NAME_BASE, NULL, 0);
-                               xpc_connection_set_instance(mXPCConnection, sessionUUID);
-                               secdebug("SecurityAgentXPCConnection", "Creating a loginwindow security agent");
-            }
-        } else {
-            mXPCConnection = xpc_connection_create_mach_service(AUTHORIZATIONHOST_BOOTSTRAP_NAME_BASE, NULL, 0);
+               // Yes, these need to be throws, as we're still in securityd, and thus still have to do flow control with exceptions.
+               if (!(mSession.attributes() & sessionHasGraphicAccess))
+                       CssmError::throwMe(CSSM_ERRCODE_NO_USER_INTERACTION);
+               if (inDarkWake())
+                       CssmError::throwMe(CSSM_ERRCODE_IN_DARK_WAKE);
+               uid_t targetUid = mHostInstance->session().originatorUid();
+
+               secnotice("SecurityAgentXPCConnection","Retrieved UID %d for this session", targetUid);
+               if (!ignoreUid && targetUid != 0 && targetUid != mNobodyUID) {
+                       mXPCConnection = xpc_connection_create_mach_service(SECURITYAGENT_BOOTSTRAP_NAME_BASE, NULL, 0);
+                       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,
+                                                                XPC_CONNECTION_MACH_SERVICE_PRIVILEGED);

                        xpc_connection_set_instance(mXPCConnection, sessionUUID);
                        xpc_connection_set_instance(mXPCConnection, sessionUUID);

-            secdebug("SecurityAgentXPCConnection", "Creating a standard authhost");
-        }
+                       secnotice("SecurityAgentXPCConnection", "Creating a loginwindow security agent");
+               }

 
                xpc_connection_set_event_handler(mXPCConnection, ^(xpc_object_t object) {
                        if (xpc_get_type(object) == XPC_TYPE_ERROR) {
 
                xpc_connection_set_event_handler(mXPCConnection, ^(xpc_object_t object) {
                        if (xpc_get_type(object) == XPC_TYPE_ERROR) {

-                               secdebug("SecurityAgentXPCConnection", "error during xpc: %s", xpc_dictionary_get_string(object, XPC_ERROR_KEY_DESCRIPTION));
+                               secnotice("SecurityAgentXPCConnection", "error during xpc: %s", xpc_dictionary_get_string(object, XPC_ERROR_KEY_DESCRIPTION));

                        }
                });
                xpc_connection_resume(mXPCConnection);
                        }
                });
                xpc_connection_resume(mXPCConnection);

-               secdebug("SecurityAgentXPCConnection", "%p activated", this);
+               secnotice("SecurityAgentXPCConnection", "%p activated", this);

        }
     catch (MacOSError &err) {
                mConnection->useAgent(NULL);    // guess not
        }
     catch (MacOSError &err) {
                mConnection->useAgent(NULL);    // guess not

-        Syslog::error("SecurityAgentConnection: error activating %s instance %p",
-                      mAuthHostType == privilegedAuthHost
-                      ? "authorizationhost"
-                      : "SecurityAgent", this);
+        Syslog::error("SecurityAgentConnection: error activating SecurityAgent instance %p", this);

                throw;
        }
 
                throw;
        }
 

-    secdebug("SecurityAgentXPCConnection", "contact didn't throw (%p)", this);
-}
-
-void
-SecurityAgentXPCConnection::reconnect()
-{
+    secnotice("SecurityAgentXPCConnection", "contact didn't throw (%p)", this);

 }
 
 void
 }
 
 void


@@ -250,15 +207,15 @@ SecurityAgentXPCQuery::killAllXPCClients()
 }
 
 
 }
 
 

-SecurityAgentXPCQuery::SecurityAgentXPCQuery(const AuthHostType type, Session &session)
-: SecurityAgentXPCConnection(type, session), mAgentConnected(false), mTerminateOnSleep(false)
+SecurityAgentXPCQuery::SecurityAgentXPCQuery(Session &session)
+: SecurityAgentXPCConnection(session), mAgentConnected(false), mTerminateOnSleep(false)

 {
 {

-    secdebug("SecurityAgentXPCQuery", "new SecurityAgentXPCQuery(%p)", this);
+    secnotice("SecurityAgentXPCQuery", "new SecurityAgentXPCQuery(%p)", this);

 }
 
 SecurityAgentXPCQuery::~SecurityAgentXPCQuery()
 {
 }
 
 SecurityAgentXPCQuery::~SecurityAgentXPCQuery()
 {

-    secdebug("SecurityAgentXPCQuery", "SecurityAgentXPCQuery(%p) dying", this);
+    secnotice("SecurityAgentXPCQuery", "SecurityAgentXPCQuery(%p) dying", this);

     if (mAgentConnected) {
         this->disconnect();
     }
     if (mAgentConnected) {
         this->disconnect();
     }


@@ -267,27 +224,34 @@ SecurityAgentXPCQuery::~SecurityAgentXPCQuery()
 void
 SecurityAgentXPCQuery::inferHints(Process &thisProcess)
 {
 void
 SecurityAgentXPCQuery::inferHints(Process &thisProcess)
 {

-    string guestPath;
-       if (SecCodeRef clientCode = thisProcess.currentGuest())
-               guestPath = codePath(clientCode);
-

     AuthItemSet clientHints;
     SecurityAgent::RequestorType type = SecurityAgent::bundle;
     pid_t clientPid = thisProcess.pid();
     uid_t clientUid = thisProcess.uid();
     AuthItemSet clientHints;
     SecurityAgent::RequestorType type = SecurityAgent::bundle;
     pid_t clientPid = thisProcess.pid();
     uid_t clientUid = thisProcess.uid();

+    string guestPath = thisProcess.getPath();
+    Boolean ignoreSession = TRUE;

 
        clientHints.insert(AuthItemRef(AGENT_HINT_CLIENT_TYPE, AuthValueOverlay(sizeof(type), &type)));
        clientHints.insert(AuthItemRef(AGENT_HINT_CLIENT_PATH, AuthValueOverlay(guestPath)));
        clientHints.insert(AuthItemRef(AGENT_HINT_CLIENT_PID, AuthValueOverlay(sizeof(clientPid), &clientPid)));
        clientHints.insert(AuthItemRef(AGENT_HINT_CLIENT_UID, AuthValueOverlay(sizeof(clientUid), &clientUid)));
 
 
        clientHints.insert(AuthItemRef(AGENT_HINT_CLIENT_TYPE, AuthValueOverlay(sizeof(type), &type)));
        clientHints.insert(AuthItemRef(AGENT_HINT_CLIENT_PATH, AuthValueOverlay(guestPath)));
        clientHints.insert(AuthItemRef(AGENT_HINT_CLIENT_PID, AuthValueOverlay(sizeof(clientPid), &clientPid)));
        clientHints.insert(AuthItemRef(AGENT_HINT_CLIENT_UID, AuthValueOverlay(sizeof(clientUid), &clientUid)));
 

+    /*
+     * If its loginwindow that's asking, override the loginwindow shield detection
+     * up front so that it can trigger SecurityAgent dialogs (like password change)
+     * for when the OD password and keychain password is out of sync.
+     */
+
+    if (guestPath == "/System/Library/CoreServices/loginwindow.app") {
+        clientHints.insert(AuthItemRef(AGENT_HINT_IGNORE_SESSION, AuthValueOverlay(sizeof(ignoreSession), &ignoreSession)));
+    }

 
        mClientHints.insert(clientHints.begin(), clientHints.end());
 
     bool validSignature = thisProcess.checkAppleSigned();
     AuthItemSet clientImmutableHints;
 
 
        mClientHints.insert(clientHints.begin(), clientHints.end());
 
     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());
 }


@@ -352,8 +316,24 @@ static void xpcArrayToAuthItemSet(AuthItemSet *setToBuild, xpc_object_t input) {
 
         size_t length;
         const void *data = xpc_dictionary_get_data(item, AUTH_XPC_ITEM_VALUE, &length);
 
         size_t length;
         const void *data = xpc_dictionary_get_data(item, AUTH_XPC_ITEM_VALUE, &length);

-        void *dataCopy = malloc(length);
-        memcpy(dataCopy, data, length);
+        void *dataCopy = 0;
+
+        // <rdar://problem/13033889> authd is holding on to multiple copies of my password in the clear
+        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
+            length = sensitiveLength;
+        } else {
+            dataCopy = malloc(length);
+            memcpy(dataCopy, data, length);
+        }

 
         uint64_t flags = xpc_dictionary_get_uint64(item, AUTH_XPC_ITEM_FLAGS);
         AuthItemRef nextItem(name, AuthValueOverlay((uint32_t)length, dataCopy), (uint32_t)flags);
 
         uint64_t flags = xpc_dictionary_get_uint64(item, AUTH_XPC_ITEM_FLAGS);
         AuthItemRef nextItem(name, AuthValueOverlay((uint32_t)length, dataCopy), (uint32_t)flags);


@@ -386,16 +366,16 @@ SecurityAgentXPCQuery::create(const char *pluginId, const char *mechanismId)
         if (doSwitchAudit) {
             mach_port_name_t jobPort;
             if (0 == audit_session_port(mSession.sessionId(), &jobPort)) {
         if (doSwitchAudit) {
             mach_port_name_t jobPort;
             if (0 == audit_session_port(mSession.sessionId(), &jobPort)) {

-                secdebug("SecurityAgentXPCQuery", "attaching an audit session port because the uid was %d", targetUid);
+                secnotice("SecurityAgentXPCQuery", "attaching an audit session port because the uid was %d", targetUid);

                 xpc_dictionary_set_mach_send(requestObject, AUTH_XPC_AUDIT_SESSION_PORT, jobPort);
                 if (mach_port_mod_refs(mach_task_self(), jobPort, MACH_PORT_RIGHT_SEND, -1) != KERN_SUCCESS) {
                 xpc_dictionary_set_mach_send(requestObject, AUTH_XPC_AUDIT_SESSION_PORT, jobPort);
                 if (mach_port_mod_refs(mach_task_self(), jobPort, MACH_PORT_RIGHT_SEND, -1) != KERN_SUCCESS) {

-                    secdebug("SecurityAgentXPCQuery", "unable to release send right for audit session, leaking");
+                    secnotice("SecurityAgentXPCQuery", "unable to release send right for audit session, leaking");

                 }
             }
         }
 
         if (doSwitchBootstrap) {
                 }
             }
         }
 
         if (doSwitchBootstrap) {

-            secdebug("SecurityAgentXPCQuery", "attaching a bootstrap port because the uid was %d", targetUid);
+            secnotice("SecurityAgentXPCQuery", "attaching a bootstrap port because the uid was %d", targetUid);

             MachPlusPlus::Bootstrap processBootstrap = Server::process().taskPort().bootstrap();
             xpc_dictionary_set_mach_send(requestObject, AUTH_XPC_BOOTSTRAP_PORT, processBootstrap);
         }
             MachPlusPlus::Bootstrap processBootstrap = Server::process().taskPort().bootstrap();
             xpc_dictionary_set_mach_send(requestObject, AUTH_XPC_BOOTSTRAP_PORT, processBootstrap);
         }


@@ -408,7 +388,7 @@ SecurityAgentXPCQuery::create(const char *pluginId, const char *mechanismId)
                 if (status == kAuthorizationResultAllow) {
                     mAgentConnected = true;
                 } else {
                 if (status == kAuthorizationResultAllow) {
                     mAgentConnected = true;
                 } else {

-                    secdebug("SecurityAgentXPCQuery", "plugin create failed in SecurityAgent");
+                    secnotice("SecurityAgentXPCQuery", "plugin create failed in SecurityAgent");

                     MacOSError::throwMe(errAuthorizationInternal);
                 }
             }
                     MacOSError::throwMe(errAuthorizationInternal);
                 }
             }


@@ -416,11 +396,11 @@ SecurityAgentXPCQuery::create(const char *pluginId, const char *mechanismId)
             if (XPC_ERROR_CONNECTION_INVALID == object) {
                 // If we get an error before getting the create response, try again without the UID
                 if (ignoreUid) {
             if (XPC_ERROR_CONNECTION_INVALID == object) {
                 // If we get an error before getting the create response, try again without the UID
                 if (ignoreUid) {

-                    secdebug("SecurityAgentXPCQuery", "failed to establish connection, no retries left");
+                    secnotice("SecurityAgentXPCQuery", "failed to establish connection, no retries left");

                     xpc_release(object);
                     MacOSError::throwMe(errAuthorizationInternal);
                 } else {
                     xpc_release(object);
                     MacOSError::throwMe(errAuthorizationInternal);
                 } else {

-                    secdebug("SecurityAgentXPCQuery", "failed to establish connection, retrying with no UID");
+                    secnotice("SecurityAgentXPCQuery", "failed to establish connection, retrying with no UID");

                     ignoreUid = true;
                     xpc_release(mXPCConnection);
                     mXPCConnection = NULL;
                     ignoreUid = true;
                     xpc_release(mXPCConnection);
                     mXPCConnection = NULL;


@@ -455,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);


@@ -495,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()


@@ -518,45 +494,20 @@ 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;

-    int retryCount = 0;
-       OSStatus status;
-       AuthValueVector arguments;
+       uint32_t retryCount = 0;

        AuthItemSet hints, context;
 
        AuthItemSet hints, context;
 

-#if defined(NOSA)
-       if (getenv("NOSA")) {
-               char answer[maxPassphraseLength+10];
-
-        string applicationPath;
-        AuthItem *applicationPathItem = mClientHints.find(AGENT_HINT_APPLICATION_PATH);
-               if (applicationPathItem)
-                 applicationPathItem->getString(applicationPath);
-
-               getNoSA(answer, sizeof(answer), "Allow %s to do %d on %s in %s? [yn][g]%s ",
-                       applicationPath.c_str(), int(action), (description ? description : "[NULL item]"),
-                       (database ? database : "[NULL database]"),
-                       mPassphraseCheck ? ":passphrase" : "");
-               // turn passphrase (no ':') into y:passphrase
-               if (mPassphraseCheck && !strchr(answer, ':')) {
-                       memmove(answer+2, answer, strlen(answer)+1);
-                       memcpy(answer, "y:", 2);
-               }
-
-               allow = answer[0] == 'y';
-               remember = answer[1] == 'g';
-               return SecurityAgent::noReason;
-       }
-#endif
-

        // prepopulate with client hints
        hints.insert(mClientHints.begin(), mClientHints.end());
 
        // prepopulate with client hints
        hints.insert(mClientHints.begin(), mClientHints.end());
 


@@ -565,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))));


@@ -591,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)
                        {


@@ -605,66 +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;
 }
 

-//
-// Perform code signature ACL access adjustment dialogs
-//
-bool QueryCodeCheck::operator () (const char *aclPath)
-{
-       OSStatus status;
-       AuthValueVector arguments;
-       AuthItemSet hints, context;
-
-#if defined(NOSA)
-       if (getenv("NOSA")) {
-               char answer[10];
-
-        string applicationPath;
-        AuthItem *applicationPathItem = mClientHints.find(AGENT_HINT_APPLICATION_PATH);
-               if (applicationPathItem)
-                 applicationPathItem->getString(applicationPath);
-
-               getNoSA(answer, sizeof(answer),
-                               "Allow %s to match an ACL for %s [yn][g]? ",
-                               applicationPath.c_str(), aclPath ? aclPath : "(unknown)");
-               allow = answer[0] == 'y';
-               remember = answer[1] == 'g';
-               return;
-       }
-#endif
-
-       // prepopulate with client hints
-       hints.insert(mClientHints.begin(), mClientHints.end());
-
-       hints.insert(AuthItemRef(AGENT_HINT_APPLICATION_PATH, AuthValueOverlay((uint32_t)strlen(aclPath), const_cast<char*>(aclPath))));
-
-       create("builtin", "code-identity");
-
-    setInput(hints, context);
-       status = invoke();
-
-    checkResult();
-
-//     MacOSError::check(status);
-
-    return kAuthorizationResultAllow == mLastResult;
-}
-
-

 //
 // 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


@@ -673,22 +602,10 @@ bool QueryCodeCheck::operator () (const char *aclPath)
 Reason QueryOld::query()
 {
        Reason reason = SecurityAgent::noReason;
 Reason QueryOld::query()
 {
        Reason reason = SecurityAgent::noReason;

-       OSStatus status;
-       AuthValueVector arguments;

        AuthItemSet hints, context;
        CssmAutoData passphrase(Allocator::standard(Allocator::sensitive));
        int retryCount = 0;
 
        AuthItemSet hints, context;
        CssmAutoData passphrase(Allocator::standard(Allocator::sensitive));
        int retryCount = 0;
 

-#if defined(NOSA)
-    // return the passphrase
-       if (getenv("NOSA")) {
-        char passphrase_[maxPassphraseLength];
-               getNoSA(passphrase, maxPassphraseLength, "Unlock %s [<CR> to cancel]: ", database.dbName());
-       passphrase.copy(passphrase_, strlen(passphrase_));
-       return database.decode(passphrase) ? SecurityAgent::noReason : SecurityAgent::invalidPassphrase;
-       }
-#endif
-

        // prepopulate with client hints
 
     const char *keychainPath = database.dbName();
        // prepopulate with client hints
 
     const char *keychainPath = database.dbName();


@@ -714,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)
         {


@@ -750,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


@@ -779,8 +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;
-       AuthValueVector arguments;

        AuthItemSet hints, context;
        CssmAutoData passphrase(Allocator::standard(Allocator::sensitive));
        int retryCount = 0;
        AuthItemSet hints, context;
        CssmAutoData passphrase(Allocator::standard(Allocator::sensitive));
        int retryCount = 0;


@@ -811,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();
 


@@ -842,8 +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;
-       AuthValueVector arguments;

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


@@ -877,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();
 


@@ -934,22 +852,10 @@ 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;
-       AuthValueVector arguments;

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

-#if defined(NOSA)
-       if (getenv("NOSA")) {
-        char passphrase_[maxPassphraseLength];
-               getNoSA(passphrase_, maxPassphraseLength,
-                       "New passphrase for %s (reason %d) [<CR> to cancel]: ",
-                       database.dbName(), reason);
-               return SecurityAgent::noReason;
-       }
-#endif
-

        // prepopulate with client hints
        hints.insert(mClientHints.begin(), mClientHints.end());
 
        // prepopulate with client hints
        hints.insert(mClientHints.begin(), mClientHints.end());
 


@@ -982,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)
                {


@@ -1062,17 +968,8 @@ 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?
-    AuthValueVector arguments;

     AuthItemSet hints, context;
 
     AuthItemSet hints, context;
 

-#if defined(NOSA)
-    if (getenv("NOSA")) {
-               // FIXME  3690984
-               return SecurityAgent::noReason;
-    }
-#endif
-

     hints.insert(mClientHints.begin(), mClientHints.end());
     hints.insert(AuthItemRef(AGENT_HINT_CUSTOM_PROMPT, AuthValueOverlay(prompt ? (UInt32)prompt->length() : 0, prompt ? prompt->data() : NULL)));
     // XXX/gh  defined by dmitch but no analogous hint in
     hints.insert(mClientHints.begin(), mClientHints.end());
     hints.insert(AuthItemRef(AGENT_HINT_CUSTOM_PROMPT, AuthValueOverlay(prompt ? (UInt32)prompt->length() : 0, prompt ? prompt->data() : NULL)));
     // XXX/gh  defined by dmitch but no analogous hint in


@@ -1082,8 +979,6 @@ Reason QueryGenericPassphrase::query(const CssmData *prompt, bool verify,
     if (false == verify) {  // import
                create("builtin", "generic-unlock");
     } else {           // verify passphrase (export)
     if (false == verify) {  // import
                create("builtin", "generic-unlock");
     } else {           // verify passphrase (export)

-                                       // new-passphrase-generic works with the pre-4 June 2004 agent;
-                                       // generic-new-passphrase is required for the new agent

                create("builtin", "generic-new-passphrase");
     }
 
                create("builtin", "generic-new-passphrase");
     }
 


@@ -1091,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);
 


@@ -1115,17 +1010,8 @@ 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?
-    AuthValueVector arguments;

     AuthItemSet hints/*NUKEME*/, context;
 
     AuthItemSet hints/*NUKEME*/, context;
 

-#if defined(NOSA)
-    if (getenv("NOSA")) {
-               // FIXME  akin to 3690984
-               return SecurityAgent::noReason;
-    }
-#endif
-

        hints.insert(mClientHints.begin(), mClientHints.end());
        create("builtin", "generic-unlock-kcblob");
 
        hints.insert(mClientHints.begin(), mClientHints.end());
        create("builtin", "generic-unlock-kcblob");
 


@@ -1146,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)


@@ -1169,7 +1055,7 @@ Reason QueryDBBlobSecret::accept(CssmManagedData &passphrase,
                try
                {
                        RefPointer<KeychainDatabase> dbToUnlock = Server::keychain(*currHdl);
                try
                {
                        RefPointer<KeychainDatabase> dbToUnlock = Server::keychain(*currHdl);

-                       dbToUnlock->unlockDb(passphrase);
+                       dbToUnlock->unlockDb(passphrase, false);

                        authenticated = true;
                        *dbHandleAuthenticated = *currHdl; // return the DbHandle that 'passphrase' authenticated with.
                }
                        authenticated = true;
                        *dbHandleAuthenticated = *currHdl; // return the DbHandle that 'passphrase' authenticated with.
                }


@@ -1186,43 +1072,16 @@ 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;

-       AuthValueVector arguments;

        int retryCount = 0;
        string username;
        string password;
 
     using CommonCriteria::Securityd::KeychainAuthLogger;
        int retryCount = 0;
        string username;
        string password;
 
     using CommonCriteria::Securityd::KeychainAuthLogger;

-    KeychainAuthLogger logger(mAuditToken, AUE_ssauthint, database, description);
-
-#if defined(NOSA)
-    /* XXX/gh  probably not complete; stolen verbatim from rogue-app query */
-    if (getenv("NOSA")) {
-               char answer[maxPassphraseLength+10];
-
-        string applicationPath;
-        AuthItem *applicationPathItem = mClientHints.find(AGENT_HINT_APPLICATION_PATH);
-               if (applicationPathItem)
-                 applicationPathItem->getString(applicationPath);
-
-               getNoSA(answer, sizeof(answer), "Allow %s to do %d on %s in %s? [yn][g]%s ",
-                       applicationPath.c_str(), int(action), (description ? description : "[NULL item]"),
-                       (database ? database : "[NULL database]"),
-                       mPassphraseCheck ? ":passphrase" : "");
-               // turn passphrase (no ':') into y:passphrase
-               if (mPassphraseCheck && !strchr(answer, ':')) {
-                       memmove(answer+2, answer, strlen(answer)+1);
-                       memcpy(answer, "y:", 2);
-               }
-
-               allow = answer[0] == 'y';
-               remember = answer[1] == 'g';
-               return SecurityAgent::noReason;
-    }
-#endif
+    KeychainAuthLogger logger(mAuditToken, (short)AUE_ssauthint, db.dbName(), description);

 
     hints.insert(mClientHints.begin(), mClientHints.end());
 
 
     hints.insert(mClientHints.begin(), mClientHints.end());
 


@@ -1235,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
 


@@ -1279,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();