threads / patch / 33460

patchgit-imap-send.txt: remove the use of sslverify=false in GMail example

Subject: [PATCH] git-imap-send.txt: remove the use of sslverify=false in GMail example

## tl;dr

8 messages between Apr 10, 2013 and Apr 24, 2013. Diffs are folded; open one to read it.

replies: 7people: 3as markdown or json

Barbu Paul - Gheorghe· Apr 10, 2013, 14:59 UTC · lore

Since GMail is SSL capable there is no need to set sslverify to false, the example using it may confuse readers that it's needed since it's also used in the previous example configurations, too

Signed-off-by: Barbu Paul - Gheorghe <barbu.paul.gheorghe@gmail.com>
---
 Documentation/git-imap-send.txt | 1 -
 1 file changed, 1 deletion(-)
Show changes to Documentation/git-imap-send.txt +0 −1
diff --git a/Documentation/git-imap-send.txt b/Documentation/git-imap-send.txt
index 875d283..b15dffe 100644
--- a/Documentation/git-imap-send.txt
+++ b/Documentation/git-imap-send.txt
@@ -123,7 +123,6 @@ to specify your account settings:
 	host = imaps://imap.gmail.com
 	user = user@gmail.com
 	port = 993
-	sslverify = false
 ---------
  You might need to instead use: folder = "[Google Mail]/Drafts" if you get an error
-- 
Barbu Paul - Gheorghe
Common sense is not so common - Voltaire
Visit My GitHub profile to see my open-source projects - https://github.com/paullik
Junio C Hamano· Apr 10, 2013, 18:44 UTC · re: Barbu Paul - Gheorghe · lore

Re: [PATCH] git-imap-send.txt: remove the use of sslverify=false in GMail example

Barbu Paul - Gheorghe <barbu.paul.gheorghe@gmail.com> writes:
Show 6 quoted lines
> Since GMail is SSL capable there is no need to set sslverify to false, the
> example using it may confuse readers that it's needed since it's also used in
> the previous example configurations, too
>
> Signed-off-by: Barbu Paul - Gheorghe <barbu.paul.gheorghe@gmail.com>
> ---
Thanks.

While removing that item from the configuration is a good thing to do in the post 1.8.2.1 era, the reason why it is does not have much to do with "GMail is SSL capable".

The configuration item is not about "Do we connect over SSL when talking to this host?", but is about "When we use SSL with this host, do we verify the certificate it gave us?".

The reason why we can run with sslverify=true against gmail is because we know imap.gmail.com gives a validly signed certificate that leads all the way to a root CA the user's OpenSSL installation is likely to trust (if your hand-rolled imap-over-ssl server uses a snakeoil certificate, even though the server may be "SSL capable", you may not be able to successfully connect to it without sslverify turned off).

Side note. Before 1.8.2 and/or 1.8.1.4, git-imap-send did not implement sslverify correctly; CVS-2013-0308 was inherited from its origin "isync", where it _did_ verify the certificate is valid, but did not make sure the certificate was for the host it thought it was talking with.

Also note that 1.8.2.1 and/or 1.8.1.6 were the first versions that support Server Name Identification (RFC4366). Connection with older versions of git-imap-send over SSL to hosts like googlemail.com that multi-home different SSL hosts can receive a valid certificate for another host that sits at the same IP address, which will lead to the sslverify check to fail.

Show 14 quoted lines
>  Documentation/git-imap-send.txt | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/Documentation/git-imap-send.txt b/Documentation/git-imap-send.txt
> index 875d283..b15dffe 100644
> --- a/Documentation/git-imap-send.txt
> +++ b/Documentation/git-imap-send.txt
> @@ -123,7 +123,6 @@ to specify your account settings:
>  	host = imaps://imap.gmail.com
>  	user = user@gmail.com
>  	port = 993
> -	sslverify = false
>  ---------
>   You might need to instead use: folder = "[Google Mail]/Drafts" if you get an error
Barbu Paul - Gheorghe· Apr 11, 2013, 13:36 UTC · re: Junio C Hamano · lore

