Re: contrib/credential/netrc/git-credential-netrc: Use of uninitialized value in string

[Ted Zlatanov]

On Tue, 3 Sep 2013 13:35:44 -0400 Jeff King  wrote: 

JK> On Tue, Sep 03, 2013 at 11:23:14AM -0400, Ted Zlatanov wrote:
>> Yes, you're right.  Something like the following (untested) could work
>> and does the wildcards, which I will make into a proper patch and test
>> if it looks OK to you.
...
JK> Yeah, that makes sense to me (and is basically what the credential-cache
JK> and credential-store helpers do internally). Thanks for working on this.

Sorry for the delay, patch posted.

Ted

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: contrib/credential/netrc/git-credential-netrc: Use of uninitialized value in string

[Jeff King]

On Tue, Sep 03, 2013 at 11:23:14AM -0400, Ted Zlatanov wrote:

> Yes, you're right.  Something like the following (untested) could work
> and does the wildcards, which I will make into a proper patch and test
> if it looks OK to you.
> 
> Ted
> 
> diff --git a/contrib/credential/netrc/git-credential-netrc 
> b/contrib/credential/netrc/git-credential-netrc
> index 6c51c43..13e537b 100755
> --- a/contrib/credential/netrc/git-credential-netrc
> +++ b/contrib/credential/netrc/git-credential-netrc
> @@ -369,7 +369,10 @@ sub find_netrc_entry {
> {
> my $entry_text = join ', ', map { "$_=$entry->{$_}" } keys 
> %$entry;
> foreach my $check (sort keys %$query) {
> -   if (defined $query->{$check}) {
> +   if (!defined $entry->{$check}) {
> +  log_debug("OK: entry has no $check token, so 
> any value satisfies check $check");
> +   }
> +   elsif (defined $query->{$check}) {
> log_debug("compare %s [%s] to [%s] (entry: 
> %s)",
>   $check,
>   $entry->{$check},

Yeah, that makes sense to me (and is basically what the credential-cache
and credential-store helpers do internally). Thanks for working on this.

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: contrib/credential/netrc/git-credential-netrc: Use of uninitialized value in string

[Ted Zlatanov]

On Tue, 27 Aug 2013 16:05:51 -0400 Jeff King  wrote: 

JK> On Mon, Aug 26, 2013 at 08:56:23PM -0700, Junio C Hamano wrote:
>> Antoine Pelisse  writes:
>> 
>> > I've tried to use the netrc credential with git-send-email
>> > (v1.8.4-rc2), and I've had the following log (running with -d -v):
>> 
>> Peff what do you think?  From credential layer's point of view, I
>> think we make it totally up to the helper to decide if a request
>> matches what it supports, and if a particular helper wants to make
>> sure it is asked for a specific protocol, that is an OK thing to do,
>> but it feels unnecessarily unfriendly and treating missing proto
>> specification as a wildcard to talk to the specified host over any
>> protocol may not hurt, I would think.

JK> Right. It is up to the credential helper to map git's request into
JK> whatever storage it has. So I think the right answer is whatever is
JK> normal and expected for netrc.

JK> Unfortunately that is not really a standardized format. The original
JK> netrc was ftp-only, and did not have a port or protocol field at all.
JK> Programs like curl extend it automatically to http, and just googling
JK> around seems to show other programs using it for imap and smtp. So I
JK> think there is some precedence in simply treating a missing "port" field
JK> as "match any port/protocol" on the machine.

JK> The upside is that it is convenient for the user. The downside is that
JK> we might accidentally send a password to a service that the user does
JK> not expect, which could compromise security. It would at least be on the
JK> matching host, but the protocol might not be as secure as the one the
JK> user intended (e.g., smtp without starttls, when the password was meant
JK> to only go over imap-over-ssl).

This gets tricky, certainly.  I'd rather make it convenient because
users will, anyway.

JK> So I'm on the fence. It is very unlikely to be a bad thing, but if it
JK> is, it may expose user passwords in cleartext. If we are going to keep
JK> the current behavior, it probably needs to be documented

I'm OK with treating missing protocols as wildcards.

JK> and certainly:

>> > Use of uninitialized value $_[2] in printf at
>> > /home/antoine/code/git/contrib/credential/netrc/git-credential-netrc
>> > line 419.
>> > compare protocol [] to [smtp] (entry: password=secret,
>> > username=apeli...@gmail.com, host=smtp.gmail.com:587)
>> > Use of uninitialized value in string eq at
>> > /home/antoine/code/git/contrib/credential/netrc/git-credential-netrc
>> > line 378.

JK> ...these should more cleanly handle the missing field.

Yes, you're right.  Something like the following (untested) could work
and does the wildcards, which I will make into a proper patch and test
if it looks OK to you.

Ted

diff --git a/contrib/credential/netrc/git-credential-netrc 
b/contrib/credential/netrc/git-credential-netrc
index 6c51c43..13e537b 100755
--- a/contrib/credential/netrc/git-credential-netrc
+++ b/contrib/credential/netrc/git-credential-netrc
@@ -369,7 +369,10 @@ sub find_netrc_entry {
{
my $entry_text = join ', ', map { "$_=$entry->{$_}" } keys 
%$entry;
foreach my $check (sort keys %$query) {
-   if (defined $query->{$check}) {
+   if (!defined $entry->{$check}) {
+  log_debug("OK: entry has no $check token, so any 
value satisfies check $check");
+   }
+   elsif (defined $query->{$check}) {
log_debug("compare %s [%s] to [%s] (entry: %s)",
  $check,
  $entry->{$check},

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: contrib/credential/netrc/git-credential-netrc: Use of uninitialized value in string

[Jeff King]

On Mon, Aug 26, 2013 at 08:56:23PM -0700, Junio C Hamano wrote:

> Antoine Pelisse  writes:
> 
> > I've tried to use the netrc credential with git-send-email
> > (v1.8.4-rc2), and I've had the following log (running with -d -v):
> 
> Peff what do you think?  From credential layer's point of view, I
> think we make it totally up to the helper to decide if a request
> matches what it supports, and if a particular helper wants to make
> sure it is asked for a specific protocol, that is an OK thing to do,
> but it feels unnecessarily unfriendly and treating missing proto
> specification as a wildcard to talk to the specified host over any
> protocol may not hurt, I would think.

Right. It is up to the credential helper to map git's request into
whatever storage it has. So I think the right answer is whatever is
normal and expected for netrc.

Unfortunately that is not really a standardized format. The original
netrc was ftp-only, and did not have a port or protocol field at all.
Programs like curl extend it automatically to http, and just googling
around seems to show other programs using it for imap and smtp. So I
think there is some precedence in simply treating a missing "port" field
as "match any port/protocol" on the machine.

The upside is that it is convenient for the user. The downside is that
we might accidentally send a password to a service that the user does
not expect, which could compromise security. It would at least be on the
matching host, but the protocol might not be as secure as the one the
user intended (e.g., smtp without starttls, when the password was meant
to only go over imap-over-ssl).

So I'm on the fence. It is very unlikely to be a bad thing, but if it
is, it may expose user passwords in cleartext. If we are going to keep
the current behavior, it probably needs to be documented, and certainly:

> > Use of uninitialized value $_[2] in printf at
> > /home/antoine/code/git/contrib/credential/netrc/git-credential-netrc
> > line 419.
> > compare protocol [] to [smtp] (entry: password=secret,
> > username=apeli...@gmail.com, host=smtp.gmail.com:587)
> > Use of uninitialized value in string eq at
> > /home/antoine/code/git/contrib/credential/netrc/git-credential-netrc
> > line 378.

...these should more cleanly handle the missing field.

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: contrib/credential/netrc/git-credential-netrc: Use of uninitialized value in string

[Junio C Hamano]

Antoine Pelisse  writes:

> I've tried to use the netrc credential with git-send-email
> (v1.8.4-rc2), and I've had the following log (running with -d -v):

Peff what do you think?  From credential layer's point of view, I
think we make it totally up to the helper to decide if a request
matches what it supports, and if a particular helper wants to make
sure it is asked for a specific protocol, that is an OK thing to do,
but it feels unnecessarily unfriendly and treating missing proto
specification as a wildcard to talk to the specified host over any
protocol may not hurt, I would think.

> We were given search token protocol and value smtp
> We were given search token host and value smtp.gmail.com:587
> We were given search token username and value apeli...@gmail.com
> Searching for host = smtp.gmail.com:587
> Searching for password = (any value)
> Searching for path = (any value)
> Searching for protocol = smtp
> Searching for username = apeli...@gmail.com
> Using GPG to open /home/antoine/.authinfo.gpg: [gpg --decrypt
> /home/antoine/.authinfo.gpg]
>
> You need a passphrase to unlock the secret key for
> user: "Antoine Pelisse "
> 2048-bit RSA key, ID DE2A8792, created 2010-12-31 (main key ID A066A853)
>
> gpg: encrypted with 2048-bit RSA key, ID DE2A8792, created 2010-12-31
>   "Antoine Pelisse "
> compare host [smtp.gmail.com:587] to [smtp.gmail.com:587] (entry:
> password=secret, username=apeli...@gmail.com, host=smtp.gmail.com:587)
> OK: any value satisfies check password
> OK: any value satisfies check path
> Use of uninitialized value $_[2] in printf at
> /home/antoine/code/git/contrib/credential/netrc/git-credential-netrc
> line 419.
> compare protocol [] to [smtp] (entry: password=secret,
> username=apeli...@gmail.com, host=smtp.gmail.com:587)
> Use of uninitialized value in string eq at
> /home/antoine/code/git/contrib/credential/netrc/git-credential-netrc
> line 378.
>
> I can fix the problem by adding a "protocol smtp" to the matching
> line, but I wonder why this would be necessary ? After all, if host
> smtp.gmail.com:587 matches, do we need to match the protocol ?
>
> Cheers,
> Antoine
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

contrib/credential/netrc/git-credential-netrc: Use of uninitialized value in string

[Antoine Pelisse]

Hello,
I've tried to use the netrc credential with git-send-email
(v1.8.4-rc2), and I've had the following log (running with -d -v):

We were given search token protocol and value smtp
We were given search token host and value smtp.gmail.com:587
We were given search token username and value apeli...@gmail.com
Searching for host = smtp.gmail.com:587
Searching for password = (any value)
Searching for path = (any value)
Searching for protocol = smtp
Searching for username = apeli...@gmail.com
Using GPG to open /home/antoine/.authinfo.gpg: [gpg --decrypt
/home/antoine/.authinfo.gpg]

You need a passphrase to unlock the secret key for
user: "Antoine Pelisse "
2048-bit RSA key, ID DE2A8792, created 2010-12-31 (main key ID A066A853)

gpg: encrypted with 2048-bit RSA key, ID DE2A8792, created 2010-12-31
  "Antoine Pelisse "
compare host [smtp.gmail.com:587] to [smtp.gmail.com:587] (entry:
password=secret, username=apeli...@gmail.com, host=smtp.gmail.com:587)
OK: any value satisfies check password
OK: any value satisfies check path
Use of uninitialized value $_[2] in printf at
/home/antoine/code/git/contrib/credential/netrc/git-credential-netrc
line 419.
compare protocol [] to [smtp] (entry: password=secret,
username=apeli...@gmail.com, host=smtp.gmail.com:587)
Use of uninitialized value in string eq at
/home/antoine/code/git/contrib/credential/netrc/git-credential-netrc
line 378.

I can fix the problem by adding a "protocol smtp" to the matching
line, but I wonder why this would be necessary ? After all, if host
smtp.gmail.com:587 matches, do we need to match the protocol ?

Cheers,
Antoine
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html