Re: Host header validation
On 10/04/17 07:42, Katya Todorova wrote: > On Wed, Apr 5, 2017 at 3:57 PM, Mark Thomas wrote: > >> On 05/04/2017 07:50, Katya Todorova wrote: >> >>> Applied. Many thanks. If you'd like to work on this further then can I suggest you take a look at Konstantin's comments: http://markmail.org/message/vp5voob7elspflax >>> >>> >>> I looked at the comments and it seems there are things to be clarified >>> before going in this direction: >>> - should we introduce a flag for turn on/off validation and in which cases >>> >> >> Currently, the validation isn't used at the point where the header is >> parsed. >> >> I'd prefer not to add an option to disable this check. It just seems like >> the wrong thing to do. >> >> I'm currently thinking that we could add the validation and log failures >> (rather than return a 400 response) warning that a future release will >> start rejecting the requests. That should prompt users to contact us with >> any false positives. > > > I think logging failure instead of returning 400 at that point is a good > approach. How much time do you think would be enough to simply log before > enforcing these checks? (asking out of curiosity) Also, would it be > possible to have this functionality in tomcat 7/8? Probably 3 to 6 months. A lot will depend on the volume of the false positive reports we get and whether or not this identifies any broken clients that are sending invalid host headers. My guess is that back-porting to 8.5.x is likely whereas back-porting to 7.0.x might happen depending on what we see with 9.0.x and 8.5.x. >> - zone id support in IPv6 addresses >>> >> >> - IPvFuture support (for this one Konstantin has already proposed to be >>> postponed for a while) >>> >> >> Looking at the spec, I think we can parse IPvFuture now. We should >> probably log any IPvFuture values with a request to report the use case to >> us so we can update the parser to handle specific instances rather tan the >> general case. >> >> If you think this is the right time to work on the first two, let me know >>> and I can prepare a patch. >>> >> >> I think that would be great. BUT. That isn't my decision to make. One of >> the key principles of the Apache Way is that contributors choose what they >> want to work on. There isn't a project leader or a management team >> assigning tasks. If you want to work on this then absolutely, go ahead. If >> there is some other aspect you'd rather be working on then by all means >> work on that. >> >> The Tomcat community is always willing to provide some pointers to >> suitable tasks where people new to Tomcat can get started but that >> shouldn't be see as assigning areas to work on. > > > Thank you for clarifying the process. I decided to look at these things > because they are related to what I've already done and would, in a way, > complete the effort around host parsing. I was asking about IPvFuture in > particular because it seems the specification will evolve and host parsing > code would need to be adapted accordingly, probably incrementally. While > the rest of the host definition seems pretty stable and in use. > That said, I'm not interested in particular in IPvFuture implementation - I > thought it would help me getting to know Tomcat better - if there's > anything else that is more important/urgent and is suitable for someone new > to Tomcat, please share some pointers. I don't see this as task assignment > but more as providing the context/priority information that I currently > lack. Great. I just wanted to make sure you - and any other potential contributors - didn't get the wrong impression. Kind regards, Mark > > Kind regards, > Katya > > Other possibilities are: >>> - performance improvements for the Host header validation - improving code coverage generally for any of the HTTP parsing code - any that attracts your interest >>> >>> >>> I'm looking at the code coverage and will take a look at host validation >>> performance. >>> >> >> Fantastic. (With the caveat of you'd rather do something else then that >> would be fantastic too.) >> >> >> Kind regards, >> >> Mark >> >> - >> To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org >> For additional commands, e-mail: dev-h...@tomcat.apache.org >> >> > - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: Host header validation
On Wed, Apr 5, 2017 at 3:57 PM, Mark Thomas wrote: > On 05/04/2017 07:50, Katya Todorova wrote: > >> >>> Applied. Many thanks. >>> >>> If you'd like to work on this further then can I suggest you take a look >>> at Konstantin's comments: >>> >>> http://markmail.org/message/vp5voob7elspflax >>> >> >> >> I looked at the comments and it seems there are things to be clarified >> before going in this direction: >> - should we introduce a flag for turn on/off validation and in which cases >> > > Currently, the validation isn't used at the point where the header is > parsed. > > I'd prefer not to add an option to disable this check. It just seems like > the wrong thing to do. > > I'm currently thinking that we could add the validation and log failures > (rather than return a 400 response) warning that a future release will > start rejecting the requests. That should prompt users to contact us with > any false positives. I think logging failure instead of returning 400 at that point is a good approach. How much time do you think would be enough to simply log before enforcing these checks? (asking out of curiosity) Also, would it be possible to have this functionality in tomcat 7/8? > - zone id support in IPv6 addresses >> > > - IPvFuture support (for this one Konstantin has already proposed to be >> postponed for a while) >> > > Looking at the spec, I think we can parse IPvFuture now. We should > probably log any IPvFuture values with a request to report the use case to > us so we can update the parser to handle specific instances rather tan the > general case. > > If you think this is the right time to work on the first two, let me know >> and I can prepare a patch. >> > > I think that would be great. BUT. That isn't my decision to make. One of > the key principles of the Apache Way is that contributors choose what they > want to work on. There isn't a project leader or a management team > assigning tasks. If you want to work on this then absolutely, go ahead. If > there is some other aspect you'd rather be working on then by all means > work on that. > > The Tomcat community is always willing to provide some pointers to > suitable tasks where people new to Tomcat can get started but that > shouldn't be see as assigning areas to work on. Thank you for clarifying the process. I decided to look at these things because they are related to what I've already done and would, in a way, complete the effort around host parsing. I was asking about IPvFuture in particular because it seems the specification will evolve and host parsing code would need to be adapted accordingly, probably incrementally. While the rest of the host definition seems pretty stable and in use. That said, I'm not interested in particular in IPvFuture implementation - I thought it would help me getting to know Tomcat better - if there's anything else that is more important/urgent and is suitable for someone new to Tomcat, please share some pointers. I don't see this as task assignment but more as providing the context/priority information that I currently lack. Kind regards, Katya Other possibilities are: >> >>> >>> - performance improvements for the Host header validation >>> >>> - improving code coverage generally for any of the HTTP parsing code >>> >>> - any that attracts your interest >>> >> >> >> I'm looking at the code coverage and will take a look at host validation >> performance. >> > > Fantastic. (With the caveat of you'd rather do something else then that > would be fantastic too.) > > > Kind regards, > > Mark > > - > To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org > For additional commands, e-mail: dev-h...@tomcat.apache.org > >
Re: Host header validation
On 05/04/2017 07:50, Katya Todorova wrote: Applied. Many thanks. If you'd like to work on this further then can I suggest you take a look at Konstantin's comments: http://markmail.org/message/vp5voob7elspflax I looked at the comments and it seems there are things to be clarified before going in this direction: - should we introduce a flag for turn on/off validation and in which cases Currently, the validation isn't used at the point where the header is parsed. I'd prefer not to add an option to disable this check. It just seems like the wrong thing to do. I'm currently thinking that we could add the validation and log failures (rather than return a 400 response) warning that a future release will start rejecting the requests. That should prompt users to contact us with any false positives. - zone id support in IPv6 addresses - IPvFuture support (for this one Konstantin has already proposed to be postponed for a while) Looking at the spec, I think we can parse IPvFuture now. We should probably log any IPvFuture values with a request to report the use case to us so we can update the parser to handle specific instances rather tan the general case. If you think this is the right time to work on the first two, let me know and I can prepare a patch. I think that would be great. BUT. That isn't my decision to make. One of the key principles of the Apache Way is that contributors choose what they want to work on. There isn't a project leader or a management team assigning tasks. If you want to work on this then absolutely, go ahead. If there is some other aspect you'd rather be working on then by all means work on that. The Tomcat community is always willing to provide some pointers to suitable tasks where people new to Tomcat can get started but that shouldn't be see as assigning areas to work on. Other possibilities are: - performance improvements for the Host header validation - improving code coverage generally for any of the HTTP parsing code - any that attracts your interest I'm looking at the code coverage and will take a look at host validation performance. Fantastic. (With the caveat of you'd rather do something else then that would be fantastic too.) Kind regards, Mark - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: Host header validation
> > Applied. Many thanks. > > If you'd like to work on this further then can I suggest you take a look > at Konstantin's comments: > > http://markmail.org/message/vp5voob7elspflax I looked at the comments and it seems there are things to be clarified before going in this direction: - should we introduce a flag for turn on/off validation and in which cases - zone id support in IPv6 addresses - IPvFuture support (for this one Konstantin has already proposed to be postponed for a while) If you think this is the right time to work on the first two, let me know and I can prepare a patch. Other possibilities are: > > - performance improvements for the Host header validation > > - improving code coverage generally for any of the HTTP parsing code > > - any that attracts your interest I'm looking at the code coverage and will take a look at host validation performance. Kind regards, Katya
Re: Host header validation
On 31/03/17 14:41, Mark Thomas wrote: > On 31/03/17 09:43, Katya Todorova wrote: >> I've created a separate pull request for leading zeros issue since I think >> it requires additional discussion whether to be submitted or not. Although >> this fix honors the specification, it leads to different behavior in case >> of IPv4 address and IPv6 address with IPv4 part. This may cause confusion: >> https://github.com/apache/tomcat/pull/49 > > Thanks for separating this out. It gives folks time to go and read the > specification and form an opinion. I've taken a look. The spec is clear that leading zeros are not permitted in IPv4 addresses as used in the authority component of a URI (or when the IPv4 address appears as part of the IPv6 address). However, I don't see the harm in allowing them and doing so makes the parsing/validation simpler and therefore faster. Therefore, I'm leaning towards being lenient and allowing leading zeros in IPv4 address components. Mark - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: Host header validation
On 31/03/17 09:43, Katya Todorova wrote: >> You can either create a pull request on github or create a Bugzilla >> issue and attach a patch. > > >> Mark >> > > I've created a separate pull request for leading zeros issue since I think > it requires additional discussion whether to be submitted or not. Although > this fix honors the specification, it leads to different behavior in case > of IPv4 address and IPv6 address with IPv4 part. This may cause confusion: > https://github.com/apache/tomcat/pull/49 Thanks for separating this out. It gives folks time to go and read the specification and form an opinion. > The rest of identified host parser issues are fixed here: > https://github.com/apache/tomcat/pull/48 Applied. Many thanks. If you'd like to work on this further then can I suggest you take a look at Konstantin's comments: http://markmail.org/message/vp5voob7elspflax Other possibilities are: - performance improvements for the Host header validation - improving code coverage generally for any of the HTTP parsing code - any that attracts your interest Kind regards, Mark - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: Host header validation
> You can either create a pull request on github or create a Bugzilla > issue and attach a patch. > Mark > I've created a separate pull request for leading zeros issue since I think it requires additional discussion whether to be submitted or not. Although this fix honors the specification, it leads to different behavior in case of IPv4 address and IPv6 address with IPv4 part. This may cause confusion: https://github.com/apache/tomcat/pull/49 The rest of identified host parser issues are fixed here: https://github.com/apache/tomcat/pull/48 Kind regards, Katya
Re: Host header validation
On 29/03/17 15:16, Katya Todorova wrote: >> >> I recommend using the code coverage reports as a guide. >> >> https://ci.apache.org/projects/tomcat/tomcat9/coverage/ > > >> >> and add test cases if they increase code coverage. Hmm. It looks like >> there is some low hanging fruit in the parsing code to improve coverage. >> >> You can run the tests with code coverage locally too. >> > > Thank you for the guidance, I'll add only these test cases that increases > code coverage. > > >> Other than that, the usual advice applies. Test limits, either side of >> limits, etc >> >> Generally, I try and reduce copy and paste as much as possible. @RunWith >> has been really good for that. There is probably scope to use it more >> throughout the Tomcat tests. >> >> Kind regards, >> >> Mark >> >> > Is there an issue tracking this change where I can attach the patch when > I'm ready? Or should I do so here? You can either create a pull request on github or create a Bugzilla issue and attach a patch. Mark - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: Host header validation
> > I recommend using the code coverage reports as a guide. > > https://ci.apache.org/projects/tomcat/tomcat9/coverage/ > > and add test cases if they increase code coverage. Hmm. It looks like > there is some low hanging fruit in the parsing code to improve coverage. > > You can run the tests with code coverage locally too. > Thank you for the guidance, I'll add only these test cases that increases code coverage. > Other than that, the usual advice applies. Test limits, either side of > limits, etc > > Generally, I try and reduce copy and paste as much as possible. @RunWith > has been really good for that. There is probably scope to use it more > throughout the Tomcat tests. > > Kind regards, > > Mark > > > - > To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org > For additional commands, e-mail: dev-h...@tomcat.apache.org > > Is there an issue tracking this change where I can attach the patch when I'm ready? Or should I do so here? Thank you, Katya
Re: Host header validation
On 29/03/17 07:06, Katya Todorova wrote: > On Tue, Mar 28, 2017 at 5:45 PM, Mark Thomas wrote: > >> On 28/03/17 15:23, Katya Todorova wrote: >>> Hi, r1787662 adds Host header validation along with a fair number of unit >> tests. It includes a performance test which indicates - on my machine at least - that the performance impact is in the noise. I'd like to see better performance for full IPv6 addresses but the current code looks to be acceptable. The validation is not yet integrated into the request processing. My primary reason for not integrating it is that it will trigger a 400 response if the header is invalid and I don't want to incorrectly reject valid headers. Therefore I have a request. Please try and break these new parsers. >>> >>> >>> I’ve looked at the new http host parsers and tried some test data. >>> Most of the test cases have already been covered but still several >>> issues popped up: >> >> Thanks for the additional test cases. This is exactly the sort of >> feedback I was looking for. >> >> Would you like to get more involved in Tomcat development? If so, >> turning these into a patch for the unit tests could be good place to >> start. You'll need to mark the tests with @Ignore for now until the >> underlying bugs are fixed. For bonus points, fix the bugs in the parser >> so the tests pass. >> > > I'll prepare a patch. Excellent. > Would you prefer to keep the test cases more compact > or more extensive? > E.g. for testing IPv6 with less components than expected we may have one > test case with only 3 components (for example) or we may have many test > cases - respectively with one, two, ..., seven components? The latter > approach would make the test class a lot bigger but might help to catch > some corner cases. > > Let me know what you think. I recommend using the code coverage reports as a guide. https://ci.apache.org/projects/tomcat/tomcat9/coverage/ and add test cases if they increase code coverage. Hmm. It looks like there is some low hanging fruit in the parsing code to improve coverage. You can run the tests with code coverage locally too. Other than that, the usual advice applies. Test limits, either side of limits, etc Generally, I try and reduce copy and paste as much as possible. @RunWith has been really good for that. There is probably scope to use it more throughout the Tomcat tests. Kind regards, Mark - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: Host header validation
On Tue, Mar 28, 2017 at 5:45 PM, Mark Thomas wrote: > On 28/03/17 15:23, Katya Todorova wrote: > > Hi, > >> r1787662 adds Host header validation along with a fair number of unit > tests. > >> It includes a performance test which indicates - on my machine at least > >> - that the performance impact is in the noise. I'd like to see better > >> performance for full IPv6 addresses but the current code looks to be > >> acceptable. > >> The validation is not yet integrated into the request processing. My > >> primary reason for not integrating it is that it will trigger a 400 > >> response if the header is invalid and I don't want to incorrectly reject > >> valid headers. Therefore I have a request. Please try and break these > >> new parsers. > > > > > > I’ve looked at the new http host parsers and tried some test data. > > Most of the test cases have already been covered but still several > > issues popped up: > > Thanks for the additional test cases. This is exactly the sort of > feedback I was looking for. > > Would you like to get more involved in Tomcat development? If so, > turning these into a patch for the unit tests could be good place to > start. You'll need to mark the tests with @Ignore for now until the > underlying bugs are fixed. For bonus points, fix the bugs in the parser > so the tests pass. > I'll prepare a patch. Would you prefer to keep the test cases more compact or more extensive? E.g. for testing IPv6 with less components than expected we may have one test case with only 3 components (for example) or we may have many test cases - respectively with one, two, ..., seven components? The latter approach would make the test class a lot bigger but might help to catch some corner cases. Let me know what you think. Katya > Mark
Re: Host header validation
On Tue, Mar 28, 2017 at 5:45 PM, Mark Thomas wrote: > On 28/03/17 15:23, Katya Todorova wrote: > > Hi, > >> r1787662 adds Host header validation along with a fair number of unit > tests. > >> It includes a performance test which indicates - on my machine at least > >> - that the performance impact is in the noise. I'd like to see better > >> performance for full IPv6 addresses but the current code looks to be > >> acceptable. > >> The validation is not yet integrated into the request processing. My > >> primary reason for not integrating it is that it will trigger a 400 > >> response if the header is invalid and I don't want to incorrectly reject > >> valid headers. Therefore I have a request. Please try and break these > >> new parsers. > > > > > > I’ve looked at the new http host parsers and tried some test data. > > Most of the test cases have already been covered but still several > > issues popped up: > > Thanks for the additional test cases. This is exactly the sort of > feedback I was looking for. > > Would you like to get more involved in Tomcat development? If so, > turning these into a patch for the unit tests could be good place to > start. You'll need to mark the tests with @Ignore for now until the > underlying bugs are fixed. For bonus points, fix the bugs in the parser > so the tests pass. > > Mark > > > > > > - IPv6 addresses containing ::: are considered valid while they should > > not be - e.g . [:::::::::] > > > > (except when “:::” are located in the end , in that case the host is > > rejected as invalid) > > > > - IPv4 part of IPv6 addresses should not contain leading zeros > > according to the following part of the specification: > > > > IPv4address = dec-octet "." dec-octet "." dec-octet "." dec-octet > > > > dec-octet = DIGIT ; 0-9 > > > > / %x31-39 DIGIT ; 10-99 > > > > / "1" 2DIGIT; 100-199 > > > > / "2" %x30-34 DIGIT ; 200-249 > > > > / "25" %x30-35 ; 250-255 > > > > However, whether leading zeros are permitted or not seems to be a > > matter of a recommendation rather than a strict rule. This may lead > > to ambiguity since many sources over the internet consider 01.02.03.04 > > as valid but [::01.02.03.04] as invalid. > > > > - IPv6 Host containing any symbol other than : after ] is considered > > valid though these trailing symbols after the ] are ignored > > > > e.g. [::1]’, [::1] a > > > > - It seems that compression just before the IPv4 part is not handled > correctly. > > > >This one is considered invalid but should be valid > [a:b:c:d:e::1.2.3.4] > > > > Most of the test data has been taken from here: > > > > [1] http://home.deds.nl/~aeron/regex/invalid_ipv6.txt > > [2] http://home.deds.nl/~aeron/regex/valid_ipv6.txt > > > > > >> Please commit any values you test with. > > > > > >> Once we are happy with the quality of these parsers, I'll integrate them > >> into the request processing. > >> Mark > > > > Kind regards, > > > > Katya > > > > > - > To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org > For additional commands, e-mail: dev-h...@tomcat.apache.org > >
Re: Host header validation
On 28/03/17 15:23, Katya Todorova wrote: > Hi, >> r1787662 adds Host header validation along with a fair number of unit tests. >> It includes a performance test which indicates - on my machine at least >> - that the performance impact is in the noise. I'd like to see better >> performance for full IPv6 addresses but the current code looks to be >> acceptable. >> The validation is not yet integrated into the request processing. My >> primary reason for not integrating it is that it will trigger a 400 >> response if the header is invalid and I don't want to incorrectly reject >> valid headers. Therefore I have a request. Please try and break these >> new parsers. > > > I’ve looked at the new http host parsers and tried some test data. > Most of the test cases have already been covered but still several > issues popped up: Thanks for the additional test cases. This is exactly the sort of feedback I was looking for. Would you like to get more involved in Tomcat development? If so, turning these into a patch for the unit tests could be good place to start. You'll need to mark the tests with @Ignore for now until the underlying bugs are fixed. For bonus points, fix the bugs in the parser so the tests pass. Mark > > - IPv6 addresses containing ::: are considered valid while they should > not be - e.g . [:::::::::] > > (except when “:::” are located in the end , in that case the host is > rejected as invalid) > > - IPv4 part of IPv6 addresses should not contain leading zeros > according to the following part of the specification: > > IPv4address = dec-octet "." dec-octet "." dec-octet "." dec-octet > > dec-octet = DIGIT ; 0-9 > > / %x31-39 DIGIT ; 10-99 > > / "1" 2DIGIT; 100-199 > > / "2" %x30-34 DIGIT ; 200-249 > > / "25" %x30-35 ; 250-255 > > However, whether leading zeros are permitted or not seems to be a > matter of a recommendation rather than a strict rule. This may lead > to ambiguity since many sources over the internet consider 01.02.03.04 > as valid but [::01.02.03.04] as invalid. > > - IPv6 Host containing any symbol other than : after ] is considered > valid though these trailing symbols after the ] are ignored > > e.g. [::1]’, [::1] a > > - It seems that compression just before the IPv4 part is not handled > correctly. > >This one is considered invalid but should be valid [a:b:c:d:e::1.2.3.4] > > Most of the test data has been taken from here: > > [1] http://home.deds.nl/~aeron/regex/invalid_ipv6.txt > [2] http://home.deds.nl/~aeron/regex/valid_ipv6.txt > > >> Please commit any values you test with. > > >> Once we are happy with the quality of these parsers, I'll integrate them >> into the request processing. >> Mark > > Kind regards, > > Katya > - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: Host header validation
Hi, > r1787662 adds Host header validation along with a fair number of unit tests. > It includes a performance test which indicates - on my machine at least > - that the performance impact is in the noise. I'd like to see better > performance for full IPv6 addresses but the current code looks to be > acceptable. > The validation is not yet integrated into the request processing. My > primary reason for not integrating it is that it will trigger a 400 > response if the header is invalid and I don't want to incorrectly reject > valid headers. Therefore I have a request. Please try and break these > new parsers. I’ve looked at the new http host parsers and tried some test data. Most of the test cases have already been covered but still several issues popped up: - IPv6 addresses containing ::: are considered valid while they should not be - e.g . [:::::::::] (except when “:::” are located in the end , in that case the host is rejected as invalid) - IPv4 part of IPv6 addresses should not contain leading zeros according to the following part of the specification: IPv4address = dec-octet "." dec-octet "." dec-octet "." dec-octet dec-octet = DIGIT ; 0-9 / %x31-39 DIGIT ; 10-99 / "1" 2DIGIT; 100-199 / "2" %x30-34 DIGIT ; 200-249 / "25" %x30-35 ; 250-255 However, whether leading zeros are permitted or not seems to be a matter of a recommendation rather than a strict rule. This may lead to ambiguity since many sources over the internet consider 01.02.03.04 as valid but [::01.02.03.04] as invalid. - IPv6 Host containing any symbol other than : after ] is considered valid though these trailing symbols after the ] are ignored e.g. [::1]’, [::1] a - It seems that compression just before the IPv4 part is not handled correctly. This one is considered invalid but should be valid [a:b:c:d:e::1.2.3.4] Most of the test data has been taken from here: [1] http://home.deds.nl/~aeron/regex/invalid_ipv6.txt [2] http://home.deds.nl/~aeron/regex/valid_ipv6.txt > Please commit any values you test with. > Once we are happy with the quality of these parsers, I'll integrate them > into the request processing. > Mark Kind regards, Katya
Re: Host header validation
On 22/03/17 14:13, Konstantin Kolinko wrote: > 2017-03-21 18:01 GMT+03:00 Mark Thomas : >> On 21 March 2017 14:14:19 GMT+00:00, Christopher Schultz >> wrote: >>> >>> How about an option to disable the validity-checking, in case someone >>> in the field finds a case they need to support, or if they don't care >>> about hostname-checking and want their "performance back"? >> >> I'm not too concerned about performance. The checks are at most 1% of the >> current processing time for a trivial servlet accessed over localhost. For >> real use cases it will be less. >> >> Some form of transition could work (eg log only) but I'm reluctant to an an >> option that effectively bypasses spec compliance. > > This needs a definition of "spec". > > I am afraid that DNS spec may evolve over time. > > > 1) https://tools.ietf.org/html/rfc7230 > RFC 7230 HTTP/1.1 Message Syntax and Routing June 2014 > > Host = uri-host [ ":" port ] ; Section 2.7.1 > > uri-host = > > > 2) https://tools.ietf.org/html/rfc3986 > RFC 3986 URI Generic Syntax January 2005 > > Updated by: 6874, 7320 > > host= IP-literal / IPv4address / reg-name > > IP-literal = "[" ( IPv6address / IPvFuture ) "]" > > but RFC 6874 updates the syntax and changes "IP-literal" into > > IP-literal = "[" ( IPv6address / IPv6addrz / IPvFuture ) "]" > > ZoneID = 1*( unreserved / pct-encoded ) > > IPv6addrz = IPv6address "%25" ZoneID > > > reg-name= *( unreserved / pct-encoded / sub-delims ) > > pct-encoded = "%" HEXDIG HEXDIG > > > DomainParseState in r1787662 is more strict and I see no support for > pct-encoded characters there. > > IPv6addrz is not supported > (I mentioned in my previous e-mail, and above is the formal syntax for it) > > IPvFuture address - I think it is too early to implement it, > but it can be a reason to have a flag to turn off this check, > > > An example of IPvFuture address with HTTPd, and its response (400): > https://bz.apache.org/bugzilla/show_bug.cgi?id=55362 > > So apparently Apache HTTP Server already has some validation of Host header. I'll take a look at the spec references you quoted. The ideal would be if we could apply a general rule now and make it more specific over time. Not sure how possible that would be. Mark - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: Host header validation
2017-03-21 18:01 GMT+03:00 Mark Thomas : > On 21 March 2017 14:14:19 GMT+00:00, Christopher Schultz > wrote: >> >>How about an option to disable the validity-checking, in case someone >>in the field finds a case they need to support, or if they don't care >>about hostname-checking and want their "performance back"? > > I'm not too concerned about performance. The checks are at most 1% of the > current processing time for a trivial servlet accessed over localhost. For > real use cases it will be less. > > Some form of transition could work (eg log only) but I'm reluctant to an an > option that effectively bypasses spec compliance. This needs a definition of "spec". I am afraid that DNS spec may evolve over time. 1) https://tools.ietf.org/html/rfc7230 RFC 7230 HTTP/1.1 Message Syntax and Routing June 2014 Host = uri-host [ ":" port ] ; Section 2.7.1 uri-host = 2) https://tools.ietf.org/html/rfc3986 RFC 3986 URI Generic Syntax January 2005 Updated by: 6874, 7320 host= IP-literal / IPv4address / reg-name IP-literal = "[" ( IPv6address / IPvFuture ) "]" but RFC 6874 updates the syntax and changes "IP-literal" into IP-literal = "[" ( IPv6address / IPv6addrz / IPvFuture ) "]" ZoneID = 1*( unreserved / pct-encoded ) IPv6addrz = IPv6address "%25" ZoneID reg-name= *( unreserved / pct-encoded / sub-delims ) pct-encoded = "%" HEXDIG HEXDIG DomainParseState in r1787662 is more strict and I see no support for pct-encoded characters there. IPv6addrz is not supported (I mentioned in my previous e-mail, and above is the formal syntax for it) IPvFuture address - I think it is too early to implement it, but it can be a reason to have a flag to turn off this check, An example of IPvFuture address with HTTPd, and its response (400): https://bz.apache.org/bugzilla/show_bug.cgi?id=55362 So apparently Apache HTTP Server already has some validation of Host header. Best regards, Konstantin Kolinko - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: Host header validation
On 21 March 2017 14:14:19 GMT+00:00, Christopher Schultz wrote: >Mark, > >On 3/19/17 4:55 PM, Mark Thomas wrote: >> Hi, >> >> r1787662 adds Host header validation along with a fair number of >> unit tests. >> >> It includes a performance test which indicates - on my machine at >> least - that the performance impact is in the noise. I'd like to >> see better performance for full IPv6 addresses but the current code >> looks to be acceptable. >> >> The validation is not yet integrated into the request processing. >> My primary reason for not integrating it is that it will trigger a >> 400 response if the header is invalid and I don't want to >> incorrectly reject valid headers. Therefore I have a request. >> Please try and break these new parsers. Please commit any values >> you test with. >> >> Once we are happy with the quality of these parsers, I'll integrate >> them into the request processing. > >How about an option to disable the validity-checking, in case someone >in the field finds a case they need to support, or if they don't care >about hostname-checking and want their "performance back"? I'm not too concerned about performance. The checks are at most 1% of the current processing time for a trivial servlet accessed over localhost. For real use cases it will be less. Some form of transition could work (eg log only) but I'm reluctant to an an option that effectively bypasses spec compliance. Mark - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: Host header validation
-BEGIN PGP SIGNED MESSAGE- Hash: SHA256 Mark, On 3/19/17 4:55 PM, Mark Thomas wrote: > Hi, > > r1787662 adds Host header validation along with a fair number of > unit tests. > > It includes a performance test which indicates - on my machine at > least - that the performance impact is in the noise. I'd like to > see better performance for full IPv6 addresses but the current code > looks to be acceptable. > > The validation is not yet integrated into the request processing. > My primary reason for not integrating it is that it will trigger a > 400 response if the header is invalid and I don't want to > incorrectly reject valid headers. Therefore I have a request. > Please try and break these new parsers. Please commit any values > you test with. > > Once we are happy with the quality of these parsers, I'll integrate > them into the request processing. How about an option to disable the validity-checking, in case someone in the field finds a case they need to support, or if they don't care about hostname-checking and want their "performance back"? - -chris -BEGIN PGP SIGNATURE- Comment: GPGTools - http://gpgtools.org Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQIcBAEBCAAGBQJY0TU7AAoJEBzwKT+lPKRYK+cP/jVu+waZHfEL7s8LrTwC8+v/ +5le9Lo8vM9T5zSAYl/xbNuOx0uVkFAz1lVMxL0Jy2pG/HhntdBRmcAyP8Ms2/Ci bdqRYtKb+ZrTZNQkFKjmKrZYOrB/IgwzTrGLQ0JewbKOd7doHtN3gsa2ekzQ5h7l D9D6w6tflVzlByPgJLQ9VbqD5MxQsmxkaFBkSzfnpKYbzsXqrHF18R9p/234PLqD hxUuHhPm39AtrfeNpXUcVLxUrcGbDlzeP9pUDxgZHM69Yi82cHdoCi65VCMysges BCSnkbP2CBdkT2/aSfZa73RplOFT2XMk6OQOMSJv6SEXRdqS58A3m/Grnjj9LCX5 EtwYuy8q+Edd8JMPV6mOO980EaiQ4nha1HgrZa8Vsmbtn2xzp017lrYJPO19Cvoc cGEiCvQVTlmtUhdPV1SdogBvIhHGpMORj7+eSRPlBmCopjzSIYfeWCoecWzyRKfj 6xZ/hSVH/IufbHolRBNWIcZWQI1Oo1VkuYLQ9xM26CcodYjFcQfm9JaD4ixCXhDW 81f4erOxGVkGRrrXVQwxJ1sdXyAbB5FHbdBj5CTSv8U3LnZK26Qk/e0PcvlXTaHm CAhelcMpplkD78Dc5Chb38t3PZk41WfwRFF1km5+XhXxfGVokFhjDMNP5SuF+1EM LkYsnfIDFyApXSyq27/S =LRYA -END PGP SIGNATURE- - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
