Title: [273875] trunk
Revision
273875
Author
commit-qu...@webkit.org
Date
2021-03-03 23:22:33 -0800 (Wed, 03 Mar 2021)

Log Message

WKRemoteObjectCoder should be able to handle NSErrors from TLS failures
https://bugs.webkit.org/show_bug.cgi?id=222401
Source/WebKit:

<rdar://problem/72103865>

Patch by Alex Christensen <achristen...@webkit.org> on 2021-03-03
Reviewed by Chris Dumez.

NSErrors from TLS failures contain values like a SecTrustRef or a SecCertificateRef,
which are not ObjC objects, and they don't like it when you call encodeWithCoder: on them.
Until r273141 it would crash when we do, but even after that we just decode a nil NSError.
Add a special case like we did in encodeNSError to successfully encode and decode these errors.

* Shared/API/Cocoa/WKRemoteObjectCoder.mm:
(decodeObjCObject):
(transformCertificatesToData):
(transformTrustToData):
(encodeError):
(transformDataToCertificates):
(transformDataToTrust):
(decodeError):
(encodeObject):
(decodeObject):

Tools:

Patch by Alex Christensen <achristen...@webkit.org> on 2021-03-03
Reviewed by Chris Dumez.

* TestWebKitAPI/Tests/WebKitCocoa/RemoteObjectRegistry.h:
* TestWebKitAPI/Tests/WebKitCocoa/RemoteObjectRegistry.mm:
(TEST):
* TestWebKitAPI/Tests/WebKitCocoa/RemoteObjectRegistryPlugIn.mm:
(-[RemoteObjectRegistryPlugIn sendError:completionHandler:]):

Modified Paths

Diff

Modified: trunk/Source/WebKit/ChangeLog (273874 => 273875)


--- trunk/Source/WebKit/ChangeLog	2021-03-04 06:13:44 UTC (rev 273874)
+++ trunk/Source/WebKit/ChangeLog	2021-03-04 07:22:33 UTC (rev 273875)
@@ -1,5 +1,29 @@
 2021-03-03  Alex Christensen  <achristen...@webkit.org>
 
+        WKRemoteObjectCoder should be able to handle NSErrors from TLS failures
+        https://bugs.webkit.org/show_bug.cgi?id=222401
+        <rdar://problem/72103865>
+
+        Reviewed by Chris Dumez.
+
+        NSErrors from TLS failures contain values like a SecTrustRef or a SecCertificateRef,
+        which are not ObjC objects, and they don't like it when you call encodeWithCoder: on them.
+        Until r273141 it would crash when we do, but even after that we just decode a nil NSError.
+        Add a special case like we did in encodeNSError to successfully encode and decode these errors.
+
+        * Shared/API/Cocoa/WKRemoteObjectCoder.mm:
+        (decodeObjCObject):
+        (transformCertificatesToData):
+        (transformTrustToData):
+        (encodeError):
+        (transformDataToCertificates):
+        (transformDataToTrust):
+        (decodeError):
+        (encodeObject):
+        (decodeObject):
+
+2021-03-03  Alex Christensen  <achristen...@webkit.org>
+
         Limit HashTable entry size to 500 bytes
         https://bugs.webkit.org/show_bug.cgi?id=222658
 

Modified: trunk/Source/WebKit/Shared/API/Cocoa/WKRemoteObjectCoder.mm (273874 => 273875)


--- trunk/Source/WebKit/Shared/API/Cocoa/WKRemoteObjectCoder.mm	2021-03-04 06:13:44 UTC (rev 273874)
+++ trunk/Source/WebKit/Shared/API/Cocoa/WKRemoteObjectCoder.mm	2021-03-04 07:22:33 UTC (rev 273875)
@@ -33,6 +33,7 @@
 #import "APIString.h"
 #import "Logging.h"
 #import "NSInvocationSPI.h"
+#import "_WKErrorRecoveryAttempting.h"
 #import "_WKRemoteObjectInterfaceInternal.h"
 #import <objc/runtime.h>
 #import <wtf/RetainPtr.h>
@@ -299,6 +300,120 @@
     encoder->_currentDictionary->set(stringKey, API::String::create(string));
 }
 
