# [PATCH] imap-send.c: set CURLOPT_USE_SSL to CURLUSESSL_TRY 2 messages from 2015-01-06 to 2015-01-06. Participants: Kyle J. McKay, Junio C Hamano. Thread: https://gitlist.dev/t/38293 ## Kyle J. McKay, 2015-01-06 11:20 Subject: [PATCH] imap-send.c: set CURLOPT_USE_SSL to CURLUSESSL_TRY Message-ID: <31f38398d53bcfb1d659cd248db522e@74d39fa044aa309eaea14b9f57fe79c> URL: https://gitlist.dev/e/31f38398d53bcfb1d659cd248db522e%4074d39fa044aa309eaea14b9f57fe79c ``` According to the cURL documentation for the CURLOPT_USE_SSL option, it is only used with plain text protocols that get upgraded to SSL using the STARTTLS command. The server.use_ssl variable is only set when we are using a protocol that is already SSL/TLS (i.e. imaps), so setting CURLOPT_USE_SSL when the server.use_ssl variable is set has no effect whatsoever. Instead, set CURLOPT_USE_SSL to CURLUSESSL_TRY when the server.use_ssl variable is NOT set so that cURL will attempt to upgrade the plain text connection to SSL/TLS using STARTTLS in that case. This much more closely matches the behavior of the non-cURL code path. Signed-off-by: Kyle J. McKay --- *** PATCH IS AGAINST NEXT *** In particular, this patch requires br/imap-send-via-libcurl imap-send.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/imap-send.c b/imap-send.c index 4dfe4c25..5251b750 100644 --- a/imap-send.c +++ b/imap-send.c @@ -1421,8 +1421,8 @@ static CURL *setup_curl(struct imap_server_conf *srvc) strbuf_release(&auth); } - if (server.use_ssl) - curl_easy_setopt(curl, CURLOPT_USE_SSL, (long)CURLUSESSL_ALL); + if (!server.use_ssl) + curl_easy_setopt(curl, CURLOPT_USE_SSL, (long)CURLUSESSL_TRY); curl_easy_setopt(curl, CURLOPT_SSL_VERIFYPEER, server.ssl_verify); curl_easy_setopt(curl, CURLOPT_SSL_VERIFYHOST, server.ssl_verify); -- 2.1.4 ``` ## Junio C Hamano, 2015-01-06 20:19 Subject: Re: [PATCH] imap-send.c: set CURLOPT_USE_SSL to CURLUSESSL_TRY Message-ID: URL: https://gitlist.dev/e/xmqqiogjoe67.fsf%40gitster.dls.corp.google.com In-Reply-To: <31f38398d53bcfb1d659cd248db522e@74d39fa044aa309eaea14b9f57fe79c> ``` "Kyle J. McKay" writes: > According to the cURL documentation for the CURLOPT_USE_SSL option, > it is only used with plain text protocols that get upgraded to SSL > using the STARTTLS command. > > The server.use_ssl variable is only set when we are using a protocol > that is already SSL/TLS (i.e. imaps), so setting CURLOPT_USE_SSL > when the server.use_ssl variable is set has no effect whatsoever. > > Instead, set CURLOPT_USE_SSL to CURLUSESSL_TRY when the server.use_ssl > variable is NOT set so that cURL will attempt to upgrade the plain > text connection to SSL/TLS using STARTTLS in that case. > > This much more closely matches the behavior of the non-cURL code path. > > Signed-off-by: Kyle J. McKay > --- It sounds sensible from the description of the approach but I'd prefer independent opinion to sanity-check me. Will queue while waiting for others to speak up. Thanks. > > *** PATCH IS AGAINST NEXT *** > > In particular, this patch requires br/imap-send-via-libcurl > > > imap-send.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/imap-send.c b/imap-send.c > index 4dfe4c25..5251b750 100644 > --- a/imap-send.c > +++ b/imap-send.c > @@ -1421,8 +1421,8 @@ static CURL *setup_curl(struct imap_server_conf *srvc) > strbuf_release(&auth); > } > > - if (server.use_ssl) > - curl_easy_setopt(curl, CURLOPT_USE_SSL, (long)CURLUSESSL_ALL); > + if (!server.use_ssl) > + curl_easy_setopt(curl, CURLOPT_USE_SSL, (long)CURLUSESSL_TRY); > > curl_easy_setopt(curl, CURLOPT_SSL_VERIFYPEER, server.ssl_verify); > curl_easy_setopt(curl, CURLOPT_SSL_VERIFYHOST, server.ssl_verify); ```