* contrib/credential/netrc/git-credential-netrc: Use of uninitialized value in string
@ 2013-08-24 14:55 Antoine Pelisse
2013-08-27 3:56 ` Junio C Hamano
0 siblings, 1 reply; 6+ messages in thread
From: Antoine Pelisse @ 2013-08-24 14:55 UTC (permalink / raw)
To: git, Ted Zlatanov
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 apelisse@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 = apelisse@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 <apelisse@gmail.com>"
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 <apelisse@gmail.com>"
compare host [smtp.gmail.com:587] to [smtp.gmail.com:587] (entry:
password=secret, username=apelisse@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=apelisse@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
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: contrib/credential/netrc/git-credential-netrc: Use of uninitialized value in string 2013-08-24 14:55 contrib/credential/netrc/git-credential-netrc: Use of uninitialized value in string Antoine Pelisse @ 2013-08-27 3:56 ` Junio C Hamano 2013-08-27 20:05 ` Jeff King 0 siblings, 1 reply; 6+ messages in thread From: Junio C Hamano @ 2013-08-27 3:56 UTC (permalink / raw) To: Jeff King; +Cc: Antoine Pelisse, git, Ted Zlatanov Antoine Pelisse <apelisse@gmail.com> 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 apelisse@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 = apelisse@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 <apelisse@gmail.com>" > 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 <apelisse@gmail.com>" > compare host [smtp.gmail.com:587] to [smtp.gmail.com:587] (entry: > password=secret, username=apelisse@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=apelisse@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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: contrib/credential/netrc/git-credential-netrc: Use of uninitialized value in string 2013-08-27 3:56 ` Junio C Hamano @ 2013-08-27 20:05 ` Jeff King 2013-09-03 15:23 ` Ted Zlatanov 0 siblings, 1 reply; 6+ messages in thread From: Jeff King @ 2013-08-27 20:05 UTC (permalink / raw) To: Junio C Hamano; +Cc: Antoine Pelisse, git, Ted Zlatanov On Mon, Aug 26, 2013 at 08:56:23PM -0700, Junio C Hamano wrote: > Antoine Pelisse <apelisse@gmail.com> 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=apelisse@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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: contrib/credential/netrc/git-credential-netrc: Use of uninitialized value in string 2013-08-27 20:05 ` Jeff King @ 2013-09-03 15:23 ` Ted Zlatanov 2013-09-03 17:35 ` Jeff King 0 siblings, 1 reply; 6+ messages in thread From: Ted Zlatanov @ 2013-09-03 15:23 UTC (permalink / raw) To: git On Tue, 27 Aug 2013 16:05:51 -0400 Jeff King <peff@peff.net> wrote: JK> On Mon, Aug 26, 2013 at 08:56:23PM -0700, Junio C Hamano wrote: >> Antoine Pelisse <apelisse@gmail.com> 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=apelisse@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}, ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: contrib/credential/netrc/git-credential-netrc: Use of uninitialized value in string 2013-09-03 15:23 ` Ted Zlatanov @ 2013-09-03 17:35 ` Jeff King 2013-10-08 14:34 ` Ted Zlatanov 0 siblings, 1 reply; 6+ messages in thread From: Jeff King @ 2013-09-03 17:35 UTC (permalink / raw) To: Ted Zlatanov; +Cc: git 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: contrib/credential/netrc/git-credential-netrc: Use of uninitialized value in string 2013-09-03 17:35 ` Jeff King @ 2013-10-08 14:34 ` Ted Zlatanov 0 siblings, 0 replies; 6+ messages in thread From: Ted Zlatanov @ 2013-10-08 14:34 UTC (permalink / raw) To: git On Tue, 3 Sep 2013 13:35:44 -0400 Jeff King <peff@peff.net> 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-10-08 14:35 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-08-24 14:55 contrib/credential/netrc/git-credential-netrc: Use of uninitialized value in string Antoine Pelisse 2013-08-27 3:56 ` Junio C Hamano 2013-08-27 20:05 ` Jeff King 2013-09-03 15:23 ` Ted Zlatanov 2013-09-03 17:35 ` Jeff King 2013-10-08 14:34 ` Ted Zlatanov
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).