Re: [PATCH] git-imap-send.txt: remove the use of sslverify=false in GMail example

On 04/10/2013 09:44 PM, Junio C Hamano wrote:
> Thanks.
My pleasure.
> While removing that item from the configuration is a good thing to
> do in the post 1.8.2.1 era, the reason why it is does not have much
> to do with "GMail is SSL capable".

Should I change the commit message in order to avoid confusion among devs that read it?

> The configuration item is not about "Do we connect over SSL when
> talking to this host?", but is about "When we use SSL with this
> host, do we verify the certificate it gave us?".

If I change it, how should it sound? It could be:

Since GMail's certificates can be sslverify-ed there is no need to set sslverify to false, the example using it may confuse readers that it's needed since it's also used in the previous example configurations, too.

Have a nice day!
-- 
Barbu Paul - Gheorghe
Common sense is not so common - Voltaire
Visit My GitHub profile to see my open-source projects - https://github.com/paullik
Simon Ruderich· Apr 11, 2013, 15:26 UTC · re: Junio C Hamano · lore

Re: [PATCH] git-imap-send.txt: remove the use of sslverify=false in GMail example

On Wed, Apr 10, 2013 at 11:44:03AM -0700, Junio C Hamano wrote:
Show 7 quoted lines
> The reason why we can run with sslverify=true against gmail is
> because we know imap.gmail.com gives a validly signed certificate
> that leads all the way to a root CA the user's OpenSSL installation
> is likely to trust (if your hand-rolled imap-over-ssl server uses a
> snakeoil certificate, even though the server may be "SSL capable",
> you may not be able to successfully connect to it without sslverify
> turned off).

Maybe imap-send should learn imap.sslCAInfo and imap.sslCAPath like http.* to handle custom certificates.

Show 11 quoted lines
>> diff --git a/Documentation/git-imap-send.txt b/Documentation/git-imap-send.txt
>> index 875d283..b15dffe 100644
>> --- a/Documentation/git-imap-send.txt
>> +++ b/Documentation/git-imap-send.txt
>> @@ -123,7 +123,6 @@ to specify your account settings:
>>  	host = imaps://imap.gmail.com
>>  	user = user@gmail.com
>>  	port = 993
>> -	sslverify = false
>>  ---------
>>   You might need to instead use: folder = "[Google Mail]/Drafts" if you get an error

I think we should remove sslverify = false from the other example as well. "Recommending" sslverify = false is IMHO a bad idea as SSL provides no protection without verification.

Regards Simon

-- 
+ privacy is necessary
+ using gnupg http://gnupg.org
+ public key id: 0x92FEFDB7E44C32F9
Barbu Paul - Gheorghe· Apr 11, 2013, 15:55 UTC · re: Simon Ruderich · lore

Re: [PATCH] git-imap-send.txt: remove the use of sslverify=false in GMail example

On 04/11/2013 06:26 PM, Simon Ruderich wrote:
> I think we should remove sslverify = false from the other example
> as well. "Recommending" sslverify = false is IMHO a bad idea as
> SSL provides no protection without verification.
Yep, that was why I thought there should be at least an example without it.
Should I create a new patch removing them all?
-- 
Barbu Paul - Gheorghe
Common sense is not so common - Voltaire
Visit My GitHub profile to see my open-source projects - https://github.com/paullik
Simon Ruderich· Apr 20, 2013, 14:08 UTC · re: Barbu Paul - Gheorghe · lore

Re: [PATCH] git-imap-send.txt: remove the use of sslverify=false in GMail example

On Thu, Apr 11, 2013 at 06:55:03PM +0300, Barbu Paul - Gheorghe wrote:
> Should I create a new patch removing them all?

Sounds like a good idea to me. And update the commit message with Junio's suggestions.

Regards Simon

-- 
+ privacy is necessary
+ using gnupg http://gnupg.org
+ public key id: 0x92FEFDB7E44C32F9
Barbu Paul - Gheorghe· Apr 22, 2013, 19:26 UTC · re: Simon Ruderich · lore