+static RetainPtr<id> decodeObjCObject(WKRemoteObjectDecoder *decoder, Class objectClass)
+{
+    id allocation = [objectClass allocWithZone:decoder.zone];
+    if (!allocation)
+        [NSException raise:NSInvalidUnarchiveOperationException format:@"Class \"%@\" returned nil from +alloc while being decoded", NSStringFromClass(objectClass)];
+
+    RetainPtr<id> result = adoptNS([allocation initWithCoder:decoder]);
+    if (!result)
+        [NSException raise:NSInvalidUnarchiveOperationException format:@"Object of class \"%@\" returned nil from -initWithCoder: while being decoded", NSStringFromClass(objectClass)];
+
+    result = [result awakeAfterUsingCoder:decoder];
+    if (!result)
+        [NSException raise:NSInvalidUnarchiveOperationException format:@"Object of class \"%@\" returned nil from -awakeAfterUsingCoder: while being decoded", NSStringFromClass(objectClass)];
+
+    return result;
+}
+
+static constexpr NSString *peerCertificateKey = @"NSErrorPeerCertificateChainKey";
+static constexpr NSString *peerTrustKey = @"NSURLErrorFailingURLPeerTrustErrorKey";
+static constexpr NSString *clientCertificateKey = @"NSErrorClientCertificateChainKey";
+
+static RetainPtr<NSArray<NSData *>> transformCertificatesToData(NSArray *input)
+{
+    auto dataArray = adoptNS([[NSMutableArray alloc] initWithCapacity:input.count]);
+    for (id certificate in input) {
+        if (CFGetTypeID(certificate) != SecCertificateGetTypeID())
+            [NSException raise:NSInvalidArgumentException format:@"Error encoding invalid certificate in chain"];
+        [dataArray addObject:(NSData *)adoptCF(SecCertificateCopyData((SecCertificateRef)certificate)).get()];
+    }
+    return dataArray;
+}
+
+static RetainPtr<NSData> transformTrustToData(SecTrustRef trust)
+{
+    if (CFGetTypeID(trust) != SecTrustGetTypeID())
+        [NSException raise:NSInvalidArgumentException format:@"Error encoding invalid SecTrustRef"];
+    CFErrorRef error = nullptr;
+    auto data = "" &error));
+    if (error)
+        [NSException raise:NSInvalidArgumentException format:@"Error serializing SecTrustRef: %@", error];
+    return data;
+}
+
+static void encodeError(WKRemoteObjectEncoder *encoder, NSError *error)
+{
+    RetainPtr<NSMutableDictionary> copy;
+    if (error.userInfo[_WKRecoveryAttempterErrorKey]) {
+        copy = adoptNS([error.userInfo mutableCopy]);
+        [copy removeObjectForKey:_WKRecoveryAttempterErrorKey];
+    }
+    if (error.userInfo[clientCertificateKey]) {
+        if (!copy)
+            copy = adoptNS([error.userInfo mutableCopy]);
+        [copy removeObjectForKey:clientCertificateKey];
+    }
+    if (NSArray *certificateChain = error.userInfo[peerCertificateKey]) {
+        if (!copy)
+            copy = adoptNS([error.userInfo mutableCopy]);
+        copy.get()[peerCertificateKey] = transformCertificatesToData(certificateChain).get();
+    }
+    if (id trust = error.userInfo[peerTrustKey]) {
+        if (!copy)
+            copy = adoptNS([error.userInfo mutableCopy]);
+        copy.get()[peerTrustKey] = transformTrustToData((SecTrustRef)trust).get();
+    }
+    if (!copy)
+        [error encodeWithCoder:encoder];
+    else
+        [[NSError errorWithDomain:error.domain code:error.code userInfo:copy.get()] encodeWithCoder:encoder];
+}
+
+static RetainPtr<NSArray> transformDataToCertificates(NSArray *input)
+{
+    auto array = adoptNS([[NSMutableArray alloc] initWithCapacity:input.count]);
+    for (NSData *data in input) {
+        if (CFGetTypeID(data) != CFDataGetTypeID())
+            [NSException raise:NSInvalidUnarchiveOperationException format:@"Error decoding certificate from object that is not data %@", NSStringFromClass([data class])];
+        auto certificate = adoptCF(SecCertificateCreateWithData(nullptr, (CFDataRef)data));
+        if (!certificate)
+            [NSException raise:NSInvalidUnarchiveOperationException format:@"Error decoding nvalid certificate in chain"];
+        [array addObject:(id)certificate.get()];
+    }
+    return array;
+}
+
+static RetainPtr<id> transformDataToTrust(NSData *data)
+{
+    if (CFGetTypeID(data) != CFDataGetTypeID())
+        [NSException raise:NSInvalidUnarchiveOperationException format:@"Invalid SecTrustRef data %@", NSStringFromClass([data class])];
+    CFErrorRef error = nullptr;
+    auto trust = adoptCF(SecTrustDeserialize((CFDataRef)data, &error));
+    if (error || !trust)
+        [NSException raise:NSInvalidUnarchiveOperationException format:@"Invalid SecTrustRef %@", error];
+    return trust;
+}
+
+static RetainPtr<NSError> decodeError(WKRemoteObjectDecoder *decoder)
+{
+    RetainPtr<NSError> error = decodeObjCObject(decoder, [NSError class]);
+    RetainPtr<NSMutableDictionary> copy;
+    if (NSArray *certificateChain = error.get().userInfo[peerCertificateKey]) {
+        copy = adoptNS([error.get().userInfo mutableCopy]);
+        copy.get()[peerCertificateKey] = transformDataToCertificates(certificateChain).get();
+    }
+    if (NSData *trust = error.get().userInfo[peerTrustKey]) {
+        if (!copy)
+            copy = adoptNS([error.get().userInfo mutableCopy]);
+        copy.get()[peerTrustKey] = transformDataToTrust(trust).get();
+    }
+    if (!copy)
+        return error;
+    return [NSError errorWithDomain:error.get().domain code:error.get().code userInfo:copy.get()];
+}
+
 static void encodeObject(WKRemoteObjectEncoder *encoder, id object)
 {
     ASSERT(object);
@@ -337,6 +452,9 @@
         return;
     }
 
