Add an entitlement check to only allow AdAttributionDaemon to be connected to by the network process
Created attachment 440257 [details] Patch
Comment on attachment 440257 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=440257&action=review > Source/WebKit/Shared/EntryPointUtilities/Cocoa/Daemon/PCMDaemonEntryPoint.mm:84 > + NSLog(@"connection attempted connection without required entitlement"); Does this look right in the log starting with lower case? > Tools/TestWebKitAPI/Tests/WebKitCocoa/EventAttribution.mm:513 > + xpc_connection_set_event_handler(connection.get(), ^(xpc_object_t event) { > + if (event == XPC_ERROR_CONNECTION_INTERRUPTED) { > + done = true; > + }); Not sure if this tests enough. Isn't the purpose to verify that the connection is never established? This just verifies that it's dropped, perhaps only eventually. Also, failing as timeout isn't always understood to be a legitimate failure in practice.
Comment on attachment 440257 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=440257&action=review >> Source/WebKit/Shared/EntryPointUtilities/Cocoa/Daemon/PCMDaemonEntryPoint.mm:84 >> + NSLog(@"connection attempted connection without required entitlement"); > > Does this look right in the log starting with lower case? I'll capitalize. > Tools/TestWebKitAPI/Tests/WebKitCocoa/EventAttribution.mm:483 > + system([NSString stringWithFormat:@"launchctl unload %@ 2> /dev/null", plistLocation.path].fileSystemRepresentation); I committed this separately in http://trac.webkit.org/r283574 >> Tools/TestWebKitAPI/Tests/WebKitCocoa/EventAttribution.mm:513 >> + }); > > Not sure if this tests enough. Isn't the purpose to verify that the connection is never established? This just verifies that it's dropped, perhaps only eventually. > > Also, failing as timeout isn't always understood to be a legitimate failure in practice. This is what it looks like at this API level when a connection is never established. I'll change this to make sure event is always equal to XPC_ERROR_CONNECTION_INTERRUPTED to make sure that nothing else happens. I agree that it's not great to have the failure mode be a timeout, but I think that's how this test needs to be.
Created attachment 440285 [details] Patch
Committed r283586 (242544@main): <https://commits.webkit.org/242544@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 440285 [details].
<rdar://problem/83910831>