[PATCH] git-imap-send.txt: remove the use of sslverify=false

Since SSL provides no protection if the certificates aren't verified it's better not to include sslverify=false in the examples. Also in the post 1.8.2.1 era git is able to properly verify the validity of a certificate as well it's origin.

Signed-off-by: Barbu Paul - Gheorghe <barbu.paul.gheorghe@gmail.com>
---
 Documentation/git-imap-send.txt | 2 --
 1 file changed, 2 deletions(-)
Show changes to Documentation/git-imap-send.txt +0 −2
diff --git a/Documentation/git-imap-send.txt b/Documentation/git-imap-send.txt
index 875d283..0d72977 100644
--- a/Documentation/git-imap-send.txt
+++ b/Documentation/git-imap-send.txt
@@ -108,7 +108,6 @@ Using direct mode with SSL:
     user = bob
     pass = p4ssw0rd
     port = 123
-    sslverify = false
 ..........................
  @@ -123,7 +122,6 @@ to specify your account settings:
 	host = imaps://imap.gmail.com
 	user = user@gmail.com
 	port = 993
-	sslverify = false
 ---------
  You might need to instead use: folder = "[Google Mail]/Drafts" if you get an error
-- 
Barbu Paul - Gheorghe
Common sense is not so common - Voltaire
Visit My GitHub profile to see my open-source projects - https://github.com/paullik
Junio C Hamano· Apr 24, 2013, 17:18 UTC · re: Barbu Paul - Gheorghe · lore

Re: [PATCH] git-imap-send.txt: remove the use of sslverify=false

Barbu Paul - Gheorghe <barbu.paul.gheorghe@gmail.com> writes:
Show 27 quoted lines
> Since SSL provides no protection if the certificates aren't verified it's
> better not to include sslverify=false in the examples.
> Also in the post 1.8.2.1 era git is able to properly verify the validity of a
> certificate as well it's origin.
>
> Signed-off-by: Barbu Paul - Gheorghe <barbu.paul.gheorghe@gmail.com>
> ---
>  Documentation/git-imap-send.txt | 2 --
>  1 file changed, 2 deletions(-)
>
> diff --git a/Documentation/git-imap-send.txt b/Documentation/git-imap-send.txt
> index 875d283..0d72977 100644
> --- a/Documentation/git-imap-send.txt
> +++ b/Documentation/git-imap-send.txt
> @@ -108,7 +108,6 @@ Using direct mode with SSL:
>      user = bob
>      pass = p4ssw0rd
>      port = 123
> -    sslverify = false
>  ..........................
>   @@ -123,7 +122,6 @@ to specify your account settings:
>  	host = imaps://imap.gmail.com
>  	user = user@gmail.com
>  	port = 993
> -	sslverify = false
>  ---------
>   You might need to instead use: folder = "[Google Mail]/Drafts" if you get an error
It is amusing that an MTA can mangle such a short patch this badly.

Count the number of preimage lines in the first hunk and you see only 5 lines but you claim it has 7. Where did the other two go? The second hunk has the same problem. "@@" that introduces the second hunk is not at the leftmost column. Where did the leading SP come from?

The examples in the documentation are primarily to demonstrate how the supported configurations and options can be used and for what purpose. its secondary purpose is to nudge the readers into the best practice.

So I'd suggest a patch that does these things instead of just removing these two:

 (0) Remove the duplication between the Examples header with ~~~~~~
     underline and the EXAMPLE header with ------ underline.
 (1) Use the second hunk of your patch to remove sslverify=false
     from that imap.gmail.com example.  As a public service, it is
     unlikely that the server side is configured to throw a
     certificate that does not verify at you.
 (2) Instead of removing sslverify=false in the imap.example.com
     example, comment it out like this:
     -	sslverify = false
     +	; sslverify = false
     Then mention that the user may want to use sslverify=false
     while troubleshooting, if he suspects that the reason he is
     having trouble connecting is because the certificate he uses at
     the private server at example.com he is trying to set up (or
     has set up) may not be verified correctly.

← back to recent threads