{"thread":{"id":"56693","subject":"[BUG] credential wildcard does not match hostnames containing an underscore","startedAt":"2021-10-12T14:25:30Z","lastAt":"2021-10-14T11:44:00Z","messageCount":17,"participants":["Alex Waite","Junio C Hamano","Jeff King","brian m. carlson","Aaron Schrab","Philip Oakley"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"438562","messageId":"28ff3572-1819-4e27-a46d-358eddd46e45@www.fastmail.com","threadId":"56693","inReplyTo":null,"subject":"[BUG] credential wildcard does not match hostnames containing an underscore","fromName":"Alex Waite","fromEmail":"alex@waite.eu","sentAt":"2021-10-12T14:25:04Z","receivedAt":"2021-10-12T14:25:30Z","isPatch":false,"sender":{"key":"alex@waite.eu","avatar":null},"body":"Hey Everyone,\n\nThis is my first time reporting a bug to this list. I attempted to follow the direction on the git website [1], and have followed the template provided by \"git bugreport\".\n\nIf my report should be formatted differently, or reported elsewhere, please let me know. :-)\n\n---Alex\n\n[1] https://git-scm.com/community\n\n-------\n\nWhat did you do before the bug happened? (Steps to reproduce your issue)\n\n I configured my ~/.gitconfig so that git credentials invoke a helper for a\n subdomain using wildcards. For example:\n\n [credential \"https://*.example.com\"]\n helper = \"/usr/local/bin/custom_helper\"\n\n This works for all tested subdomains /except/ for those which contain an\n underscore.\n\n authenticates without prompting:\n git clone https://testA.example.com\n git clone https://test-b.example.com\n\n prompts for authentication:\n git clone https://test_c.example.com\n\n\nWhat did you expect to happen? (Expected behavior)\n\n I expected the pattern matching to work for all resolved URLs.\n\n\nWhat happened instead? (Actual behavior)\n\n It does not match URLs which contain an underscore.\n\n\nWhat's different between what you expected and what actually happened?\n\n It only matches URLs which do not contain an underscore.\n\n\nAnything else you want to add:\n\n If I don't use pattern matching, and instead state the URL explicitly in\n ~/.gitconfig, it works as expected. For example, the following works:\n\n [credential \"https://test_c.example.com\"]\n helper = \"/usr/local/bin/custom_helper\"\n\n As part of writing this bug report, I learned that underscores are not valid\n DNS characters for hostnames (but are valid for other record types, which are\n largely irrelevant to git).\n\n What is notable is that git pattern matching enforces the spec more strictly\n than without pattern matching (and more strictly than the OS and every DNS\n server between my system and the authoritative DNS server).\n\n At minimum, git should be consistent with itself.\n\n As for which behavior is \"correct\", the question is whether git wishes to\n follow/enforce the spec tightly, or not get in the way of a real-world oddity\n that everything else seems to tolerate.\n\n This also likely affects patterns for http..*\n https://git-scm.com/docs/git-config#Documentation/git-config.txt-httplturlgt\n\n"},{"id":"438580","messageId":"xmqqk0ii3zl6.fsf@gitster.g","threadId":"56693","inReplyTo":"28ff3572-1819-4e27-a46d-358eddd46e45@www.fastmail.com","subject":"Re: [BUG] credential wildcard does not match hostnames containing an underscore","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-10-12T17:47:01Z","receivedAt":"2021-10-12T17:47:05Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Alex Waite\" writes:\n\n> This works for all tested subdomains /except/ for those which contain an\n> underscore.\n>\n> authenticates without prompting:\n> git clone https://testA.example.com\n> git clone https://test-b.example.com\n>\n> prompts for authentication:\n> git clone https://test_c.example.com\n\nHmph, given that hostnames cannot have '_' (cf. RFC1123 2.1 \"Host\nNames and Numbers\", for example), the third URL seems invalid. Is\nthis even a bug?\n\n\n"},{"id":"438581","messageId":"2883c3a9-a44f-4b24-acac-5ed573319d27@www.fastmail.com","threadId":"56693","inReplyTo":"xmqqk0ii3zl6.fsf@gitster.g","subject":"Re: [BUG] credential wildcard does not match hostnames containing an underscore","fromName":"Alex Waite","fromEmail":"alex@waite.eu","sentAt":"2021-10-12T18:00:55Z","receivedAt":"2021-10-12T18:01:18Z","isPatch":false,"sender":{"key":"alex@waite.eu","avatar":null},"body":"Thanks for the response. :-)\n\n> Hmph, given that hostnames cannot have '_' (cf. RFC1123 2.1 \"Host\n> Names and Numbers\", for example), the third URL seems invalid. Is\n> this even a bug?\n\nThat is a fair question, and I do acknowledge that later on in my bug report (where I provide some additional information).\n\nThe core issue, IMO, is that git is not consistent with itself. I can write a static rule that will match (\"https://test_c.example.com\") but cannot write a pattern that will do so.\n\nFrom a user perspective, a URL containing an underscore in the hostname works for everything else (including all other git operations), but not with pattern matching. It took me a couple hours to figure out /why/, as GIT_TRACE does not provide debugging for how git config rules do (or don't) match patterns.\n\nSpecifying in the docs which characters are matched would have helped understand why it was behaving so.\n\nIn any case, I felt it was opaque, and was worth sharing.\n\n---Alex\n"},{"id":"438583","messageId":"xmqqfst63xno.fsf@gitster.g","threadId":"56693","inReplyTo":"2883c3a9-a44f-4b24-acac-5ed573319d27@www.fastmail.com","subject":"Re: [BUG] credential wildcard does not match hostnames containing an underscore","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-10-12T18:28:43Z","receivedAt":"2021-10-12T18:28:47Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Alex Waite\" writes:\n\n> Thanks for the response. :-)\n>\n>> Hmph, given that hostnames cannot have '_' (cf. RFC1123 2.1 \"Host\n>> Names and Numbers\", for example), the third URL seems invalid. Is\n>> this even a bug?\n>\n> That is a fair question, and I do acknowledge that later on in my bug report (where I provide some additional information).\n>\n> The core issue, IMO, is that git is not consistent with itself. I\n> can write a static rule that will match\n> (\"https://test_c.example.com\") but cannot write a pattern that\n> will do so.\n\nI do not know if that is avoidable.\n\nTo be able to match a concrete URL that came from the running\nsession with a pattern taken from the configuration, we'd need to do\nsome parsing to figure out which part of the URL matches the\nwildcard, and to be able to tell which substrings are \"parts\", there\nneeds some syntactic rule that says what constitutes a valid word.\n\nI guess that we could make them consistent by treating a literal as\na pattern that does not happen to have any wildcard and reject the\n\"test_c\" hostname the same way in both cases. I do not offhand know\nhow desirable such a change would be.\n\n\n"},{"id":"438591","messageId":"YWXzGeiUSMeq5Key@coredump.intra.peff.net","threadId":"56693","inReplyTo":"xmqqk0ii3zl6.fsf@gitster.g","subject":"Re: [BUG] credential wildcard does not match hostnames containing an underscore","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-10-12T20:42:01Z","receivedAt":"2021-10-12T20:42:04Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Oct 12, 2021 at 10:47:01AM -0700, Junio C Hamano wrote:\n\n> \"Alex Waite\" writes:\n> \n> > This works for all tested subdomains /except/ for those which contain an\n> > underscore.\n> >\n> > authenticates without prompting:\n> > git clone https://testA.example.com\n> > git clone https://test-b.example.com\n> >\n> > prompts for authentication:\n> > git clone https://test_c.example.com\n> \n> Hmph, given that hostnames cannot have '_' (cf. RFC1123 2.1 \"Host\n> Names and Numbers\", for example), the third URL seems invalid. Is\n> this even a bug?\n\nThat may be so for hostnames in general, but URLs seem to allow it. RFC\n3986 says:\n\n host = IP-literal / IPv4address / reg-name\n reg-name = *( unreserved / pct-encoded / sub-delims )\n unreserved = ALPHA / DIGIT / \"-\" / \".\" / \"_\" / \"~\"\n\nSo underscore is definitely allowed in the host portion. Our code\ncomplains during url_normalize(), in this code:\n\n if (allow_globs)\n spanned = strspn(url, URL_HOST_CHARS \"*\");\n else\n spanned = strspn(url, URL_HOST_CHARS);\n \n if (spanned < colon_ptr - url) {\n /* Host name has invalid characters */\n if (out_info) {\n out_info->url = NULL;\n out_info->err = _(\"invalid characters in host name\");\n }\n strbuf_release(&norm);\n return NULL;\n }\n\nbecause earlier we define URL_HOST_CHARS without underscore:\n\n #define URL_HOST_CHARS URL_ALPHADIGIT \".-[:]\" /* IPv6 literals need [:] */\n\nI'm not sure why, given that this otherwise seems to match according to\nthe rfc. This code comes from 3402a8dc48 (config: add helper to\nnormalize and match URLs, 2013-07-31), but there's no mention of\nunderscore there. Possibly it came from earlier rules (rfc1738, for\nexample, has a stricter grammar that allows only alphabit and dashes).\n\nI can't imagine it would cause any problems to allow it here (as noted,\nwe're perfectly happy to use the name in other contexts, and I don't\nthink there any syntactic gotchas here).\n\nAdding \"_\" to that #define does make it work as expected.\n\n-Peff\n"},{"id":"438592","messageId":"YWXz+eFDxElPbZUF@coredump.intra.peff.net","threadId":"56693","inReplyTo":"2883c3a9-a44f-4b24-acac-5ed573319d27@www.fastmail.com","subject":"Re: [BUG] credential wildcard does not match hostnames containing an underscore","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-10-12T20:45:45Z","receivedAt":"2021-10-12T20:45:47Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Oct 12, 2021 at 08:00:55PM +0200, Alex Waite wrote:\n\n> From a user perspective, a URL containing an underscore in the\n> hostname works for everything else (including all other git\n> operations), but not with pattern matching. It took me a couple hours\n> to figure out /why/, as GIT_TRACE does not provide debugging for how\n> git config rules do (or don't) match patterns.\n\nOne thing I noticed here: url_normalize() does complain about parsing\nthis URL, but we don't propagate its error message to stderr. Perhaps we\nshould do so with warning(), but I'm a bit afraid that we may be relying\non this code to silently ignore invalid urls (i.e., showing the error\nwould cause some other innocuous cases to start issuing noisy and\nuseless warnings).\n\n-Peff\n"},{"id":"438594","messageId":"YWX13C7xsLcu+jZA@coredump.intra.peff.net","threadId":"56693","inReplyTo":"YWXzGeiUSMeq5Key@coredump.intra.peff.net","subject":"Re: [BUG] credential wildcard does not match hostnames containing an underscore","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-10-12T20:53:48Z","receivedAt":"2021-10-12T20:53:51Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Oct 12, 2021 at 04:42:01PM -0400, Jeff King wrote:\n\n> That may be so for hostnames in general, but URLs seem to allow it. RFC\n> 3986 says:\n> \n> host = IP-literal / IPv4address / reg-name\n> reg-name = *( unreserved / pct-encoded / sub-delims )\n> unreserved = ALPHA / DIGIT / \"-\" / \".\" / \"_\" / \"~\"\n> \n> So underscore is definitely allowed in the host portion. Our code\n> complains during url_normalize(), in this code:\n> \n> if (allow_globs)\n> spanned = strspn(url, URL_HOST_CHARS \"*\");\n> else\n> spanned = strspn(url, URL_HOST_CHARS);\n> \n> if (spanned < colon_ptr - url) {\n> /* Host name has invalid characters */\n> if (out_info) {\n> out_info->url = NULL;\n> out_info->err = _(\"invalid characters in host name\");\n> }\n> strbuf_release(&norm);\n> return NULL;\n> }\n> \n> because earlier we define URL_HOST_CHARS without underscore:\n> \n> #define URL_HOST_CHARS URL_ALPHADIGIT \".-[:]\" /* IPv6 literals need [:] */\n> \n> I'm not sure why, given that this otherwise seems to match according to\n> the rfc. This code comes from 3402a8dc48 (config: add helper to\n> normalize and match URLs, 2013-07-31), but there's no mention of\n> underscore there. Possibly it came from earlier rules (rfc1738, for\n> example, has a stricter grammar that allows only alphabit and dashes).\n\nSorry, I meant to cc the author of 3402a8dc48, which I've now done. It's\nbeen a while, but maybe he remembers something (I couldn't find anything\ndigging in the archive, either).\n\n-Peff\n"},{"id":"438596","messageId":"YWX6OkJANJGN0RnT@coredump.intra.peff.net","threadId":"56693","inReplyTo":"YWX13C7xsLcu+jZA@coredump.intra.peff.net","subject":"[PATCH] urlmatch: add underscore to URL_HOST_CHARS","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-10-12T21:12:26Z","receivedAt":"2021-10-12T21:12:29Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Oct 12, 2021 at 04:53:48PM -0400, Jeff King wrote:\n\n> > because earlier we define URL_HOST_CHARS without underscore:\n> > \n> > #define URL_HOST_CHARS URL_ALPHADIGIT \".-[:]\" /* IPv6 literals need [:] */\n> > \n> > I'm not sure why, given that this otherwise seems to match according to\n> > the rfc. This code comes from 3402a8dc48 (config: add helper to\n> > normalize and match URLs, 2013-07-31), but there's no mention of\n> > underscore there. Possibly it came from earlier rules (rfc1738, for\n> > example, has a stricter grammar that allows only alphabit and dashes).\n> \n> Sorry, I meant to cc the author of 3402a8dc48, which I've now done. It's\n> been a while, but maybe he remembers something (I couldn't find anything\n> digging in the archive, either).\n\nAbsent any other input, I'd propose the patch below.\n\n-- >8 --\nSubject: urlmatch: add underscore to URL_HOST_CHARS\n\nWhen parsing a URL to normalize it, we allow hostnames to contain only\ndot (\".\") or dash (\"-\"), plus brackets and colons for IPv6 literals.\nThis matches the old URL standard in RFC 1738, which says:\n\n host = hostname | hostnumber\n hostname = *[ domainlabel \".\" ] toplabel\n domainlabel = alphadigit | alphadigit *[ alphadigit | \"-\" ] alphadigit\n\nBut this was later updated by RFC 3986, which is more liberal:\n\n host = IP-literal / IPv4address / reg-name\n reg-name = *( unreserved / pct-encoded / sub-delims )\n unreserved = ALPHA / DIGIT / \"-\" / \".\" / \"_\" / \"~\"\n\nWhile names with underscore in them are not common and possibly violate\nsome DNS rules, they do work in practice, and we will happily contact\nthem over http://, git://, or ssh://. It seems odd to ignore them for\npurposes of URL matching, especially when the URL RFC seems to allow\nthem.\n\nThere shouldn't be any downside here. It's not a syntactically\nsignificant character in a URL, so we won't be confused about parsing;\nwe'd have simply rejected such a URL previously (the test here checks\nthe url code directly, but the obvious user-visible effect would be\nfailing to match credential.http://foo_bar.example.com.helper, or\nsimilar config in http..*).\n\nArguably we'd want to allow tilde (\"~\") here, too. There's likewise\nprobably no downside, but I didn't add it simply because it seems like\nan even less likely character to appear in a hostname.\n\nReported-by: Alex Waite \nSigned-off-by: Jeff King \n---\nI'm on the fence regarding \"~\". I didn't actually test that things like\ncurl even allow it (I did for underscore by creating a throwaway DNS\nname).\n\n t/t0110-urlmatch-normalization.sh | 2 +-\n urlmatch.c | 2 +-\n 2 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t0110-urlmatch-normalization.sh b/t/t0110-urlmatch-normalization.sh\nindex f99529d838..4dc9fecf72 100755\n--- a/t/t0110-urlmatch-normalization.sh\n+++ b/t/t0110-urlmatch-normalization.sh\n@@ -47,7 +47,7 @@ test_expect_success 'url authority' '\n \ttest-tool urlmatch-normalization \"scheme://@host\" &&\n \ttest-tool urlmatch-normalization \"scheme://%00@host\" &&\n \t! test-tool urlmatch-normalization \"scheme://%%@host\" &&\n-\t! test-tool urlmatch-normalization \"scheme://host_\" &&\n+\ttest-tool urlmatch-normalization \"scheme://host_\" &&\n \ttest-tool urlmatch-normalization \"scheme://user:pass@host/\" &&\n \ttest-tool urlmatch-normalization \"scheme://@host/\" &&\n \ttest-tool urlmatch-normalization \"scheme://host/\" &&\ndiff --git a/urlmatch.c b/urlmatch.c\nindex 33a2ccd306..03ad3f30a9 100644\n--- a/urlmatch.c\n+++ b/urlmatch.c\n@@ -5,7 +5,7 @@\n #define URL_DIGIT \"0123456789\"\n #define URL_ALPHADIGIT URL_ALPHA URL_DIGIT\n #define URL_SCHEME_CHARS URL_ALPHADIGIT \"+.-\"\n-#define URL_HOST_CHARS URL_ALPHADIGIT \".-[:]\" /* IPv6 literals need [:] */\n+#define URL_HOST_CHARS URL_ALPHADIGIT \".-_[:]\" /* IPv6 literals need [:] */\n #define URL_UNSAFE_CHARS \" <>\\\"%{}|\\\\^`\" /* plus 0x00-0x1F,0x7F-0xFF */\n #define URL_GEN_RESERVED \":/?#[]@\"\n #define URL_SUB_RESERVED \"!$&'()*+,;=\"\n-- \n2.33.0.1387.g4e339dd0af\n\n"},{"id":"438597","messageId":"YWX6PJrjgp6rHZu/@camp.crustytoothpaste.net","threadId":"56693","inReplyTo":"28ff3572-1819-4e27-a46d-358eddd46e45@www.fastmail.com","subject":"Re: [BUG] credential wildcard does not match hostnames containing an underscore","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2021-10-12T21:12:28Z","receivedAt":"2021-10-12T21:13:10Z","isPatch":false,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2021-10-12 at 14:25:04, Alex Waite wrote:\n> What did you do before the bug happened? (Steps to reproduce your issue)\n> \n> I configured my ~/.gitconfig so that git credentials invoke a helper for a\n> subdomain using wildcards. For example:\n> \n> [credential \"https://*.example.com\"]\n> helper = \"/usr/local/bin/custom_helper\"\n> \n> This works for all tested subdomains /except/ for those which contain an\n> underscore.\n> \n> authenticates without prompting:\n> git clone https://testA.example.com\n> git clone https://test-b.example.com\n> \n> prompts for authentication:\n> git clone https://test_c.example.com\n> \n> \n> What did you expect to happen? (Expected behavior)\n> \n> I expected the pattern matching to work for all resolved URLs.\n\nAs mentioned below and elsewhere in this thread, this isn't a valid\nhostname, and as a result, this isn't even a valid URL according to RFC\n3986 unless you intended it to be resolved in a system other than DNS.\n\nI don't personally see a reason to accept locally specified hostnames\n(e.g., in a hosts file) which don't conform to RFC 1123 or which\notherwise don't conform to the DNS standards for hostnames, but perhaps\nothers can see a good reason to do so.\n\n> Anything else you want to add:\n> \n> If I don't use pattern matching, and instead state the URL explicitly in\n> ~/.gitconfig, it works as expected. For example, the following works:\n> \n> [credential \"https://test_c.example.com\"]\n> helper = \"/usr/local/bin/custom_helper\"\n> \n> As part of writing this bug report, I learned that underscores are not valid\n> DNS characters for hostnames (but are valid for other record types, which are\n> largely irrelevant to git).\n> \n> What is notable is that git pattern matching enforces the spec more strictly\n> than without pattern matching (and more strictly than the OS and every DNS\n> server between my system and the authoritative DNS server).\n> \n> At minimum, git should be consistent with itself.\n> \n> As for which behavior is \"correct\", the question is whether git wishes to\n> follow/enforce the spec tightly, or not get in the way of a real-world oddity\n> that everything else seems to tolerate.\n\nThere are a variety of systems which won't accept such a hostname, so I\nthink at best we should reject such hostnames altogether and prevent\nthis from working at all, since they are likely to be subtly broken in a\nvariety of ways and we won't want to try to fix all of the cases in\nwhich things are broken. To me, this appears to be simply a case where\nwe should improve error handling.\n-- \nbrian m. carlson (he/him or they/them)\nToronto, Ontario, CA\n"},{"id":"438599","messageId":"YWX8d/VTrkOz5tga@camp.crustytoothpaste.net","threadId":"56693","inReplyTo":"YWXzGeiUSMeq5Key@coredump.intra.peff.net","subject":"Re: [BUG] credential wildcard does not match hostnames containing an underscore","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2021-10-12T21:21:59Z","receivedAt":"2021-10-12T21:22:05Z","isPatch":false,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2021-10-12 at 20:42:01, Jeff King wrote:\n> On Tue, Oct 12, 2021 at 10:47:01AM -0700, Junio C Hamano wrote:\n> \n> > \"Alex Waite\" writes:\n> > \n> > > This works for all tested subdomains /except/ for those which contain an\n> > > underscore.\n> > >\n> > > authenticates without prompting:\n> > > git clone https://testA.example.com\n> > > git clone https://test-b.example.com\n> > >\n> > > prompts for authentication:\n> > > git clone https://test_c.example.com\n> > \n> > Hmph, given that hostnames cannot have '_' (cf. RFC1123 2.1 \"Host\n> > Names and Numbers\", for example), the third URL seems invalid. Is\n> > this even a bug?\n> \n> That may be so for hostnames in general, but URLs seem to allow it. RFC\n> 3986 says:\n> \n> host = IP-literal / IPv4address / reg-name\n> reg-name = *( unreserved / pct-encoded / sub-delims )\n> unreserved = ALPHA / DIGIT / \"-\" / \".\" / \"_\" / \"~\"\n\nThat's what the schema says. The text says this:\n\n A host identified by a registered name is a sequence of characters\n usually intended for lookup within a locally defined host or service\n name registry, though the URI's scheme-specific semantics may require\n that a specific registry (or fixed name table) be used instead. The\n most common name registry mechanism is the Domain Name System (DNS).\n A registered name intended for lookup in the DNS uses the syntax\n defined in Section 3.5 of [RFC1034] and Section 2.1 of [RFC1123].\n\nThose RFCs disallow the underscore.\n\nIf we plan to allow names that are not registered in the DNS, we should\nclearly specify what those are and document how they work in conjunction\nwith libcurl (which presumably does a DNS lookup on them). It's my\nguess that there are going to be system resolvers which are not going to\naccept this syntax in getaddrinfo and as a result, we're going to have\nvarious breakage across systems if we try to accept this.\n\nI'm happy to put in a change to reject these hostnames altogether, but I\nwon't get to it before Friday.\n-- \nbrian m. carlson (he/him or they/them)\nToronto, Ontario, CA\n"},{"id":"438600","messageId":"YWX+6OgzN4CDzomO@coredump.intra.peff.net","threadId":"56693","inReplyTo":"YWX8d/VTrkOz5tga@camp.crustytoothpaste.net","subject":"Re: [BUG] credential wildcard does not match hostnames containing an underscore","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-10-12T21:32:24Z","receivedAt":"2021-10-12T21:32:26Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Oct 12, 2021 at 09:21:59PM +0000, brian m. carlson wrote:\n\n> > That may be so for hostnames in general, but URLs seem to allow it. RFC\n> > 3986 says:\n> > \n> > host = IP-literal / IPv4address / reg-name\n> > reg-name = *( unreserved / pct-encoded / sub-delims )\n> > unreserved = ALPHA / DIGIT / \"-\" / \".\" / \"_\" / \"~\"\n> \n> That's what the schema says. The text says this:\n> \n> A host identified by a registered name is a sequence of characters\n> usually intended for lookup within a locally defined host or service\n> name registry, though the URI's scheme-specific semantics may require\n> that a specific registry (or fixed name table) be used instead. The\n> most common name registry mechanism is the Domain Name System (DNS).\n> A registered name intended for lookup in the DNS uses the syntax\n> defined in Section 3.5 of [RFC1034] and Section 2.1 of [RFC1123].\n> \n> Those RFCs disallow the underscore.\n\nThanks, I skimmed looking for some resolution to this mismatch, but\ndidn't find that paragraph.\n\n> If we plan to allow names that are not registered in the DNS, we should\n> clearly specify what those are and document how they work in conjunction\n> with libcurl (which presumably does a DNS lookup on them). It's my\n> guess that there are going to be system resolvers which are not going to\n> accept this syntax in getaddrinfo and as a result, we're going to have\n> various breakage across systems if we try to accept this.\n\nI don't think this makes anything worse. Either the underscore works or\nit doesn't for general use on your system. This just means we'll allow\nhttp..* config for it.\n\nAnd it does indeed work fine on my system, via DNS. My stub resolver is\nglibc, and curl itself is fine with it. The server side answering the\nquery was djbdns (tinydns, with dnscache as a recursive resolver in\nbetween). I could believe that other implementations may be more strict,\nthough.\n\n> I'm happy to put in a change to reject these hostnames altogether, but I\n> won't get to it before Friday.\n\nIMHO _that_ is the thing that will produce breakage. People who are not\nusing URL-specific config but are happily using foo_bar.example.com will\nnow get a failure for something that used to work.\n\n-Peff\n"},{"id":"438604","messageId":"YWYCh3+37d27QNjW@camp.crustytoothpaste.net","threadId":"56693","inReplyTo":"YWX+6OgzN4CDzomO@coredump.intra.peff.net","subject":"Re: [BUG] credential wildcard does not match hostnames containing an underscore","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2021-10-12T21:48:33Z","receivedAt":"2021-10-12T21:48:40Z","isPatch":false,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2021-10-12 at 21:32:24, Jeff King wrote:\n> On Tue, Oct 12, 2021 at 09:21:59PM +0000, brian m. carlson wrote:\n> > I'm happy to put in a change to reject these hostnames altogether, but I\n> > won't get to it before Friday.\n> \n> IMHO _that_ is the thing that will produce breakage. People who are not\n> using URL-specific config but are happily using foo_bar.example.com will\n> now get a failure for something that used to work.\n\nThere's a well-known bug on Tumblr, where it allocated hostnames for\nusers that happened to start or end with a dash, which is not allowed.\nThis worked great on Windows systems, which don't care, but every Unix\nsystem was broken.\n\nWhen we decide to allow this particular case, we end up with the problem\nthat people won't see consistent behavior across systems and tools. It\nmay be that libgit2 rejects this, or Git LFS, or some other tool in the\necosystem, and then we'll have people complaining that \"Well, Git\naccepts it, so why don't you?\" which I am not eager to see. I, for\nexample, have absolutely zero control over the URL parsing library\nthat's used in Git LFS, and the Go team has demonstrated that they don't\ncare one bit about supporting Git-related tooling. That doesn't even\ninclude a variety of proprietary Unix systems that might have different\nrules or resolvers.\n\nI am also not eager to see additional bug reports for this case that\nwill need to be fixed under the precedent that we accepted a patch to\nfix it before. If there's a concern that rejecting these hostnames\naltogether would break existing users, then we can just do nothing, and\ntell users that their syntax is not valid and they need to fix their\nhostnames. This rule has been documented since before ISO standardized\nC, so it shouldn't be new to anyone deploying systems or DNS.\n\nSo I'm fine with doing nothing, or rejecting these hostnames, but not\nallowing more lenient syntax, because it will probably be broken\nsomewhere and we (or someone else in the ecosystem) will have to deal\nwith it again down the line.\n-- \nbrian m. carlson (he/him or they/them)\nToronto, Ontario, CA\n"},{"id":"438605","messageId":"YWYEXFkbg1LiVUfn@coredump.intra.peff.net","threadId":"56693","inReplyTo":"YWYCh3+37d27QNjW@camp.crustytoothpaste.net","subject":"Re: [BUG] credential wildcard does not match hostnames containing an underscore","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-10-12T21:55:40Z","receivedAt":"2021-10-12T21:55:43Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Oct 12, 2021 at 09:48:33PM +0000, brian m. carlson wrote:\n\n> I am also not eager to see additional bug reports for this case that\n> will need to be fixed under the precedent that we accepted a patch to\n> fix it before. If there's a concern that rejecting these hostnames\n> altogether would break existing users, then we can just do nothing, and\n> tell users that their syntax is not valid and they need to fix their\n> hostnames. This rule has been documented since before ISO standardized\n> C, so it shouldn't be new to anyone deploying systems or DNS.\n> \n> So I'm fine with doing nothing, or rejecting these hostnames, but not\n> allowing more lenient syntax, because it will probably be broken\n> somewhere and we (or someone else in the ecosystem) will have to deal\n> with it again down the line.\n\nFWIW, I'm fine with doing nothing. But it will still come back further\ndown the line, because it _does_ work most of the way, and has for a\nlong time.\n\n-Peff\n"},{"id":"438606","messageId":"YWYE2LZp/EfoBpN/@camp.crustytoothpaste.net","threadId":"56693","inReplyTo":"YWYCh3+37d27QNjW@camp.crustytoothpaste.net","subject":"Re: [BUG] credential wildcard does not match hostnames containing an underscore","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2021-10-12T21:57:44Z","receivedAt":"2021-10-12T21:57:53Z","isPatch":false,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2021-10-12 at 21:48:33, brian m. carlson wrote:\n> On 2021-10-12 at 21:32:24, Jeff King wrote:\n> > On Tue, Oct 12, 2021 at 09:21:59PM +0000, brian m. carlson wrote:\n> > > I'm happy to put in a change to reject these hostnames altogether, but I\n> > > won't get to it before Friday.\n> > \n> > IMHO _that_ is the thing that will produce breakage. People who are not\n> > using URL-specific config but are happily using foo_bar.example.com will\n> > now get a failure for something that used to work.\n> \n> There's a well-known bug on Tumblr, where it allocated hostnames for\n> users that happened to start or end with a dash, which is not allowed.\n> This worked great on Windows systems, which don't care, but every Unix\n> system was broken.\n> \n> When we decide to allow this particular case, we end up with the problem\n> that people won't see consistent behavior across systems and tools. It\n> may be that libgit2 rejects this, or Git LFS, or some other tool in the\n> ecosystem, and then we'll have people complaining that \"Well, Git\n> accepts it, so why don't you?\" which I am not eager to see. I, for\n> example, have absolutely zero control over the URL parsing library\n> that's used in Git LFS, and the Go team has demonstrated that they don't\n> care one bit about supporting Git-related tooling. That doesn't even\n> include a variety of proprietary Unix systems that might have different\n> rules or resolvers.\n> \n> I am also not eager to see additional bug reports for this case that\n> will need to be fixed under the precedent that we accepted a patch to\n> fix it before. If there's a concern that rejecting these hostnames\n> altogether would break existing users, then we can just do nothing, and\n> tell users that their syntax is not valid and they need to fix their\n> hostnames. This rule has been documented since before ISO standardized\n> C, so it shouldn't be new to anyone deploying systems or DNS.\n\nI also just checked, and RFC 5280 specifies the rules for RFC 1123\nregarding host names in certificates. So even if we did accept this, no\npublicly trusted CA could issue a certificate for such a domain, because\nto do so would be misissuance. So this at best could help people who\nare either using plain HTTP or an internal CA using broken tools,\nneither of which I think argue in favor of supporting this.\n-- \nbrian m. carlson (he/him or they/them)\nToronto, Ontario, CA\n"},{"id":"438610","messageId":"YWYLRvUEkoudH5n0@pug.qqx.org","threadId":"56693","inReplyTo":"YWYE2LZp/EfoBpN/@camp.crustytoothpaste.net","subject":"Re: [BUG] credential wildcard does not match hostnames containing an underscore","fromName":"Aaron Schrab","fromEmail":"aaron@schrab.com","sentAt":"2021-10-12T22:25:10Z","receivedAt":"2021-10-12T22:32:57Z","isPatch":false,"sender":{"key":"aaron@schrab.com","avatar":"https://avatars.githubusercontent.com/u/39620?v=4"},"body":"At 21:57 +0000 12 Oct 2021, \"brian m. carlson\" wrote:\n>I also just checked, and RFC 5280 specifies the rules for RFC 1123\n>regarding host names in certificates. So even if we did accept this, no\n>publicly trusted CA could issue a certificate for such a domain, because\n>to do so would be misissuance. So this at best could help people who\n>are either using plain HTTP or an internal CA using broken tools,\n>neither of which I think argue in favor of supporting this.\n\nOr people using a wildcard certificate.\n"},{"id":"438664","messageId":"c84ef8d0-0ed7-4267-9952-d9a2bc053ad6@www.fastmail.com","threadId":"56693","inReplyTo":"YWYLRvUEkoudH5n0@pug.qqx.org","subject":"Re: [BUG] credential wildcard does not match hostnames containing an underscore","fromName":"Alex Waite","fromEmail":"alex@waite.eu","sentAt":"2021-10-13T16:21:16Z","receivedAt":"2021-10-13T16:21:41Z","isPatch":false,"sender":{"key":"alex@waite.eu","avatar":null},"body":"2021.10.13, aaron@schrab.com:\n> At 21:57 +0000 12 Oct 2021, \"brian m. carlson\" \n> wrote:\n>>I also just checked, and RFC 5280 specifies the rules for RFC 1123\n>>regarding host names in certificates. So even if we did accept this, no\n>>publicly trusted CA could issue a certificate for such a domain, because\n>>to do so would be misissuance. So this at best could help people who\n>>are either using plain HTTP or an internal CA using broken tools,\n>>neither of which I think argue in favor of supporting this.\n>\n> Or people using a wildcard certificate.\n\nI didn't expect to kick off such a big discussion with this bug. ;-)\n\nJust to add my 2 cents: the environment where I bumped into this is using a wildcard cert. It's among a fleet of (busy) internal domains that host data for scientific analysis. Lots of tools talk with those domains, and this is the first time I've been made aware of something struggling with the domains containing an underscore.\n\nI'm not saying whether git should or should not change its behavior here. But the above is why I was surprised to learn the an underscore is not valid. Because everything (DNS servers, dig, Apache, and git itself) seems to happily use it.\n\nIn my view, the primary bug is how difficult it was to debug what was going wrong. This is most easily solved by improving the git docs to specify which characters will be matched. Even better if GIT_TRACE (or something similar) can inform/warn the user about matching.\n\nIn any case, thanks everyone for looking into this. :-)\n\n---Alex\n"},{"id":"438752","messageId":"6ea198bf-6013-6cdf-3e56-7574975b1d7a@iee.email","threadId":"56693","inReplyTo":"c84ef8d0-0ed7-4267-9952-d9a2bc053ad6@www.fastmail.com","subject":"Re: [BUG] credential wildcard does not match hostnames containing an underscore","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.email","sentAt":"2021-10-14T11:43:55Z","receivedAt":"2021-10-14T11:44:00Z","isPatch":false,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"On 13/10/2021 17:21, Alex Waite wrote:\n> In my view, the primary bug is how difficult it was to debug what was going wrong. This is most easily solved by improving the git docs to specify which characters will be matched. Even better if GIT_TRACE (or something similar) can inform/warn the user about matching.\nIt did sound to me that the documentation route was a way out of the\nconfusing situation.\n--\nPhilip\n"}]}