+    if (objectClass == [NSError class])
+        return encodeError(encoder, object);
+    
     [object encodeWithCoder:encoder];
 }
 
@@ -873,23 +991,14 @@
 
     if (objectClass == [NSString class])
         return decodeString(decoder);
+    
+    if (objectClass == [NSError class])
+        return decodeError(decoder).autorelease();
 
     if (objectClass == [NSMutableString class])
         return [NSMutableString stringWithString:decodeString(decoder)];
 
-    id result = [objectClass allocWithZone:decoder.zone];
-    if (!result)
-        [NSException raise:NSInvalidUnarchiveOperationException format:@"Class \"%s\" returned nil from +alloc while being decoded", className.data()];
-
-    result = [result initWithCoder:decoder];
-    if (!result)
-        [NSException raise:NSInvalidUnarchiveOperationException format:@"Object of class \"%s\" returned nil from -initWithCoder: while being decoded", className.data()];
-
-    result = [result awakeAfterUsingCoder:decoder];
-    if (!result)
-        [NSException raise:NSInvalidUnarchiveOperationException format:@"Object of class \"%s\" returned nil from -awakeAfterUsingCoder: while being decoded", className.data()];
-
-    return [result autorelease];
+    return decodeObjCObject(decoder, objectClass).autorelease();
 }
 
 static id decodeObject(WKRemoteObjectDecoder *decoder, const API::Dictionary* dictionary, const HashSet<CFTypeRef>& allowedClasses)

Modified: trunk/Tools/ChangeLog (273874 => 273875)


--- trunk/Tools/ChangeLog	2021-03-04 06:13:44 UTC (rev 273874)
+++ trunk/Tools/ChangeLog	2021-03-04 07:22:33 UTC (rev 273875)
@@ -1,3 +1,16 @@
+2021-03-03  Alex Christensen  <achristen...@webkit.org>
+
+        WKRemoteObjectCoder should be able to handle NSErrors from TLS failures
+        https://bugs.webkit.org/show_bug.cgi?id=222401
+
+        Reviewed by Chris Dumez.
+
+        * TestWebKitAPI/Tests/WebKitCocoa/RemoteObjectRegistry.h:
+        * TestWebKitAPI/Tests/WebKitCocoa/RemoteObjectRegistry.mm:
+        (TEST):
+        * TestWebKitAPI/Tests/WebKitCocoa/RemoteObjectRegistryPlugIn.mm:
+        (-[RemoteObjectRegistryPlugIn sendError:completionHandler:]):
+
 2021-03-03  Chris Dumez  <cdu...@apple.com>
 
         Unreviewed, reverting r273851.

Modified: trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/RemoteObjectRegistry.h (273874 => 273875)


--- trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/RemoteObjectRegistry.h	2021-03-04 06:13:44 UTC (rev 273874)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/RemoteObjectRegistry.h	2021-03-04 07:22:33 UTC (rev 273875)
@@ -52,6 +52,7 @@
 - (void)doNotCallCompletionHandler:(void (^)())completionHandler;
 - (void)sendRequest:(NSURLRequest *)request response:(NSURLResponse *)response challenge:(NSURLAuthenticationChallenge *)challenge error:(NSError *)error completionHandler:(void (^)(NSURLRequest *, NSURLResponse *, NSURLAuthenticationChallenge *, NSError *))completionHandler;
 - (void)callUIProcessMethodWithReplyBlock;
