Bug 160945 – Move WKSignedPublicKeyAndChallengeString into WebCore and make it proper C++

• Component: New Bugs
• Status: RESOLVED FIXED

[Anders Carlsson]

Move WKSignedPublicKeyAndChallengeString into WebCore and make it proper C++

[Anders Carlsson]

Created attachment 286309 [details]
Patch

[Anders Carlsson]

Created attachment 286312 [details]
Patch

[Anders Carlsson]

Created attachment 286316 [details]
Patch

Comment on attachment 286316 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=286316&action=review

> Source/WebCore/platform/mac/SSLKeyGeneratorMac.mm:72
> +    const CSSM_KEY *cssmPublicKey;

The * should go next to the type.

> Source/WebCore/platform/mac/SSLKeyGeneratorMac.mm:76
> +    const CSSM_ACCESS_CREDENTIALS credentials { };

We normally don’t const-qualify local, non-static variables.

> Source/WebCore/platform/mac/SSLKeyGeneratorMac.mm:104
> +    const CSSM_KEY *cssmPrivateKey;

Star on the wrong side.

> Source/WebCore/platform/mac/SSLKeyGeneratorMac.mm:108
> +    const CSSM_ACCESS_CREDENTIALS *credentials;

Ditto.

> Source/WebCore/platform/mac/SSLKeyGeneratorMac.mm:142
> +    // For security purposes, private keys imported by WebKit should not be accessible by 
> +    // all applications. (4126976) Passing nullptr to SecTrustedApplicationCreateFromPath tells
> +    // that function to assume the application bundle. Our access to the newly-created private
> +    // key is necessary to sign challange responses.

I don’t think the 4126976 reference is needed. I’d remove the first and last sentences of the comment.

> Source/WebCore/platform/mac/SSLKeyGeneratorMac.mm:179
> +    ASSERT(challenge.length());

Isn’t this assertion wrong? We have a call site below that can pass us the empty string as a challenge.

> Source/WebCore/platform/mac/SSLKeyGeneratorMac.mm:204
> +    signedPublicKeyAndChallenge.signature.Length *= 8;

Mystery 8.

> Source/WebCore/platform/mac/SSLKeyGeneratorMac.mm:206
> +    CSSM_DATA encodedSignedPublicKeyAndChallenge {0, nullptr };

Missing space after the {.

Comment on attachment 286316 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=286316&action=review

> Source/WTF/wtf/spi/cocoa/SecuritySPI.h:33
> +#include <Security/keyTemplates.h>

I think this isn’t present in the Apple-internal iOS SDK.

[Anders Carlsson]

Committed r204567: <http://trac.webkit.org/changeset/204567>