Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix SSL client certificate not found on macOS Sierra #1105

Closed
wants to merge 11 commits into from
Closed

fix SSL client certificate not found on macOS Sierra #1105

Changes from 3 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
dbc4b6a
fix SSL client certificate not found on MacOS Sierra
schweikert Nov 4, 2016
73c3ed5
match subject
schweikert Nov 4, 2016
bf227d9
another attempt at fixing certificate retrieval
schweikert Nov 4, 2016
fea9050
fixes after initial review
schweikert Nov 6, 2016
465f324
improve matching of SSL identity
schweikert Nov 6, 2016
57a216e
remove unneeded change
schweikert Nov 6, 2016
4dc08c2
code quality fixes, based on review
schweikert Nov 8, 2016
1db5dc6
fix #endif comment which references wrong #if
schweikert Nov 9, 2016
b6ecc9b
avoid inserting empty line
schweikert Nov 9, 2016
e854d58
minor fixes after review by @jay
schweikert Nov 9, 2016
a815fdd
revert removal of older API check at runtime
schweikert Nov 11, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
40 changes: 33 additions & 7 deletions lib/vtls/darwinssl.c
View file
Open in desktop
Expand Up @@ -882,15 +882,18 @@ static OSStatus CopyIdentityWithLabelOldSchool(char *label,
static OSStatus CopyIdentityWithLabel(char *label,
SecIdentityRef *out_cert_and_key)
{
CFArrayRef keys_list;
CFIndex keys_list_count;
CFIndex i;
OSStatus status = errSecItemNotFound;

#if CURL_BUILD_MAC_10_7 || CURL_BUILD_IOS
/* SecItemCopyMatching() was introduced in iOS and Snow Leopard.
kSecClassIdentity was introduced in Lion. If both exist, let's use them
to find the certificate. */
if(SecItemCopyMatching != NULL && kSecClassIdentity != NULL) {
CFTypeRef keys[4];
CFTypeRef values[4];
CFTypeRef keys[5];
CFTypeRef values[5];
CFDictionaryRef query_dict;
CFStringRef label_cf = CFStringCreateWithCString(NULL, label,
kCFStringEncodingUTF8);
Expand All @@ -900,21 +903,43 @@ static OSStatus CopyIdentityWithLabel(char *label,
keys[0] = kSecClass;
values[1] = kCFBooleanTrue; /* we want a reference */
keys[1] = kSecReturnRef;
values[2] = kSecMatchLimitOne; /* one is enough, thanks */
values[2] = kSecMatchLimitAll; /* one is enough, thanks */
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if an unlimited number of results may be returned then the comment "one is enough" is wrong isn't it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed...

keys[2] = kSecMatchLimit;
/* identity searches need a SecPolicyRef in order to work */
values[3] = SecPolicyCreateSSL(false, label_cf);
values[3] = SecPolicyCreateSSL(false, NULL);
keys[3] = kSecMatchPolicy;
/* match the name of the certificate (this doesn't seem to work :( ) */
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this doesn't work why are you proposing it? I'm not familiar with this code so I'm not doing anything more than a cursory look, but I don't understand this.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have the impression that it is a bug in Macos Sierra that it isn't working this way. Still, it might work correctly with later Macos versions and seems to be the right thing to use the API to do what is being attempted here.

values[4] = label_cf;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why did you get rid of this CFRelease

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a mistake. I will readd it. Thanks for noticing.

keys[4] = kSecAttrLabel;
query_dict = CFDictionaryCreate(NULL, (const void **)keys,
(const void **)values, 4L,
&kCFCopyStringDictionaryKeyCallBacks,
&kCFTypeDictionaryValueCallBacks);
CFRelease(values[3]);
CFRelease(label_cf);

/* Do we have a match? */
status = SecItemCopyMatching(query_dict, (CFTypeRef *)out_cert_and_key);
status = SecItemCopyMatching(query_dict, (CFTypeRef *) &keys_list);

/* Because kSecAttrLabel matching doesn't work with kSecClassIdentity,
* we need to find the correct identity ourselves */
keys_list_count = CFArrayGetCount( keys_list );
*out_cert_and_key = NULL;
for(i=0; i<keys_list_count; i++) {
OSStatus err = noErr;
SecCertificateRef cert = NULL;
*out_cert_and_key = (SecIdentityRef) CFArrayGetValueAtIndex(keys_list, i);
err = SecIdentityCopyCertificate(*out_cert_and_key, &cert);
if(err == noErr) {
CFStringRef cert_summary = CopyCertSubject(cert);
if(CFStringCompare(cert_summary, label_cf, NULL) == kCFCompareEqualTo) {
break;
}
}
*out_cert_and_key = NULL;
status = 1;
}

CFRelease(query_dict);
CFRelease(label_cf);
}
else {
#if CURL_SUPPORT_MAC_10_6
Expand Down Expand Up @@ -1210,6 +1235,7 @@ static CURLcode darwinssl_connect_step1(struct connectdata *conn,
err = CopyIdentityWithLabel(data->set.str[STRING_CERT], &cert_and_key);

if(err == noErr) {

SecCertificateRef cert = NULL;
CFTypeRef certs_c[1];
CFArrayRef certs;
Expand Down