+- (void)sendError:(NSError *)error completionHandler:(void (^)(NSError *))completionHandler;
 
 @end
 

Modified: trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/RemoteObjectRegistry.mm (273874 => 273875)


--- trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/RemoteObjectRegistry.mm	2021-03-04 06:13:44 UTC (rev 273874)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/RemoteObjectRegistry.mm	2021-03-04 07:22:33 UTC (rev 273875)
@@ -25,9 +25,11 @@
 
 #import "config.h"
 
+#import "HTTPServer.h"
 #import "PlatformUtilities.h"
 #import "RemoteObjectRegistry.h"
 #import "Test.h"
+#import "TestNavigationDelegate.h"
 #import "WKWebViewConfigurationExtras.h"
 #import <WebKit/WKProcessPoolPrivate.h>
 #import <WebKit/WKWebViewPrivate.h>
@@ -38,7 +40,7 @@
 #import <wtf/RetainPtr.h>
 #import <wtf/WeakObjCPtr.h>
 
-TEST(WebKit, RemoteObjectRegistry)
+TEST(RemoteObjectRegistry, Basic)
 {
     __block bool isDone = false;
 
@@ -196,7 +198,7 @@
 
 @end
 
-TEST(WebKit, RemoteObjectRegistry_CallReplyBlockAfterOriginatingWebViewDeallocates)
+TEST(RemoteObjectRegistry, CallReplyBlockAfterOriginatingWebViewDeallocates)
 {
     auto localObject = adoptNS([[LocalObject alloc] init]);
     WeakObjCPtr<WKWebView> weakWebViewPtr;
@@ -228,3 +230,28 @@
 
     localObject->completionHandlerFromWebProcess();
 }
+
+TEST(RemoteObjectRegistry, SerializeErrorWithCertificates)
+{
+    TestWebKitAPI::HTTPServer server({ }, TestWebKitAPI::HTTPServer::Protocol::Https);
+    auto webView = adoptNS([[WKWebView alloc] initWithFrame:CGRectZero configuration:[WKWebViewConfiguration _test_configurationWithTestPlugInClassName:@"RemoteObjectRegistryPlugIn"]]);
+    auto delegate = adoptNS([TestNavigationDelegate new]);
+    webView.get().navigationDelegate = delegate.get();
+    [webView loadRequest:server.request()];
+    NSError *error = [delegate waitForDidFailProvisionalNavigation];
+    NSString *key = @"NSErrorPeerCertificateChainKey";
+    EXPECT_WK_STREQ(error.domain, "NSURLErrorDomain");
+    EXPECT_TRUE(error.userInfo[key]);
+    
+    _WKRemoteObjectInterface *interface = [_WKRemoteObjectInterface remoteObjectInterfaceWithProtocol:@protocol(RemoteObjectProtocol)];
+    id <RemoteObjectProtocol> object = [[webView _remoteObjectRegistry] remoteObjectProxyWithInterface:interface];
+    __block bool roundTripComplete = false;
+    [object sendError:error completionHandler:^(NSError *deserializedError) {
+        EXPECT_WK_STREQ(deserializedError.domain, "NSURLErrorDomain");
+        NSArray *chain = deserializedError.userInfo[key];
+        EXPECT_TRUE(chain);
+        EXPECT_EQ(CFGetTypeID(chain[0]), SecCertificateGetTypeID());
+        roundTripComplete = true;
+    }];
+    TestWebKitAPI::Util::run(&roundTripComplete);
+}

Modified: trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/RemoteObjectRegistryPlugIn.mm (273874 => 273875)


--- trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/RemoteObjectRegistryPlugIn.mm	2021-03-04 06:13:44 UTC (rev 273874)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/RemoteObjectRegistryPlugIn.mm	2021-03-04 07:22:33 UTC (rev 273875)
@@ -118,6 +118,11 @@
     completionHandler(request, response, challenge, error);
 }
 
+- (void)sendError:(NSError *)error completionHandler:(void (^)(NSError *))completionHandler
+{
+    completionHandler(error);
+}
+
 - (void)callUIProcessMethodWithReplyBlock
 {
     id <LocalObjectProtocol> localObject = [[_browserContextController _remoteObjectRegistry] remoteObjectProxyWithInterface:localObjectInterface()];
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to