From 16bb32e104d348e05ec84436ded662ae4f761019 Mon Sep 17 00:00:00 2001 From: Stefan Eissing Date: Thu, 5 Jan 2023 11:13:17 +0100 Subject: sectransp: fix for incomplete read/writes SecureTransport expects result code errSSLWouldBlock when the requested length could not be sent/recieved in full. The previous code returned noErr, which let SecureTransport to believe that the IO had terminated prematurely. Fixes #10227 Closes #10235 --- lib/vtls/sectransp.c | 128 +++++++++++++++++++++++++++++++++++---------------- 1 file changed, 88 insertions(+), 40 deletions(-) diff --git a/lib/vtls/sectransp.c b/lib/vtls/sectransp.c index c6b566f0c..98fc61a5e 100644 --- a/lib/vtls/sectransp.c +++ b/lib/vtls/sectransp.c @@ -136,6 +136,15 @@ /* The last #include file should be: */ #include "memdebug.h" + +#define DEBUG_CF 0 + +#if DEBUG_CF +#define CF_DEBUGF(x) x +#else +#define CF_DEBUGF(x) do { } while(0) +#endif + /* From MacTypes.h (which we can't include because it isn't present in iOS: */ #define ioErr -36 #define paramErr -50 @@ -838,6 +847,8 @@ static OSStatus bio_cf_in_read(SSLConnectionRef connection, DEBUGASSERT(data); nread = Curl_conn_cf_recv(cf->next, data, buf, *dataLength, &result); + CF_DEBUGF(infof(data, CFMSG(cf, "bio_read(len=%zu) -> %zd, result=%d"), + *dataLength, nread, result)); if(nread < 0) { switch(result) { case CURLE_OK: @@ -851,6 +862,9 @@ static OSStatus bio_cf_in_read(SSLConnectionRef connection, } nread = 0; } + else if((size_t)nread < *dataLength) { + rtn = errSSLWouldBlock; + } *dataLength = nread; return rtn; } @@ -865,22 +879,27 @@ static OSStatus bio_cf_out_write(SSLConnectionRef connection, struct Curl_easy *data = connssl->call_data; ssize_t nwritten; CURLcode result; - OSStatus ortn = noErr; + OSStatus rtn = noErr; DEBUGASSERT(data); nwritten = Curl_conn_cf_send(cf->next, data, buf, *dataLength, &result); + CF_DEBUGF(infof(data, CFMSG(cf, "bio_send(len=%zu) -> %zd, result=%d"), + *dataLength, nwritten, result)); if(nwritten <= 0) { if(result == CURLE_AGAIN) { - ortn = errSSLWouldBlock; + rtn = errSSLWouldBlock; backend->ssl_direction = true; } else { - ortn = ioErr; + rtn = ioErr; } nwritten = 0; } + else if((size_t)nwritten < *dataLength) { + rtn = errSSLWouldBlock; + } *dataLength = nwritten; - return ortn; + return rtn; } #ifndef CURL_DISABLE_VERBOSE_STRINGS @@ -1638,6 +1657,7 @@ static CURLcode sectransp_connect_step1(struct Curl_cfilter *cf, DEBUGASSERT(backend); + CF_DEBUGF(infof(data, CFMSG(cf, "connect_step1"))); GetDarwinVersionNumber(&darwinver_maj, &darwinver_min); #endif /* CURL_BUILD_MAC */ @@ -2236,7 +2256,8 @@ static int append_cert_to_array(struct Curl_easy *data, return CURLE_OK; } -static CURLcode verify_cert_buf(struct Curl_easy *data, +static CURLcode verify_cert_buf(struct Curl_cfilter *cf, + struct Curl_easy *data, const unsigned char *certbuf, size_t buflen, SSLContextRef ctx) { @@ -2244,7 +2265,12 @@ static CURLcode verify_cert_buf(struct Curl_easy *data, long res; unsigned char *der; size_t derlen, offset = 0; - + OSStatus ret; + SecTrustResultType trust_eval; + CFMutableArrayRef array = NULL; + SecTrustRef trust = NULL; + CURLcode result = CURLE_PEER_FAILED_VERIFICATION; + (void)cf; /* * Certbuf now contains the contents of the certificate file, which can be * - a single DER certificate, @@ -2254,11 +2280,11 @@ static CURLcode verify_cert_buf(struct Curl_easy *data, * Go through certbuf, and convert any PEM certificate in it into DER * format. */ - CFMutableArrayRef array = CFArrayCreateMutable(kCFAllocatorDefault, 0, - &kCFTypeArrayCallBacks); + array = CFArrayCreateMutable(kCFAllocatorDefault, 0, &kCFTypeArrayCallBacks); if(!array) { failf(data, "SSL: out of memory creating CA certificate array"); - return CURLE_OUT_OF_MEMORY; + result = CURLE_OUT_OF_MEMORY; + goto out; } while(offset < buflen) { @@ -2270,10 +2296,10 @@ static CURLcode verify_cert_buf(struct Curl_easy *data, */ res = pem_to_der((const char *)certbuf + offset, &der, &derlen); if(res < 0) { - CFRelease(array); failf(data, "SSL: invalid CA certificate #%d (offset %zu) in bundle", n, offset); - return CURLE_SSL_CACERT_BADFILE; + result = CURLE_SSL_CACERT_BADFILE; + goto out; } offset += res; @@ -2281,8 +2307,9 @@ static CURLcode verify_cert_buf(struct Curl_easy *data, /* This is not a PEM file, probably a certificate in DER format. */ rc = append_cert_to_array(data, certbuf, buflen, array); if(rc != CURLE_OK) { - CFRelease(array); - return rc; + CF_DEBUGF(infof(data, CFMSG(cf, "append_cert for CA failed"))); + result = rc; + goto out; } break; } @@ -2294,63 +2321,73 @@ static CURLcode verify_cert_buf(struct Curl_easy *data, rc = append_cert_to_array(data, der, derlen, array); free(der); if(rc != CURLE_OK) { - CFRelease(array); - return rc; + CF_DEBUGF(infof(data, CFMSG(cf, "append_cert for CA failed"))); + result = rc; + goto out; } } - SecTrustRef trust; - OSStatus ret = SSLCopyPeerTrust(ctx, &trust); + ret = SSLCopyPeerTrust(ctx, &trust); if(!trust) { failf(data, "SSL: error getting certificate chain"); - CFRelease(array); - return CURLE_PEER_FAILED_VERIFICATION; + goto out; } else if(ret != noErr) { - CFRelease(array); failf(data, "SSLCopyPeerTrust() returned error %d", ret); - return CURLE_PEER_FAILED_VERIFICATION; + goto out; } + CF_DEBUGF(infof(data, CFMSG(cf, "setting %d trust anchors"), n)); ret = SecTrustSetAnchorCertificates(trust, array); if(ret != noErr) { - CFRelease(array); - CFRelease(trust); failf(data, "SecTrustSetAnchorCertificates() returned error %d", ret); - return CURLE_PEER_FAILED_VERIFICATION; + goto out; } ret = SecTrustSetAnchorCertificatesOnly(trust, true); if(ret != noErr) { - CFRelease(array); - CFRelease(trust); failf(data, "SecTrustSetAnchorCertificatesOnly() returned error %d", ret); - return CURLE_PEER_FAILED_VERIFICATION; + goto out; } - SecTrustResultType trust_eval = 0; + trust_eval = 0; ret = SecTrustEvaluate(trust, &trust_eval); - CFRelease(array); - CFRelease(trust); if(ret != noErr) { failf(data, "SecTrustEvaluate() returned error %d", ret); - return CURLE_PEER_FAILED_VERIFICATION; + goto out; } switch(trust_eval) { case kSecTrustResultUnspecified: + /* what does this really mean? */ + CF_DEBUGF(infof(data, CFMSG(cf, "trust result: Unspecified"))); + result = CURLE_OK; + goto out; case kSecTrustResultProceed: - return CURLE_OK; + CF_DEBUGF(infof(data, CFMSG(cf, "trust result: Proceed"))); + result = CURLE_OK; + goto out; case kSecTrustResultRecoverableTrustFailure: + failf(data, "SSL: peer not verified: RecoverableTrustFailure"); + goto out; case kSecTrustResultDeny: + failf(data, "SSL: peer not verified: Deny"); + goto out; default: - failf(data, "SSL: certificate verification failed (result: %d)", - trust_eval); - return CURLE_PEER_FAILED_VERIFICATION; + failf(data, "SSL: perr not verified: result=%d", trust_eval); + goto out; } + +out: + if(trust) + CFRelease(trust); + if(array) + CFRelease(array); + return result; } -static CURLcode verify_cert(struct Curl_easy *data, const char *cafile, +static CURLcode verify_cert(struct Curl_cfilter *cf, + struct Curl_easy *data, const char *cafile, const struct curl_blob *ca_info_blob, SSLContextRef ctx) { @@ -2359,6 +2396,7 @@ static CURLcode verify_cert(struct Curl_easy *data, const char *cafile, size_t buflen; if(ca_info_blob) { + CF_DEBUGF(infof(data, CFMSG(cf, "verify_peer, CA from config blob"))); certbuf = (unsigned char *)malloc(ca_info_blob->len + 1); if(!certbuf) { return CURLE_OUT_OF_MEMORY; @@ -2368,6 +2406,8 @@ static CURLcode verify_cert(struct Curl_easy *data, const char *cafile, certbuf[ca_info_blob->len]='\0'; } else if(cafile) { + CF_DEBUGF(infof(data, CFMSG(cf, "verify_peer, CA from file '%s'"), + cafile)); if(read_cert(cafile, &certbuf, &buflen) < 0) { failf(data, "SSL: failed to read or invalid CA certificate"); return CURLE_SSL_CACERT_BADFILE; @@ -2376,7 +2416,7 @@ static CURLcode verify_cert(struct Curl_easy *data, const char *cafile, else return CURLE_SSL_CACERT_BADFILE; - result = verify_cert_buf(data, certbuf, buflen, ctx); + result = verify_cert_buf(cf, data, certbuf, buflen, ctx); free(certbuf); return result; } @@ -2503,8 +2543,10 @@ static CURLcode sectransp_connect_step2(struct Curl_cfilter *cf, || ssl_connect_2_reading == connssl->connecting_state || ssl_connect_2_writing == connssl->connecting_state); DEBUGASSERT(backend); + CF_DEBUGF(infof(data, CFMSG(cf, "connect_step2"))); /* Here goes nothing: */ +check_handshake: err = SSLHandshake(backend->ssl_ctx); if(err != noErr) { @@ -2519,14 +2561,14 @@ static CURLcode sectransp_connect_step2(struct Curl_cfilter *cf, case -9841: if((conn_config->CAfile || conn_config->ca_info_blob) && conn_config->verifypeer) { - CURLcode result = verify_cert(data, conn_config->CAfile, + CURLcode result = verify_cert(cf, data, conn_config->CAfile, conn_config->ca_info_blob, backend->ssl_ctx); if(result) return result; } /* the documentation says we need to call SSLHandshake() again */ - return sectransp_connect_step2(cf, data); + goto check_handshake; /* Problem with encrypt / decrypt */ case errSSLPeerDecodeError: @@ -2966,6 +3008,7 @@ static CURLcode sectransp_connect_step3(struct Curl_cfilter *cf, { struct ssl_connect_data *connssl = cf->ctx; + CF_DEBUGF(infof(data, CFMSG(cf, "connect_step3"))); /* There is no step 3! * Well, okay, let's collect server certificates, and if verbose mode is on, * let's print the details of the server certificates. */ @@ -3074,6 +3117,7 @@ sectransp_connect_common(struct Curl_cfilter *cf, struct Curl_easy *data, } if(ssl_connect_done == connssl->connecting_state) { + CF_DEBUGF(infof(data, CFMSG(cf, "connected"))); connssl->state = ssl_connection_complete; *done = TRUE; } @@ -3119,6 +3163,7 @@ static void sectransp_close(struct Curl_cfilter *cf, struct Curl_easy *data) DEBUGASSERT(backend); if(backend->ssl_ctx) { + CF_DEBUGF(infof(data, CFMSG(cf, "close"))); (void)SSLClose(backend->ssl_ctx); #if CURL_BUILD_MAC_10_8 || CURL_BUILD_IOS if(SSLCreateContext) @@ -3162,6 +3207,7 @@ static int sectransp_shutdown(struct Curl_cfilter *cf, what = SOCKET_READABLE(cf->conn->sock[cf->sockindex], SSL_SHUTDOWN_TIMEOUT); + CF_DEBUGF(infof(data, CFMSG(cf, "shutdown"))); while(loop--) { if(what < 0) { /* anything that gets here is fatally bad */ @@ -3230,6 +3276,7 @@ static int sectransp_check_cxn(struct Curl_cfilter *cf, DEBUGASSERT(backend); if(backend->ssl_ctx) { + CF_DEBUGF(infof(data, CFMSG(cf, "check connection"))); err = SSLGetSessionState(backend->ssl_ctx, &state); if(err == noErr) return state == kSSLConnected || state == kSSLHandshake; @@ -3250,6 +3297,7 @@ static bool sectransp_data_pending(struct Curl_cfilter *cf, DEBUGASSERT(backend); if(backend->ssl_ctx) { /* SSL is in use */ + CF_DEBUGF(infof(data, CFMSG(cf, "data_pending"))); err = SSLGetBufferedReadSize(backend->ssl_ctx, &buffer); if(err == noErr) return buffer > 0UL; @@ -3407,7 +3455,7 @@ static ssize_t sectransp_recv(struct Curl_cfilter *cf, case -9841: if((conn_config->CAfile || conn_config->ca_info_blob) && conn_config->verifypeer) { - CURLcode result = verify_cert(data, conn_config->CAfile, + CURLcode result = verify_cert(cf, data, conn_config->CAfile, conn_config->ca_info_blob, backend->ssl_ctx); if(result) -- cgit v1.2.1