On Wed, Sep 3, 2014 at 11:23 PM, Toby Peterson <[email protected]> wrote:
>
>> On Sep 3, 2014, at 03:41, Vilmos Nebehaj <[email protected]> wrote:
>>
>> On Wed, Sep 3, 2014 at 11:55 AM, Vilmos Nebehaj <[email protected]> wrote:
>>> On Wed, Sep 3, 2014 at 1:16 AM, Toby Peterson <[email protected]> wrote:
>>>> On Aug 29, 2014, at 03:55, Vilmos Nebehaj <[email protected]> wrote:
>>>>>
>>>>> On Fri, Aug 29, 2014 at 1:56 AM, Nick Zitzmann <[email protected]> 
>>>>> wrote:
>>>>>>
>>>>>> On Aug 28, 2014, at 6:02 PM, Vilmos Nebehaj <[email protected]> wrote:
>>>>>>
>>>>>>> The comment about wildcard certificates was a red herring it seems.
>>>>>>>
>>>>>>> The problem is that if the user via --cacert supplies a certificate
>>>>>>> bundle with multiple CA certificates in it, curl_darwinssl.c will only
>>>>>>> use the first one.
>>>>>>>
>>>>>>> For a fix, see https://github.com/ldx/curl/tree/darwinsslfix
>>>>>>>
>>>>>>> Can someone confirm this works? I tested it on OS X 10.9 with
>>>>>>> - the cacerts.pem bundle from the ticket,
>>>>>>> - a cert file containing only one cert and
>>>>>>> - a DER cert file.
>>>>>>
>>>>>> Great! I can confirm that this works with the PEM bundle in the bug 
>>>>>> report.
>>>>>>
>>>>>> Could you please clean up the compiler warnings, fix the code style 
>>>>>> issues (which you can see by building the project with --enable-debug 
>>>>>> specified), remove the "SSL: parsing CA certificate file" and "SSL: 
>>>>>> certificate verification succeeded" verbose log messages, and then 
>>>>>> submit a pull request?
>>>>>
>>>>> Here it is:
>>>>>
>>>>> https://github.com/bagder/curl/pull/114
>>>>>
>>>>> Thanks Nick!
>>>>
>>>> Quick followup. 4c134bc seems to function as intended - thanks! However, 
>>>> the second change (0426670) breaks the build on iOS, because 
>>>> SecCertificateCopyPublicKey is not available. I'm not aware of a good 
>>>> replacement, unfortunately. #ifdef'ing that check out works, of course.
>>>
>>> The only reason for using SecCertificateCopyPublicKey() is to check if
>>> the CA certificate was valid. Let me try some other
>>> SecCetificateCopy*() functions that are available on iPhone to see if
>>> they also catch invalid certificates.
>>
>> This PR fixes the issue:
>>
>> https://github.com/bagder/curl/pull/116
>>
>> Toby, can you test this compiles for iPhone? Thanks!
>
> Yes, that change builds for iOS, and also appears to work (no cert 
> verification errors).

Thanks!

Attached a patch with this fix.

Cheers,
Vilmos

> - Toby
>
>>
>>> Vilmos
>>>
>>>> - Toby
>>>>
>>>>>
>>>>> Cheers,
>>>>> Vilmos
>>>>>
>>>>>> Thanks!
>>>>>>
>>>>>> Nick Zitzmann
>>>>>> <http://www.chronosnet.com/>
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>> -------------------------------------------------------------------
>>>>>> List admin: http://cool.haxx.se/list/listinfo/curl-library
>>>>>> Etiquette:  http://curl.haxx.se/mail/etiquette.html
>>>>>
>>>>> -------------------------------------------------------------------
>>>>> List admin: http://cool.haxx.se/list/listinfo/curl-library
>>>>> Etiquette:  http://curl.haxx.se/mail/etiquette.html
>>>>
>>>> -------------------------------------------------------------------
>>>> List admin: http://cool.haxx.se/list/listinfo/curl-library
>>>> Etiquette:  http://curl.haxx.se/mail/etiquette.html
>>
>> -------------------------------------------------------------------
>> List admin: http://cool.haxx.se/list/listinfo/curl-library
>> Etiquette:  http://curl.haxx.se/mail/etiquette.html
>
> -------------------------------------------------------------------
> List admin: http://cool.haxx.se/list/listinfo/curl-library
> Etiquette:  http://curl.haxx.se/mail/etiquette.html
From bfcee0c63e55712f632e25d323d0ddc3b5984662 Mon Sep 17 00:00:00 2001
From: Vilmos Nebehaj <[email protected]>
Date: Wed, 3 Sep 2014 11:39:16 +0200
Subject: [PATCH] darwinssl: Use CopyCertSubject() to check CA cert.

SecCertificateCopyPublicKey() is not available on iPhone. Use
CopyCertSubject() instead to see if the certificate returned by
SecCertificateCreateWithData() is valid.

Reported-by: Toby Peterson
---
 lib/vtls/curl_darwinssl.c | 19 +++++++++++++++----
 1 file changed, 15 insertions(+), 4 deletions(-)

diff --git a/lib/vtls/curl_darwinssl.c b/lib/vtls/curl_darwinssl.c
index 3726357..f229c6f 100644
--- a/lib/vtls/curl_darwinssl.c
+++ b/lib/vtls/curl_darwinssl.c
@@ -1672,14 +1672,25 @@ static int append_cert_to_array(struct SessionHandle *data,
     }
 
     /* Check if cacert is valid. */
-    SecKeyRef key;
-    OSStatus ret = SecCertificateCopyPublicKey(cacert, &key);
-    if(ret != noErr) {
+    CFStringRef subject = CopyCertSubject(cacert);
+    if(subject) {
+      char subject_cbuf[128];
+      memset(subject_cbuf, 0, 128);
+      if(!CFStringGetCString(subject,
+                            subject_cbuf,
+                            128,
+                            kCFStringEncodingUTF8)) {
+        CFRelease(cacert);
+        failf(data, "SSL: invalid CA certificate subject");
+        return CURLE_SSL_CACERT;
+      }
+      CFRelease(subject);
+    }
+    else {
       CFRelease(cacert);
       failf(data, "SSL: invalid CA certificate");
       return CURLE_SSL_CACERT;
     }
-    CFRelease(key);
 
     CFArrayAppendValue(array, cacert);
     CFRelease(cacert);
-- 
2.1.0

-------------------------------------------------------------------
List admin: http://cool.haxx.se/list/listinfo/curl-library
Etiquette:  http://curl.haxx.se/mail/etiquette.html

Reply via email to