From a6f82f7514b02546268d3b5460fabf320e57ed3b Mon Sep 17 00:00:00 2001 From: Apple Date: Mon, 17 Jun 2019 20:49:46 +0000 Subject: [PATCH] Security-58286.240.4.tar.gz --- CircleJoinRequested/CircleJoinRequested.m | 39 +++++------ OSX/sec/securityd/OTATrustUtilities.m | 27 ++++++++ OSX/sec/securityd/SecTrustLoggingServer.h | 2 + OSX/trustd/trustd.c | 2 + keychain/ckks/CKKSAPSReceiver.m | 26 +++++--- keychain/ckks/tests/CKKSAPSHandlingTests.m | 75 +++++++++++++++++----- keychain/ckks/tests/CKKSAPSReceiverTests.m | 4 +- 7 files changed, 130 insertions(+), 45 deletions(-) diff --git a/CircleJoinRequested/CircleJoinRequested.m b/CircleJoinRequested/CircleJoinRequested.m index 3236a2bb..4f6ed2e2 100644 --- a/CircleJoinRequested/CircleJoinRequested.m +++ b/CircleJoinRequested/CircleJoinRequested.m @@ -753,7 +753,7 @@ static bool processEvents() CFErrorRef error = NULL; CFErrorRef departError = NULL; - SOSCCStatus circleStatus = SOSCCThisDeviceIsInCircleNonCached(&error); + SOSCCStatus circleStatus = SOSCCThisDeviceIsInCircle(&error); enum DepartureReason departureReason = SOSCCGetLastDepartureReason(&departError); // Error due to XPC failure does not provide information about the circle. @@ -782,28 +782,29 @@ static bool processEvents() } PSKeychainSyncIsUsingICDP(); - - // Refresh because sometimes we're fixed elsewhere before we get here. - CFReleaseNull(error); - circleStatus = SOSCCThisDeviceIsInCircleNonCached(&error); - + if(_isAccountICDP){ if((circleStatus == kSOSCCError || circleStatus == kSOSCCCircleAbsent || circleStatus == kSOSCCNotInCircle) && _hasPostedFollowupAndStillInError == false) { - if(circleStatus == kSOSCCError) { - secnotice("cjr", "error from SOSCCThisDeviceIsInCircle: %@", error); - } + secnotice("cjr", "error from SOSCCThisDeviceIsInCircle: %@", error); secnotice("cjr", "iCDP: We need to get back into the circle"); doOnceInMain(^{ - NSError *localError = nil; - CDPFollowUpController *cdpd = [[CDPFollowUpController alloc] init]; - CDPFollowUpContext *context = [CDPFollowUpContext contextForStateRepair]; - [cdpd postFollowUpWithContext:context error:&localError ]; - if(localError){ - secnotice("cjr", "request to CoreCDP to follow up failed: %@", localError); + if(_isAccountICDP){ + NSError *localError = nil; + CDPFollowUpController *cdpd = [[CDPFollowUpController alloc] init]; + CDPFollowUpContext *context = [CDPFollowUpContext contextForStateRepair]; + [cdpd postFollowUpWithContext:context error:&localError ]; + if(localError){ + secnotice("cjr", "request to CoreCDP to follow up failed: %@", localError); + } + else{ + secnotice("cjr", "CoreCDP handling follow up"); + _hasPostedFollowupAndStillInError = true; + } } else{ - secnotice("cjr", "CoreCDP handling follow up"); - _hasPostedFollowupAndStillInError = true; + postKickedOutAlert(kSOSPasswordChanged); + state.lastCircleStatus = kSOSCCError; + [state writeToStorage]; } }); state.lastCircleStatus = circleStatus; @@ -820,7 +821,9 @@ static bool processEvents() _executeProcessEventsOnce = true; return false; } - } else if(circleStatus == kSOSCCError && state.lastCircleStatus != kSOSCCError && (departureReason == kSOSNeverLeftCircle)) { + } + else if(circleStatus == kSOSCCError && state.lastCircleStatus != kSOSCCError && (departureReason == kSOSNeverLeftCircle) + && !_isAccountICDP) { secnotice("cjr", "SA: error from SOSCCThisDeviceIsInCircle: %@", error); CFIndex errorCode = CFErrorGetCode(error); if(errorCode == kSOSErrorPublicKeyAbsent){ diff --git a/OSX/sec/securityd/OTATrustUtilities.m b/OSX/sec/securityd/OTATrustUtilities.m index 4582e3da..774536a5 100644 --- a/OSX/sec/securityd/OTATrustUtilities.m +++ b/OSX/sec/securityd/OTATrustUtilities.m @@ -65,6 +65,7 @@ #if TARGET_OS_IPHONE && !TARGET_OS_SIMULATOR #import +#include #endif #if TARGET_OS_OSX @@ -437,6 +438,27 @@ static BOOL DeleteAssetFromDisk(void) { return NO; } +#if TARGET_OS_IPHONE && !TARGET_OS_SIMULATOR +static bool ChangeFileProtectionToClassD(NSURL *fileURL, NSError **error) { + BOOL result = YES; + int file_fd = open([fileURL fileSystemRepresentation], O_RDONLY); + if (file_fd) { + int retval = fcntl(file_fd, F_SETPROTECTIONCLASS, PROTECTION_CLASS_D); + if (retval < 0) { + MakeOTATrustError(error, OTATrustLogLevelError, NSPOSIXErrorDomain, errno, + @"set proteciton class error for asset %d: %s", errno, strerror(errno)); + result = NO; + } + close(file_fd); + } else { + MakeOTATrustError(error, OTATrustLogLevelError, NSPOSIXErrorDomain, errno, + @"open error for asset %d: %s", errno, strerror(errno)); + result = NO; + } + return result; +} +#endif + static BOOL CopyFileToDisk(NSString *filename, NSURL *localURL, NSError **error) { if (SecOTAPKIIsSystemTrustd()) { NSURL *toFileURL = GetAssetFileURL(filename); @@ -450,7 +472,12 @@ static BOOL CopyFileToDisk(NSString *filename, NSURL *localURL, NSError **error) @"copyfile error for asset %d: %s", errno, strerror(errno)); return NO; } else { + /* make sure we can read this file before first unlock */ +#if TARGET_OS_IPHONE && !TARGET_OS_SIMULATOR + return ChangeFileProtectionToClassD(toFileURL, error); +#else return YES; +#endif } } return NO; diff --git a/OSX/sec/securityd/SecTrustLoggingServer.h b/OSX/sec/securityd/SecTrustLoggingServer.h index 2087be49..5b15b9a4 100644 --- a/OSX/sec/securityd/SecTrustLoggingServer.h +++ b/OSX/sec/securityd/SecTrustLoggingServer.h @@ -28,4 +28,6 @@ #define _SECURITY_SECTRUSTLOGGINGSERVER_H_ +void DisableLocalization(void); + #endif /* _SECURITY_SECTRUSTLOGGINGSERVER_H_ */ diff --git a/OSX/trustd/trustd.c b/OSX/trustd/trustd.c index 0ccac927..8e56e848 100644 --- a/OSX/trustd/trustd.c +++ b/OSX/trustd/trustd.c @@ -715,6 +715,8 @@ static void trustd_sandbox(void) { int main(int argc, char *argv[]) { + DisableLocalization(); + char *wait4debugger = getenv("WAIT4DEBUGGER"); if (wait4debugger && !strcasecmp("YES", wait4debugger)) { seccritical("SIGSTOPing self, awaiting debugger"); diff --git a/keychain/ckks/CKKSAPSReceiver.m b/keychain/ckks/CKKSAPSReceiver.m index 1d62a535..76833561 100644 --- a/keychain/ckks/CKKSAPSReceiver.m +++ b/keychain/ckks/CKKSAPSReceiver.m @@ -56,8 +56,8 @@ @interface CKKSAPSReceiver() -// If we receive >0 notifications for a CKRecordZoneID that hasn't been registered yet, give them a fake update when they register -@property NSMutableSet* undeliveredUpdates; +// If we receive notifications for a CKRecordZoneID that hasn't been registered yet, send them a their updates when they register +@property NSMutableDictionary*>* undeliveredUpdates; @end @implementation CKKSAPSReceiver @@ -104,7 +104,7 @@ _apsConnectionClass = apsConnectionClass; _apsConnection = NULL; - _undeliveredUpdates = [[NSMutableSet alloc] init]; + _undeliveredUpdates = [NSMutableDictionary dictionary]; // APS might be slow. This doesn't need to happen immediately, so let it happen later. __weak __typeof(self) weakSelf = self; @@ -138,12 +138,14 @@ } [self.zoneMap setObject:receiver forKey: zoneID]; - if([strongSelf.undeliveredUpdates containsObject:zoneID]) { - [strongSelf.undeliveredUpdates removeObject:zoneID]; - // Now, send the receiver its fake notification! - secerror("ckks: sending fake push to newly-registered zone(%@): %@", zoneID, receiver); - [receiver notifyZoneChange:nil]; + NSMutableSet* currentPendingMessages = self.undeliveredUpdates[zoneID]; + [self.undeliveredUpdates removeObjectForKey:zoneID]; + + for(CKRecordZoneNotification* message in currentPendingMessages.allObjects) { + // Now, send the receiver its notification! + secerror("ckks: sending stored push(%@) to newly-registered zone(%@): %@", message, zoneID, receiver); + [receiver notifyZoneChange:message]; } [finished fulfill]; @@ -186,8 +188,12 @@ } else { secerror("ckks: received push for unregistered zone: %@", rznotification); if(rznotification.recordZoneID) { - // TODO: save the rznofication itself - [self.undeliveredUpdates addObject: rznotification.recordZoneID]; + NSMutableSet* currentPendingMessages = self.undeliveredUpdates[rznotification.recordZoneID]; + if(currentPendingMessages) { + [currentPendingMessages addObject:rznotification]; + } else { + self.undeliveredUpdates[rznotification.recordZoneID] = [NSMutableSet setWithObject:rznotification]; + } } } } else { diff --git a/keychain/ckks/tests/CKKSAPSHandlingTests.m b/keychain/ckks/tests/CKKSAPSHandlingTests.m index 9245595f..bcdedb93 100644 --- a/keychain/ckks/tests/CKKSAPSHandlingTests.m +++ b/keychain/ckks/tests/CKKSAPSHandlingTests.m @@ -87,6 +87,19 @@ [super tearDown]; } ++ (APSIncomingMessage*)messageWithTracingEnabledForZoneID:(CKRecordZoneID*)zoneID { + APSIncomingMessage* apsMessage = [CKKSAPSReceiverTests messageForZoneID:zoneID]; + NSUUID* nsuuid = [NSUUID UUID]; + uuid_t uuid = {0}; + [nsuuid getUUIDBytes:(unsigned char*)&uuid]; + NSData* uuidData = [NSData dataWithBytes:&uuid length:sizeof(uuid)]; + + apsMessage.tracingUUID = uuidData; + apsMessage.tracingEnabled = YES; + + return apsMessage; +} + - (void)testSendPushMetricUponRequest { for(CKRecordZoneID* zoneID in self.ckksZones) { [self putFakeKeyHierarchyInCloudKit:zoneID]; @@ -127,14 +140,7 @@ [manateeProcessTimeoutOp addSuccessDependency:manateeProcessOp]; [self.operationQueue addOperation:manateeProcessTimeoutOp]; - APSIncomingMessage* apsMessage = [CKKSAPSReceiverTests messageForZoneID:self.keychainZoneID]; - NSUUID* nsuuid = [NSUUID UUID]; - uuid_t uuid = {0}; - [nsuuid getUUIDBytes:(unsigned char*)&uuid]; - NSData* uuidData = [NSData dataWithBytes:&uuid length:sizeof(uuid)]; - - apsMessage.tracingUUID = uuidData; - apsMessage.tracingEnabled = YES; + APSIncomingMessage* apsMessage = [CKKSAPSHandlingTests messageWithTracingEnabledForZoneID:self.keychainZoneID]; // Inject a message at the APS layer // Because we can only make APS receivers once iCloud tells us the push environment after sign-in, we can't use our normal injection strategy, and fell back on global state. @@ -197,13 +203,8 @@ [keychainProcessTimeoutOp addSuccessDependency:keychainProcessOp]; [self.operationQueue addOperation:keychainProcessTimeoutOp]; - APSIncomingMessage* apsMessage = [CKKSAPSReceiverTests messageForZoneID:self.keychainZoneID]; - NSUUID* nsuuid = [NSUUID UUID]; - uuid_t uuid = {0}; - [nsuuid getUUIDBytes:(unsigned char*)&uuid]; - NSData* uuidData = [NSData dataWithBytes:&uuid length:sizeof(uuid)]; - - apsMessage.tracingUUID = uuidData; + // Create a push that matchs all push tracing patterns except for the enabled flag + APSIncomingMessage* apsMessage = [CKKSAPSHandlingTests messageWithTracingEnabledForZoneID:self.keychainZoneID]; apsMessage.tracingEnabled = NO; // Inject a message at the APS layer @@ -231,6 +232,50 @@ [self findGenericPassword:@"keychain-view" expecting:errSecSuccess]; } +- (void)testSendPushMetricEvenIfPushArrivesEarly { + CKRecordZoneID* pushTestZone = [[CKRecordZoneID alloc] initWithZoneName:@"PushTestZone" ownerName:CKCurrentUserDefaultName]; + [self.ckksZones addObject:pushTestZone]; + self.zones[pushTestZone] = [[FakeCKZone alloc] initZone: pushTestZone]; + + for(CKRecordZoneID* zoneID in self.ckksZones) { + [self putFakeKeyHierarchyInCloudKit:zoneID]; + [self saveTLKMaterialToKeychain:zoneID]; + [self expectCKKSTLKSelfShareUpload:zoneID]; + } + + // The push wakes securityd, so it happens before pushTestZone is created locally + // Send 2, just to test our infrastructure + APSIncomingMessage* apsMessage = [CKKSAPSHandlingTests messageWithTracingEnabledForZoneID:pushTestZone]; + APSIncomingMessage* apsMessage2 = [CKKSAPSHandlingTests messageWithTracingEnabledForZoneID:pushTestZone]; + + // Inject a message at the APS layer + // Because we can only make APS receivers once iCloud tells us the push environment after sign-in, we can't use our normal injection strategy, and fell back on global state. + CKKSAPSReceiver* apsReceiver = [CKKSAPSReceiver receiverForEnvironment:self.apsEnvironment + namedDelegatePort:SecCKKSAPSNamedPort + apsConnectionClass:[FakeAPSConnection class]]; + XCTAssertNotNil(apsReceiver, "Should have gotten an APS receiver"); + + [apsReceiver connection:nil didReceiveIncomingMessage:apsMessage]; + [apsReceiver connection:nil didReceiveIncomingMessage:apsMessage2]; + + // Expect four metric pushes, two per push: one from receiving the push and one from after we finish the fetch + // AFAICT there's no way to introspect a metric object to ensure we did it right + OCMExpect([self.mockContainerExpectations submitEventMetric:[OCMArg any]]); + OCMExpect([self.mockContainerExpectations submitEventMetric:[OCMArg any]]); + OCMExpect([self.mockContainerExpectations submitEventMetric:[OCMArg any]]); + OCMExpect([self.mockContainerExpectations submitEventMetric:[OCMArg any]]); + + // Launch! + [self.injectedManager findOrCreateView:pushTestZone.zoneName]; + + [self startCKKSSubsystem]; + + for(CKKSKeychainView* view in self.ckksViews) { + XCTAssertEqual(0, [view.keyHierarchyConditions[SecCKKSZoneKeyStateReady] wait:20*NSEC_PER_SEC], "Key state should enter 'ready' for view %@", view); + } + OCMVerifyAllWithDelay(self.mockDatabase, 20); +} + @end #endif diff --git a/keychain/ckks/tests/CKKSAPSReceiverTests.m b/keychain/ckks/tests/CKKSAPSReceiverTests.m index 94731dd6..fd3555a5 100644 --- a/keychain/ckks/tests/CKKSAPSReceiverTests.m +++ b/keychain/ckks/tests/CKKSAPSReceiverTests.m @@ -166,7 +166,7 @@ [self waitForExpectationsWithTimeout:5.0 handler:nil]; } -- (void)testReceiveNullNotificationIfRegisteredAfterDelivery { +- (void)testReceiveNotificationIfRegisteredAfterDelivery { CKKSAPSReceiver* apsr = [[CKKSAPSReceiver alloc] initWithEnvironmentName:@"testenvironment" namedDelegatePort:SecCKKSAPSNamedPort apsConnectionClass:[FakeAPSConnection class]]; @@ -180,7 +180,7 @@ CKKSAPSNotificationReceiver* anr = [[CKKSAPSNotificationReceiver alloc] initWithExpectation:[self expectationWithDescription:@"receive notification"] block: ^(CKRecordZoneNotification* notification) { - XCTAssertNil(notification, "Should not have received a notification, since we weren't alive to receive it"); + XCTAssertNotNil(notification, "Should have received a (stored) notification"); }]; CKKSCondition* registered = [apsr registerReceiver:anr forZoneID:self.testZoneID]; -- 2.45.2