Bug 133863 - [iOS] Networking process always decodes keys
Summary: [iOS] Networking process always decodes keys
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: mitz
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-06-13 10:10 PDT by mitz
Modified: 2014-06-13 11:05 PDT (History)
2 users (show)

See Also:


Attachments
Disallow decoding keys by default (5.17 KB, patch)
2014-06-13 10:13 PDT, mitz
andersca: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description mitz 2014-06-13 10:10:47 PDT
[iOS] Networking process always decodes keys
Comment 1 mitz 2014-06-13 10:13:15 PDT
Created attachment 233058 [details]
Disallow decoding keys by default
Comment 2 Anders Carlsson 2014-06-13 10:33:03 PDT
Comment on attachment 233058 [details]
Disallow decoding keys by default

I think "decoding keys" is too vague. How about decoding keychain keys or keychain items?
Comment 3 mitz 2014-06-13 10:49:48 PDT
(In reply to comment #2)
> (From update of attachment 233058 [details])
> I think "decoding keys" is too vague. How about decoding keychain keys or keychain items?

I’m going to change this to setAllowsDecodingSecKeyRef etc.
Comment 4 Anders Carlsson 2014-06-13 10:51:57 PDT
Comment on attachment 233058 [details]
Disallow decoding keys by default

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

r=me with the naming change we discussed.

> Source/WebKit2/Shared/cf/ArgumentCodersCF.cpp:649
> +    if (keyDecodingAllowed)
> +        SecKeyFindWithPersistentRef(keyData.get(), &key);

Will this do the right thing if key decoding is disallowed? Shouldn't it just return false in that case?
Comment 5 mitz 2014-06-13 10:54:37 PDT
(In reply to comment #4)
> (From update of attachment 233058 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=233058&action=review
> 
> r=me with the naming change we discussed.
> 
> > Source/WebKit2/Shared/cf/ArgumentCodersCF.cpp:649
> > +    if (keyDecodingAllowed)
> > +        SecKeyFindWithPersistentRef(keyData.get(), &key);
> 
> Will this do the right thing if key decoding is disallowed? Shouldn't it just return false in that case?

Leaving key set to nullptr will follow the code path we already take when we don’t have access to the key for any other reason (such as, on Mac, the user denying access through the Security dialog, or on both platforms the case where the key has been deleted between when it was sent and when it was received).
Comment 6 mitz 2014-06-13 11:05:14 PDT
Fixed in <http://trac.webkit.org/r169938>.