{"thread":{"id":"34760","subject":"contrib/credential/netrc/git-credential-netrc: Use of uninitialized value in string","startedAt":"2013-08-24T14:55:32Z","lastAt":"2013-10-08T14:34:52Z","messageCount":6,"participants":["Antoine Pelisse","Junio C Hamano","Jeff King","Ted Zlatanov"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"225793","messageId":"CALWbr2ynAvevTxFd3duAfFzgv0DCGTDxFUaniW-8hM+eoLiOEw@mail.gmail.com","threadId":"34760","inReplyTo":null,"subject":"contrib/credential/netrc/git-credential-netrc: Use of uninitialized value in string","fromName":"Antoine Pelisse","fromEmail":"apelisse@gmail.com","sentAt":"2013-08-24T14:55:32Z","receivedAt":"2013-08-24T14:55:32Z","isPatch":false,"sender":{"key":"apelisse@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1929644?v=4"},"body":"Hello,\nI've tried to use the netrc credential with git-send-email\n(v1.8.4-rc2), and I've had the following log (running with -d -v):\n\nWe were given search token protocol and value smtp\nWe were given search token host and value smtp.gmail.com:587\nWe were given search token username and value apelisse@gmail.com\nSearching for host = smtp.gmail.com:587\nSearching for password = (any value)\nSearching for path = (any value)\nSearching for protocol = smtp\nSearching for username = apelisse@gmail.com\nUsing GPG to open /home/antoine/.authinfo.gpg: [gpg --decrypt\n/home/antoine/.authinfo.gpg]\n\nYou need a passphrase to unlock the secret key for\nuser: \"Antoine Pelisse \"\n2048-bit RSA key, ID DE2A8792, created 2010-12-31 (main key ID A066A853)\n\ngpg: encrypted with 2048-bit RSA key, ID DE2A8792, created 2010-12-31\n \"Antoine Pelisse \"\ncompare host [smtp.gmail.com:587] to [smtp.gmail.com:587] (entry:\npassword=secret, username=apelisse@gmail.com, host=smtp.gmail.com:587)\nOK: any value satisfies check password\nOK: any value satisfies check path\nUse of uninitialized value $_[2] in printf at\n/home/antoine/code/git/contrib/credential/netrc/git-credential-netrc\nline 419.\ncompare protocol [] to [smtp] (entry: password=secret,\nusername=apelisse@gmail.com, host=smtp.gmail.com:587)\nUse of uninitialized value in string eq at\n/home/antoine/code/git/contrib/credential/netrc/git-credential-netrc\nline 378.\n\nI can fix the problem by adding a \"protocol smtp\" to the matching\nline, but I wonder why this would be necessary ? After all, if host\nsmtp.gmail.com:587 matches, do we need to match the protocol ?\n\nCheers,\nAntoine\n"},{"id":"225951","messageId":"xmqqr4dfhjmg.fsf@gitster.dls.corp.google.com","threadId":"34760","inReplyTo":"CALWbr2ynAvevTxFd3duAfFzgv0DCGTDxFUaniW-8hM+eoLiOEw@mail.gmail.com","subject":"Re: contrib/credential/netrc/git-credential-netrc: Use of uninitialized value in string","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-08-27T03:56:23Z","receivedAt":"2013-08-27T03:56:23Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Antoine Pelisse writes:\n\n> I've tried to use the netrc credential with git-send-email\n> (v1.8.4-rc2), and I've had the following log (running with -d -v):\n\nPeff what do you think? From credential layer's point of view, I\nthink we make it totally up to the helper to decide if a request\nmatches what it supports, and if a particular helper wants to make\nsure it is asked for a specific protocol, that is an OK thing to do,\nbut it feels unnecessarily unfriendly and treating missing proto\nspecification as a wildcard to talk to the specified host over any\nprotocol may not hurt, I would think.\n\n> We were given search token protocol and value smtp\n> We were given search token host and value smtp.gmail.com:587\n> We were given search token username and value apelisse@gmail.com\n> Searching for host = smtp.gmail.com:587\n> Searching for password = (any value)\n> Searching for path = (any value)\n> Searching for protocol = smtp\n> Searching for username = apelisse@gmail.com\n> Using GPG to open /home/antoine/.authinfo.gpg: [gpg --decrypt\n> /home/antoine/.authinfo.gpg]\n>\n> You need a passphrase to unlock the secret key for\n> user: \"Antoine Pelisse \"\n> 2048-bit RSA key, ID DE2A8792, created 2010-12-31 (main key ID A066A853)\n>\n> gpg: encrypted with 2048-bit RSA key, ID DE2A8792, created 2010-12-31\n> \"Antoine Pelisse \"\n> compare host [smtp.gmail.com:587] to [smtp.gmail.com:587] (entry:\n> password=secret, username=apelisse@gmail.com, host=smtp.gmail.com:587)\n> OK: any value satisfies check password\n> OK: any value satisfies check path\n> Use of uninitialized value $_[2] in printf at\n> /home/antoine/code/git/contrib/credential/netrc/git-credential-netrc\n> line 419.\n> compare protocol [] to [smtp] (entry: password=secret,\n> username=apelisse@gmail.com, host=smtp.gmail.com:587)\n> Use of uninitialized value in string eq at\n> /home/antoine/code/git/contrib/credential/netrc/git-credential-netrc\n> line 378.\n>\n> I can fix the problem by adding a \"protocol smtp\" to the matching\n> line, but I wonder why this would be necessary ? After all, if host\n> smtp.gmail.com:587 matches, do we need to match the protocol ?\n>\n> Cheers,\n> Antoine\n"},{"id":"226058","messageId":"20130827200550.GA17443@sigill.intra.peff.net","threadId":"34760","inReplyTo":"xmqqr4dfhjmg.fsf@gitster.dls.corp.google.com","subject":"Re: contrib/credential/netrc/git-credential-netrc: Use of uninitialized value in string","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-08-27T20:05:51Z","receivedAt":"2013-08-27T20:05:51Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Aug 26, 2013 at 08:56:23PM -0700, Junio C Hamano wrote:\n\n> Antoine Pelisse writes:\n> \n> > I've tried to use the netrc credential with git-send-email\n> > (v1.8.4-rc2), and I've had the following log (running with -d -v):\n> \n> Peff what do you think? From credential layer's point of view, I\n> think we make it totally up to the helper to decide if a request\n> matches what it supports, and if a particular helper wants to make\n> sure it is asked for a specific protocol, that is an OK thing to do,\n> but it feels unnecessarily unfriendly and treating missing proto\n> specification as a wildcard to talk to the specified host over any\n> protocol may not hurt, I would think.\n\nRight. It is up to the credential helper to map git's request into\nwhatever storage it has. So I think the right answer is whatever is\nnormal and expected for netrc.\n\nUnfortunately that is not really a standardized format. The original\nnetrc was ftp-only, and did not have a port or protocol field at all.\nPrograms like curl extend it automatically to http, and just googling\naround seems to show other programs using it for imap and smtp. So I\nthink there is some precedence in simply treating a missing \"port\" field\nas \"match any port/protocol\" on the machine.\n\nThe upside is that it is convenient for the user. The downside is that\nwe might accidentally send a password to a service that the user does\nnot expect, which could compromise security. It would at least be on the\nmatching host, but the protocol might not be as secure as the one the\nuser intended (e.g., smtp without starttls, when the password was meant\nto only go over imap-over-ssl).\n\nSo I'm on the fence. It is very unlikely to be a bad thing, but if it\nis, it may expose user passwords in cleartext. If we are going to keep\nthe current behavior, it probably needs to be documented, and certainly:\n\n> > Use of uninitialized value $_[2] in printf at\n> > /home/antoine/code/git/contrib/credential/netrc/git-credential-netrc\n> > line 419.\n> > compare protocol [] to [smtp] (entry: password=secret,\n> > username=apelisse@gmail.com, host=smtp.gmail.com:587)\n> > Use of uninitialized value in string eq at\n> > /home/antoine/code/git/contrib/credential/netrc/git-credential-netrc\n> > line 378.\n\n...these should more cleanly handle the missing field.\n\n-Peff\n"},{"id":"226624","messageId":"87k3iyhqu5.fsf@lifelogs.com","threadId":"34760","inReplyTo":"20130827200550.GA17443@sigill.intra.peff.net","subject":"Re: contrib/credential/netrc/git-credential-netrc: Use of uninitialized value in string","fromName":"Ted Zlatanov","fromEmail":"tzz@lifelogs.com","sentAt":"2013-09-03T15:23:14Z","receivedAt":"2013-09-03T15:23:14Z","isPatch":false,"sender":{"key":"tzz@lifelogs.com","avatar":"https://avatars.githubusercontent.com/u/67764?v=4"},"body":"On Tue, 27 Aug 2013 16:05:51 -0400 Jeff King wrote: \n\nJK> On Mon, Aug 26, 2013 at 08:56:23PM -0700, Junio C Hamano wrote:\n>> Antoine Pelisse writes:\n>> \n>> > I've tried to use the netrc credential with git-send-email\n>> > (v1.8.4-rc2), and I've had the following log (running with -d -v):\n>> \n>> Peff what do you think? From credential layer's point of view, I\n>> think we make it totally up to the helper to decide if a request\n>> matches what it supports, and if a particular helper wants to make\n>> sure it is asked for a specific protocol, that is an OK thing to do,\n>> but it feels unnecessarily unfriendly and treating missing proto\n>> specification as a wildcard to talk to the specified host over any\n>> protocol may not hurt, I would think.\n\nJK> Right. It is up to the credential helper to map git's request into\nJK> whatever storage it has. So I think the right answer is whatever is\nJK> normal and expected for netrc.\n\nJK> Unfortunately that is not really a standardized format. The original\nJK> netrc was ftp-only, and did not have a port or protocol field at all.\nJK> Programs like curl extend it automatically to http, and just googling\nJK> around seems to show other programs using it for imap and smtp. So I\nJK> think there is some precedence in simply treating a missing \"port\" field\nJK> as \"match any port/protocol\" on the machine.\n\nJK> The upside is that it is convenient for the user. The downside is that\nJK> we might accidentally send a password to a service that the user does\nJK> not expect, which could compromise security. It would at least be on the\nJK> matching host, but the protocol might not be as secure as the one the\nJK> user intended (e.g., smtp without starttls, when the password was meant\nJK> to only go over imap-over-ssl).\n\nThis gets tricky, certainly. I'd rather make it convenient because\nusers will, anyway.\n\nJK> So I'm on the fence. It is very unlikely to be a bad thing, but if it\nJK> is, it may expose user passwords in cleartext. If we are going to keep\nJK> the current behavior, it probably needs to be documented\n\nI'm OK with treating missing protocols as wildcards.\n\nJK> and certainly:\n\n>> > Use of uninitialized value $_[2] in printf at\n>> > /home/antoine/code/git/contrib/credential/netrc/git-credential-netrc\n>> > line 419.\n>> > compare protocol [] to [smtp] (entry: password=secret,\n>> > username=apelisse@gmail.com, host=smtp.gmail.com:587)\n>> > Use of uninitialized value in string eq at\n>> > /home/antoine/code/git/contrib/credential/netrc/git-credential-netrc\n>> > line 378.\n\nJK> ...these should more cleanly handle the missing field.\n\nYes, you're right. Something like the following (untested) could work\nand does the wildcards, which I will make into a proper patch and test\nif it looks OK to you.\n\nTed\n\ndiff --git a/contrib/credential/netrc/git-credential-netrc b/contrib/credential/netrc/git-credential-netrc\nindex 6c51c43..13e537b 100755\n--- a/contrib/credential/netrc/git-credential-netrc\n+++ b/contrib/credential/netrc/git-credential-netrc\n@@ -369,7 +369,10 @@ sub find_netrc_entry {\n {\n my $entry_text = join ', ', map { \"$_=$entry->{$_}\" } keys %$entry;\n foreach my $check (sort keys %$query) {\n- if (defined $query->{$check}) {\n+ if (!defined $entry->{$check}) {\n+ log_debug(\"OK: entry has no $check token, so any value satisfies check $check\");\n+ }\n+ elsif (defined $query->{$check}) {\n log_debug(\"compare %s [%s] to [%s] (entry: %s)\",\n $check,\n $entry->{$check},\n"},{"id":"226646","messageId":"20130903173543.GC1050@sigill.intra.peff.net","threadId":"34760","inReplyTo":"87k3iyhqu5.fsf@lifelogs.com","subject":"Re: contrib/credential/netrc/git-credential-netrc: Use of uninitialized value in string","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-09-03T17:35:44Z","receivedAt":"2013-09-03T17:35:44Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Sep 03, 2013 at 11:23:14AM -0400, Ted Zlatanov wrote:\n\n> Yes, you're right. Something like the following (untested) could work\n> and does the wildcards, which I will make into a proper patch and test\n> if it looks OK to you.\n> \n> Ted\n> \n> diff --git a/contrib/credential/netrc/git-credential-netrc b/contrib/credential/netrc/git-credential-netrc\n> index 6c51c43..13e537b 100755\n> --- a/contrib/credential/netrc/git-credential-netrc\n> +++ b/contrib/credential/netrc/git-credential-netrc\n> @@ -369,7 +369,10 @@ sub find_netrc_entry {\n> {\n> my $entry_text = join ', ', map { \"$_=$entry->{$_}\" } keys %$entry;\n> foreach my $check (sort keys %$query) {\n> - if (defined $query->{$check}) {\n> + if (!defined $entry->{$check}) {\n> + log_debug(\"OK: entry has no $check token, so any value satisfies check $check\");\n> + }\n> + elsif (defined $query->{$check}) {\n> log_debug(\"compare %s [%s] to [%s] (entry: %s)\",\n> $check,\n> $entry->{$check},\n\nYeah, that makes sense to me (and is basically what the credential-cache\nand credential-store helpers do internally). Thanks for working on this.\n\n-Peff\n"},{"id":"228612","messageId":"87vc17x243.fsf@flea.lifelogs.com","threadId":"34760","inReplyTo":"20130903173543.GC1050@sigill.intra.peff.net","subject":"Re: contrib/credential/netrc/git-credential-netrc: Use of uninitialized value in string","fromName":"Ted Zlatanov","fromEmail":"tzz@lifelogs.com","sentAt":"2013-10-08T14:34:52Z","receivedAt":"2013-10-08T14:34:52Z","isPatch":false,"sender":{"key":"tzz@lifelogs.com","avatar":"https://avatars.githubusercontent.com/u/67764?v=4"},"body":"On Tue, 3 Sep 2013 13:35:44 -0400 Jeff King wrote: \n\nJK> On Tue, Sep 03, 2013 at 11:23:14AM -0400, Ted Zlatanov wrote:\n>> Yes, you're right. Something like the following (untested) could work\n>> and does the wildcards, which I will make into a proper patch and test\n>> if it looks OK to you.\n...\nJK> Yeah, that makes sense to me (and is basically what the credential-cache\nJK> and credential-store helpers do internally). Thanks for working on this.\n\nSorry for the delay, patch posted.\n\nTed\n"}]}