Re: Reduce default for maxParameterCount
пт, 5 июл. 2024 г. в 23:40, Christopher Schultz : > > Mark, > > On 7/2/24 06:33, Mark Thomas wrote: > > [...] > > I would support a move to throw an unchecked exception from > getParameter* in older versions of Tomcat in order to produce a hard-fail. > > But I'm somewhat more bullish about this kind of thing. The good news is > that anyone disturbed by this will already have an application bug they > didn't know they had... which is the whole point of making it a hard-fail. > > Hmm. Existing applications using FailedRequestFilter, though... > > On application startup, we could check to see if the FailedRequestFilter > has been installed at all and, if not, configure to hard-fail. WDYT? It is solvable by simply adding a try/catch (for this exception) around the getParameters call in the FailedRequestFilter. The expected use of the filter is that it is placed "in front" of the app, so the result is that it is the one who triggers parameter parsing. Thus if parsing results in an exception - just catch it and go on. :) Best regards, K.Kolinko - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: Reduce default for maxParameterCount
Mark, On 7/2/24 06:33, Mark Thomas wrote: On 01/07/2024 07:17, Michael Osipov wrote: On 2024/06/27 17:13:56 Christopher Schultz wrote: Michael, On 6/27/24 08:46, Michael Osipov wrote: On 2023/03/09 14:23:33 Christopher Schultz wrote: A potential use-case for "large numbers of parameters" might be an application that uses something like a multi-select list and the number of choices is stupendously high. As in, when the application was designed, the designers said "we can use a multi-select list for this postal-code selector" and then 5-10 years later, someone said "hey let's dump all postal codes in the entire US into this multi-select" and there you have thousands of possibilities which the browser will happily package-off to the server. FTR, guess what? We have been hit by this. Nostradamus. Spot on. We have an HTML form for the user where he sees a diff between a src and dest. Technical values, can be thousands. The selected value ids are POSTed. Users have been complaining that they miss data. I took me some time to remember (actually weeks between report and memory) that I have replicated maxParameterCount="1000" to our server.xml without using FailedRequestFilter. Retrospectively, it should have gone hand in hand with that filter and not without. Rather fail fast than suffer data truncation. I don't think there is really a way for us to push a into all web applications. I mean... there MAY be a way to do it, but it will likely be ugly and we would also have to "move" it if the application defines filters in a specific order including the FailedRequestFilter. The good news is it doesn't do anything weird like trigger request-parsing or try to do anything with character sets or whatever. The real question is whether or not this kind of thing should be handled in a Filter or just handled by Tomcat itself. Why bother waiting for the application to check: just throw an exception and kill the request processing. I have spent the whole Friday to provide data to engineeing that has been truncated. It seems that we have upto 3000 form values submitted. I have bumbed to 5000 now. I would really really expect that Tomcat fails hard with 4xx if the input is invalid and not issue a simple INFO at the log. The huge problem is that the request is seen as 2xx or 3xx in the access log and if you have a lot of requests or forms it will be needle in the haystack to identify which is really the problem. Even worse, if this has not been written by you you can play ping pong with the software vendor. Therefore, I'd like all of us (committers) to reconsider this soft non-failing approach. It is not helpful. If the client provides garbage it should fail immediately. With Tomcat 11.0.x you will get a hard fail. Prior to Tomcat 11 our hands are somewhat tied by the Servlet specification since getParameter() and friends are documented to not throw an exception. We can't change the default behaviour for Tomcat versions before 11 as that runs the risk of breaking existing applications that have been designed for the current behaviour. All we can do is make that hard failure optional and it already is. For a (very) long time we have had the FailedRequestFilter for folks that wanted a hard failure if there was an issue with parameter parsing. Changing the default for maxParameterCount from 10,000 to 1,000 doesn't change this. The documentation for maxParameterCount already documents all of this. I don't see a need for any changes here. I would support a move to throw an unchecked exception from getParameter* in older versions of Tomcat in order to produce a hard-fail. But I'm somewhat more bullish about this kind of thing. The good news is that anyone disturbed by this will already have an application bug they didn't know they had... which is the whole point of making it a hard-fail. Hmm. Existing applications using FailedRequestFilter, though... On application startup, we could check to see if the FailedRequestFilter has been installed at all and, if not, configure to hard-fail. WDYT? -chris - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: Reduce default for maxParameterCount
On 2024/07/02 11:06:55 Rémy Maucherat wrote: > On Tue, Jul 2, 2024 at 1:05 PM Mark Thomas wrote: > > > > On 02/07/2024 12:01, Michael Osipov wrote: > > > On 2024/07/02 10:33:29 Mark Thomas wrote: > > >> On 01/07/2024 07:17, Michael Osipov wrote: > > > > > > > > >>> I would really really expect that Tomcat fails hard with 4xx if the > > >>> input is invalid and not issue a simple INFO at the log. The huge > > >>> problem is that the request is seen as 2xx or 3xx in the access log and > > >>> if you have a lot of requests or forms it will be needle in the > > >>> haystack to identify which is really the problem. > > >>> Even worse, if this has not been written by you you can play ping pong > > >>> with the software vendor. > > >>> Therefore, I'd like all of us (committers) to reconsider this soft > > >>> non-failing approach. It is not helpful. If the client provides garbage > > >>> it should fail immediately. > > >> > > >> With Tomcat 11.0.x you will get a hard fail. > > >> > > >> Prior to Tomcat 11 our hands are somewhat tied by the Servlet > > >> specification since getParameter() and friends are documented to not > > >> throw an exception. > > >> > > >> We can't change the default behaviour for Tomcat versions before 11 as > > >> that runs the risk of breaking existing applications that have been > > >> designed for the current behaviour. > > >> > > >> All we can do is make that hard failure optional and it already is. For > > >> a (very) long time we have had the FailedRequestFilter for folks that > > >> wanted a hard failure if there was an issue with parameter parsing. > > >> > > >> Changing the default for maxParameterCount from 10,000 to 1,000 doesn't > > >> change this. > > >> > > >> The documentation for maxParameterCount already documents all of this. > > >> > > >> I don't see a need for any changes here. > > > > > > Thanks, I see. As a comprise would a move to WARN log message be > > > accepted? This will at least draw people's attention. INFORMATION is just > > > lost with other output. > > > > That seems reasonable to me. > > It seems UserDataHelper needs some changes then, careful about last > minute breakage. I won't oppose, August release would be just fine. - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: Reduce default for maxParameterCount
On Tue, Jul 2, 2024 at 1:05 PM Mark Thomas wrote: > > On 02/07/2024 12:01, Michael Osipov wrote: > > On 2024/07/02 10:33:29 Mark Thomas wrote: > >> On 01/07/2024 07:17, Michael Osipov wrote: > > > > >>> I would really really expect that Tomcat fails hard with 4xx if the input > >>> is invalid and not issue a simple INFO at the log. The huge problem is > >>> that the request is seen as 2xx or 3xx in the access log and if you have > >>> a lot of requests or forms it will be needle in the haystack to identify > >>> which is really the problem. > >>> Even worse, if this has not been written by you you can play ping pong > >>> with the software vendor. > >>> Therefore, I'd like all of us (committers) to reconsider this soft > >>> non-failing approach. It is not helpful. If the client provides garbage > >>> it should fail immediately. > >> > >> With Tomcat 11.0.x you will get a hard fail. > >> > >> Prior to Tomcat 11 our hands are somewhat tied by the Servlet > >> specification since getParameter() and friends are documented to not > >> throw an exception. > >> > >> We can't change the default behaviour for Tomcat versions before 11 as > >> that runs the risk of breaking existing applications that have been > >> designed for the current behaviour. > >> > >> All we can do is make that hard failure optional and it already is. For > >> a (very) long time we have had the FailedRequestFilter for folks that > >> wanted a hard failure if there was an issue with parameter parsing. > >> > >> Changing the default for maxParameterCount from 10,000 to 1,000 doesn't > >> change this. > >> > >> The documentation for maxParameterCount already documents all of this. > >> > >> I don't see a need for any changes here. > > > > Thanks, I see. As a comprise would a move to WARN log message be accepted? > > This will at least draw people's attention. INFORMATION is just lost with > > other output. > > That seems reasonable to me. It seems UserDataHelper needs some changes then, careful about last minute breakage. Rémy > 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: Reduce default for maxParameterCount
On 02/07/2024 12:01, Michael Osipov wrote: On 2024/07/02 10:33:29 Mark Thomas wrote: On 01/07/2024 07:17, Michael Osipov wrote: I would really really expect that Tomcat fails hard with 4xx if the input is invalid and not issue a simple INFO at the log. The huge problem is that the request is seen as 2xx or 3xx in the access log and if you have a lot of requests or forms it will be needle in the haystack to identify which is really the problem. Even worse, if this has not been written by you you can play ping pong with the software vendor. Therefore, I'd like all of us (committers) to reconsider this soft non-failing approach. It is not helpful. If the client provides garbage it should fail immediately. With Tomcat 11.0.x you will get a hard fail. Prior to Tomcat 11 our hands are somewhat tied by the Servlet specification since getParameter() and friends are documented to not throw an exception. We can't change the default behaviour for Tomcat versions before 11 as that runs the risk of breaking existing applications that have been designed for the current behaviour. All we can do is make that hard failure optional and it already is. For a (very) long time we have had the FailedRequestFilter for folks that wanted a hard failure if there was an issue with parameter parsing. Changing the default for maxParameterCount from 10,000 to 1,000 doesn't change this. The documentation for maxParameterCount already documents all of this. I don't see a need for any changes here. Thanks, I see. As a comprise would a move to WARN log message be accepted? This will at least draw people's attention. INFORMATION is just lost with other output. That seems reasonable to me. Mark - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: Reduce default for maxParameterCount
On 2024/07/02 10:33:29 Mark Thomas wrote: > On 01/07/2024 07:17, Michael Osipov wrote: > > On 2024/06/27 17:13:56 Christopher Schultz wrote: > >> Michael, > >> > >> On 6/27/24 08:46, Michael Osipov wrote: > >>> On 2023/03/09 14:23:33 Christopher Schultz wrote: > A potential use-case for "large numbers of parameters" might be an > application that uses something like a multi-select list and the number > of choices is stupendously high. As in, when the application was > designed, the designers said "we can use a multi-select list for this > postal-code selector" and then 5-10 years later, someone said "hey let's > dump all postal codes in the entire US into this multi-select" and there > you have thousands of possibilities which the browser will happily > package-off to the server. > >>> > >>> FTR, guess what? We have been hit by this. > >> > >> Nostradamus. > > > > Spot on. > > > >>> We have an HTML form for the user where he sees a diff between a src > >>> and dest. Technical values, can be thousands. The selected value ids > >>> are POSTed. Users have been complaining that they miss data. I took > >>> me some time to remember (actually weeks between report and memory) > >>> that I have replicated maxParameterCount="1000" to our server.xml > >>> without using FailedRequestFilter. Retrospectively, it should have > >>> gone hand in hand with that filter and not without. Rather fail fast > >>> than suffer data truncation. > >> > >> I don't think there is really a way for us to push a into all > >> web applications. > >> > >> I mean... there MAY be a way to do it, but it will likely be ugly and we > >> would also have to "move" it if the application defines filters in a > >> specific order including the FailedRequestFilter. > >> > >> The good news is it doesn't do anything weird like trigger > >> request-parsing or try to do anything with character sets or whatever. > >> > >> The real question is whether or not this kind of thing should be handled > >> in a Filter or just handled by Tomcat itself. Why bother waiting for the > >> application to check: just throw an exception and kill the request > >> processing. > > > > I have spent the whole Friday to provide data to engineeing that has been > > truncated. It seems that we have upto 3000 form values submitted. I have > > bumbed to 5000 now. > > > > I would really really expect that Tomcat fails hard with 4xx if the input > > is invalid and not issue a simple INFO at the log. The huge problem is that > > the request is seen as 2xx or 3xx in the access log and if you have a lot > > of requests or forms it will be needle in the haystack to identify which is > > really the problem. > > Even worse, if this has not been written by you you can play ping pong with > > the software vendor. > > Therefore, I'd like all of us (committers) to reconsider this soft > > non-failing approach. It is not helpful. If the client provides garbage it > > should fail immediately. > > With Tomcat 11.0.x you will get a hard fail. > > Prior to Tomcat 11 our hands are somewhat tied by the Servlet > specification since getParameter() and friends are documented to not > throw an exception. > > We can't change the default behaviour for Tomcat versions before 11 as > that runs the risk of breaking existing applications that have been > designed for the current behaviour. > > All we can do is make that hard failure optional and it already is. For > a (very) long time we have had the FailedRequestFilter for folks that > wanted a hard failure if there was an issue with parameter parsing. > > Changing the default for maxParameterCount from 10,000 to 1,000 doesn't > change this. > > The documentation for maxParameterCount already documents all of this. > > I don't see a need for any changes here. Thanks, I see. As a comprise would a move to WARN log message be accepted? This will at least draw people's attention. INFORMATION is just lost with other output. Michael - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: Reduce default for maxParameterCount
On 01/07/2024 07:17, Michael Osipov wrote: On 2024/06/27 17:13:56 Christopher Schultz wrote: Michael, On 6/27/24 08:46, Michael Osipov wrote: On 2023/03/09 14:23:33 Christopher Schultz wrote: A potential use-case for "large numbers of parameters" might be an application that uses something like a multi-select list and the number of choices is stupendously high. As in, when the application was designed, the designers said "we can use a multi-select list for this postal-code selector" and then 5-10 years later, someone said "hey let's dump all postal codes in the entire US into this multi-select" and there you have thousands of possibilities which the browser will happily package-off to the server. FTR, guess what? We have been hit by this. Nostradamus. Spot on. We have an HTML form for the user where he sees a diff between a src and dest. Technical values, can be thousands. The selected value ids are POSTed. Users have been complaining that they miss data. I took me some time to remember (actually weeks between report and memory) that I have replicated maxParameterCount="1000" to our server.xml without using FailedRequestFilter. Retrospectively, it should have gone hand in hand with that filter and not without. Rather fail fast than suffer data truncation. I don't think there is really a way for us to push a into all web applications. I mean... there MAY be a way to do it, but it will likely be ugly and we would also have to "move" it if the application defines filters in a specific order including the FailedRequestFilter. The good news is it doesn't do anything weird like trigger request-parsing or try to do anything with character sets or whatever. The real question is whether or not this kind of thing should be handled in a Filter or just handled by Tomcat itself. Why bother waiting for the application to check: just throw an exception and kill the request processing. I have spent the whole Friday to provide data to engineeing that has been truncated. It seems that we have upto 3000 form values submitted. I have bumbed to 5000 now. I would really really expect that Tomcat fails hard with 4xx if the input is invalid and not issue a simple INFO at the log. The huge problem is that the request is seen as 2xx or 3xx in the access log and if you have a lot of requests or forms it will be needle in the haystack to identify which is really the problem. Even worse, if this has not been written by you you can play ping pong with the software vendor. Therefore, I'd like all of us (committers) to reconsider this soft non-failing approach. It is not helpful. If the client provides garbage it should fail immediately. With Tomcat 11.0.x you will get a hard fail. Prior to Tomcat 11 our hands are somewhat tied by the Servlet specification since getParameter() and friends are documented to not throw an exception. We can't change the default behaviour for Tomcat versions before 11 as that runs the risk of breaking existing applications that have been designed for the current behaviour. All we can do is make that hard failure optional and it already is. For a (very) long time we have had the FailedRequestFilter for folks that wanted a hard failure if there was an issue with parameter parsing. Changing the default for maxParameterCount from 10,000 to 1,000 doesn't change this. The documentation for maxParameterCount already documents all of this. I don't see a need for any changes here. Mark - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: Reduce default for maxParameterCount
On 2024/06/27 17:13:56 Christopher Schultz wrote: > Michael, > > On 6/27/24 08:46, Michael Osipov wrote: > > On 2023/03/09 14:23:33 Christopher Schultz wrote: > >> A potential use-case for "large numbers of parameters" might be an > >> application that uses something like a multi-select list and the number > >> of choices is stupendously high. As in, when the application was > >> designed, the designers said "we can use a multi-select list for this > >> postal-code selector" and then 5-10 years later, someone said "hey let's > >> dump all postal codes in the entire US into this multi-select" and there > >> you have thousands of possibilities which the browser will happily > >> package-off to the server. > > > > FTR, guess what? We have been hit by this. > > Nostradamus. Spot on. > > We have an HTML form for the user where he sees a diff between a src > > and dest. Technical values, can be thousands. The selected value ids > > are POSTed. Users have been complaining that they miss data. I took > > me some time to remember (actually weeks between report and memory) > > that I have replicated maxParameterCount="1000" to our server.xml > > without using FailedRequestFilter. Retrospectively, it should have > > gone hand in hand with that filter and not without. Rather fail fast > > than suffer data truncation. > > I don't think there is really a way for us to push a into all > web applications. > > I mean... there MAY be a way to do it, but it will likely be ugly and we > would also have to "move" it if the application defines filters in a > specific order including the FailedRequestFilter. > > The good news is it doesn't do anything weird like trigger > request-parsing or try to do anything with character sets or whatever. > > The real question is whether or not this kind of thing should be handled > in a Filter or just handled by Tomcat itself. Why bother waiting for the > application to check: just throw an exception and kill the request > processing. I have spent the whole Friday to provide data to engineeing that has been truncated. It seems that we have upto 3000 form values submitted. I have bumbed to 5000 now. I would really really expect that Tomcat fails hard with 4xx if the input is invalid and not issue a simple INFO at the log. The huge problem is that the request is seen as 2xx or 3xx in the access log and if you have a lot of requests or forms it will be needle in the haystack to identify which is really the problem. Even worse, if this has not been written by you you can play ping pong with the software vendor. Therefore, I'd like all of us (committers) to reconsider this soft non-failing approach. It is not helpful. If the client provides garbage it should fail immediately. Michael - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: Reduce default for maxParameterCount
Michael, On 6/27/24 08:46, Michael Osipov wrote: On 2023/03/09 14:23:33 Christopher Schultz wrote: A potential use-case for "large numbers of parameters" might be an application that uses something like a multi-select list and the number of choices is stupendously high. As in, when the application was designed, the designers said "we can use a multi-select list for this postal-code selector" and then 5-10 years later, someone said "hey let's dump all postal codes in the entire US into this multi-select" and there you have thousands of possibilities which the browser will happily package-off to the server. FTR, guess what? We have been hit by this. Nostradamus. We have an HTML form for the user where he sees a diff between a src and dest. Technical values, can be thousands. The selected value ids are POSTed. Users have been complaining that they miss data. I took me some time to remember (actually weeks between report and memory) that I have replicated maxParameterCount="1000" to our server.xml without using FailedRequestFilter. Retrospectively, it should have gone hand in hand with that filter and not without. Rather fail fast than suffer data truncation. I don't think there is really a way for us to push a into all web applications. I mean... there MAY be a way to do it, but it will likely be ugly and we would also have to "move" it if the application defines filters in a specific order including the FailedRequestFilter. The good news is it doesn't do anything weird like trigger request-parsing or try to do anything with character sets or whatever. The real question is whether or not this kind of thing should be handled in a Filter or just handled by Tomcat itself. Why bother waiting for the application to check: just throw an exception and kill the request processing. It could be a well-documented Exception class that any application could configure in an configuration and they can intercept it. -chris - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: Reduce default for maxParameterCount
On Fri, Mar 24, 2023 at 10:01 AM Mark Thomas wrote: > > On 23/03/2023 20:20, Christopher Schultz wrote: > > Mark, > > > > On 3/22/23 07:38, Mark Thomas wrote: > >> Any more thoughts on this? > >> > >> There hasn't been much movement from the spec EG on this, so my > >> current thinking is to revert this change for 10.1.x and earlier to > >> wait and see what the Servlet EG decides. > > > > I'd like to leave our changes in, but I understand that Konstantin has a > > good point about silently discarding parameters. > > > > There is no particular reason not to implement option (c) (throw > > RuntimeException if the maximum number of parameters is exceeded). > > Anyone affected by it can change the setting, and an appropriate error > > message can direct operators to that setting to make it easy. > > The problem with option c) is that there would be no way for someone to > get back to the current behaviour of accepting the first 10,000 > parameters and then silently swallowing the rest. I agree that seems > unlikely but with such a wide user-base I wouldn't be surprised if that > was a problem for a few users. > > Which brings us back to Konstantin's point that this really needs to be > configurable. I hope that is the direction the Servlet EG is going to > head in but wherever the EG ends up, it isn't going to get there in time > for the April releases. > > I did think of another possible interim option this morning: > > - leave 11.0.x as is with a hard-coded limit of 1,000 > - for 10.1.x and earlier >- revert the change to the hard-coded limit >- configure a lower limit of 1,000 in server.xml >- review next steps once the Servlet EG has decided on a plan for > Servlet 6.1 Ok if it addresses some concern. 1000 seems already like "a lot" to me though so I don't really understand. Of course it is a limit that wasn't there before, but it's not the first time a similarly "high" limit is added due to resource use (IMO the HTTP header size is the most "limiting" one). > This effectively introduces the lower limit for "new" users. Upgrading > users will retain their current limit but should see the entry in the > change log, the note in the migration guide and the diff in server.xml. > We can also call it out as one of the key changes in the release > announcement. +1 Rémy > > 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: Reduce default for maxParameterCount
On 23/03/2023 20:20, Christopher Schultz wrote: Mark, On 3/22/23 07:38, Mark Thomas wrote: Any more thoughts on this? There hasn't been much movement from the spec EG on this, so my current thinking is to revert this change for 10.1.x and earlier to wait and see what the Servlet EG decides. I'd like to leave our changes in, but I understand that Konstantin has a good point about silently discarding parameters. There is no particular reason not to implement option (c) (throw RuntimeException if the maximum number of parameters is exceeded). Anyone affected by it can change the setting, and an appropriate error message can direct operators to that setting to make it easy. The problem with option c) is that there would be no way for someone to get back to the current behaviour of accepting the first 10,000 parameters and then silently swallowing the rest. I agree that seems unlikely but with such a wide user-base I wouldn't be surprised if that was a problem for a few users. Which brings us back to Konstantin's point that this really needs to be configurable. I hope that is the direction the Servlet EG is going to head in but wherever the EG ends up, it isn't going to get there in time for the April releases. I did think of another possible interim option this morning: - leave 11.0.x as is with a hard-coded limit of 1,000 - for 10.1.x and earlier - revert the change to the hard-coded limit - configure a lower limit of 1,000 in server.xml - review next steps once the Servlet EG has decided on a plan for Servlet 6.1 This effectively introduces the lower limit for "new" users. Upgrading users will retain their current limit but should see the entry in the change log, the note in the migration guide and the diff in server.xml. We can also call it out as one of the key changes in the release announcement. Mark - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: Reduce default for maxParameterCount
Mark, On 3/22/23 07:38, Mark Thomas wrote: Any more thoughts on this? There hasn't been much movement from the spec EG on this, so my current thinking is to revert this change for 10.1.x and earlier to wait and see what the Servlet EG decides. I'd like to leave our changes in, but I understand that Konstantin has a good point about silently discarding parameters. There is no particular reason not to implement option (c) (throw RuntimeException if the maximum number of parameters is exceeded). Anyone affected by it can change the setting, and an appropriate error message can direct operators to that setting to make it easy. -chris On 15/03/2023 15:05, Mark Thomas wrote: On 15/03/2023 11:22, Konstantin Kolinko wrote: ср, 15 мар. 2023 г. в 13:29, Konstantin Kolinko : ср, 15 мар. 2023 г. в 13:15, Konstantin Kolinko : ср, 15 мар. 2023 г. в 12:07, Mark Thomas : On 14/03/2023 21:13, Christopher Schultz wrote: On 3/14/23 13:57, Mark Thomas wrote: On 09/03/2023 14:23, Christopher Schultz wrote: I would go for a 1000 limit for all currently-supported versions. It's *very* easy to raise the limit if it interferes with a specific application's functions. I *would* add an entry in the "notable changes" for each release e.g. https://tomcat.apache.org/migration-10.1.html#Tomcat_10.1.x_noteable_changes Makes sense. I'll do that. -1 unless the behaviour of "silently dropping extra parameters" is changed as well. Silent loss of data is not what I want to see in production. Fair point. Although I'll note that that is exactly what happens if the current limit is exceeded. I accept that, by lowering the limit, it is now more likely that limit will be exceeded. How much more likely I don't know and I don't think we have any reasonable way to determine. Also, the failure isn't completely silent. There will be an INFO log message the first time it happens in a 24 hour period. Proposals: 1. I think that maxParameterCount would better be configured per-Context. The count of parameters is a property of a specific web application. Makes sense. As an migration path for 10.1.x, 9.0.x and 8.5.x, do we want to make the Connector attribute the default to be used if a value is not explicitly set on the Context? That makes the new feature backwards compatible. We can remove the Connector setting in 11.0.x. 2. I wonder if we can make handling of the errors configurable. I think that the following options are possible: a) Drop parameters that exceeded the limit, or failed to decode. This is what we do now. b) If there is any error, ignore all parameters and behave as if none were provided. I'm wary of doing anything that will cause currently working applications to start breaking. c) Blow up by throwing a RuntimeException for any call to Request.getParameter() methods. It may be an IllegalStateException. This topic (error handling in parameters) is currently under discussion in the Servlet EG (https://github.com/jakartaee/servlet/issues/431). That discussion isn't particularly active but it is one of the current servlet issues on my TODO list so there will hopefully be some progress. My first thought was to go with c). I know that it contradicts with Servlet API JavaDoc, but if it is configurable then it is a possible option. I suppose that a web application should have error handling configured and should be able to deal with errors. If we go with c), it requires adding try/catch to safeguard getParameter() calls in the following classes of Tomcat: - org.apache.catalina.filters.FailedRequestFilter - org.apache.catalina.valves.ExtendedAccessLogValve (The ExtendedAccessLogValve can be configured to log the value of a parameter.) 3. I propose to change the default behaviour to b), "ignoring all parameters". The loss of data will be clearly visible to the applications. It would not go unnoted. In an ideal world, the Servlet spec would have opted for c) from the start. I wonder if it might not be better to revert this change for 10.1.x and earlier until the Servlet EG resolves #431 and then reconsider our options with (potentially) a new default behaviour in 11.0.x. If we don't revert then, of the current options: My concern with both b) and c) is that they could break applications that currently work. I don't like doing that if we don't have to in a point release. That leaves a). My main concern with a) is how to raise visibility of exceeding the limit. What if we changed the way UserDataHelper works (or introduced something new) that limited the number of log messages per period and thereby avoided the DoS risk via excessive logging but still generated enough log messages to raise awareness of the issue. Mark - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org -
Re: Reduce default for maxParameterCount
On 23/03/2023 12:02, Konstantin Kolinko wrote:
Thanks for the continued feedback. Having someone to bounce ideas off is
really helpful.
ср, 22 мар. 2023 г. в 14:38, Mark Thomas :
Any more thoughts on this?
1. If we cannot agree on the required behaviour, it is one more reason
to make it configurable.
I think you are right. I have proposed a new
RequestParameterErrorListener over at
https://github.com/jakartaee/servlet/issues/431
The details aren't defined yet. I'm just looking for consensus on the
general approach at this point.
As I said, it would be more useful to configure it at a Context.
Agreed.
2. Regarding the default behaviour,
Throwing an exception was also my first thought,
and it seems more natural.
Valid cases are being made for both throwing an exception and ignoring
the error. Which is what makes this so tricky.
I'm thinking default to current behaviour but with simple options on the
context to change to other approaches.
Regarding implementation, I thought that in
org.apache.catalina.connector.Request all lines
if (!parametersParsed) {
parseParameters();
}
could be amended with "if (parseFailed) throw new
IllegalStateException(parseFailedReason)", or maybe put this "throw"
into parseParameters().
BTW, Spring Framework has a feature that routing of requests can be
configured with annotations.
https://docs.spring.io/spring-framework/docs/5.3.25/reference/html/web.html#mvc-ann-requestmapping-params-and-headers
In this case parameters parsing is hidden from the caller (done by
framework), and also a Request may be omitted from method signature
(so one wouldn't check its attributes to check for failed parsing). In
this case it makes sense to throw an exception to report a failure.
Agreed.
3. Regarding UserDataHelper,
1) If we rely on it, it means being too late.
At the time one considers reading the logs, data loss has already happened.
2) If you look at my mail regarding code paths (7th email in this thread),
if I have read the code correctly, I think that in case of
"d) Request.getParameter() was called, and request was "multipart/form-data"."
there is no logging.
That probably needs fixing.
4. Regarding the value for maxParameterCount
500 parameters may mean 100 rows of 5 values each;
100 rows may mean daily values for 3 months.
1000 parameters may mean a year of daily data with 3 values each day.
It is not what one would frequently see in practice, but it could happen.
There hasn't been much movement from the spec EG on this, so my current
thinking is to revert this change for 10.1.x and earlier to wait and see
what the Servlet EG decides.
I'm still leaning in this direction at the moment.
5. If someone is thinking about improved API,
a) I wonder whether ExtendedAccessLogValve calling of getParameter()
could be improved,
so that it does not trigger parameter parsing that includes reading
the body of a POST.
Or maybe do reading, but with a lower timeout. Essentially in the same
way as when skipping a body of a failed request.
It is not a job for an AccessLogValve to spend time on parameter parsing.
b) I wonder whether parameter parsing could be done asynchronously.
I have seen feature requests along those lines. async is going to
complicate error handling. But the error listener may help here.
Mark
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: Reduce default for maxParameterCount
ср, 22 мар. 2023 г. в 14:38, Mark Thomas :
>
> Any more thoughts on this?
>
1. If we cannot agree on the required behaviour, it is one more reason
to make it configurable.
As I said, it would be more useful to configure it at a Context.
2. Regarding the default behaviour,
Throwing an exception was also my first thought,
and it seems more natural.
Regarding implementation, I thought that in
org.apache.catalina.connector.Request all lines
if (!parametersParsed) {
parseParameters();
}
could be amended with "if (parseFailed) throw new
IllegalStateException(parseFailedReason)", or maybe put this "throw"
into parseParameters().
BTW, Spring Framework has a feature that routing of requests can be
configured with annotations.
https://docs.spring.io/spring-framework/docs/5.3.25/reference/html/web.html#mvc-ann-requestmapping-params-and-headers
In this case parameters parsing is hidden from the caller (done by
framework), and also a Request may be omitted from method signature
(so one wouldn't check its attributes to check for failed parsing). In
this case it makes sense to throw an exception to report a failure.
3. Regarding UserDataHelper,
1) If we rely on it, it means being too late.
At the time one considers reading the logs, data loss has already happened.
2) If you look at my mail regarding code paths (7th email in this thread),
if I have read the code correctly, I think that in case of
"d) Request.getParameter() was called, and request was "multipart/form-data"."
there is no logging.
4. Regarding the value for maxParameterCount
500 parameters may mean 100 rows of 5 values each;
100 rows may mean daily values for 3 months.
1000 parameters may mean a year of daily data with 3 values each day.
It is not what one would frequently see in practice, but it could happen.
> There hasn't been much movement from the spec EG on this, so my current
> thinking is to revert this change for 10.1.x and earlier to wait and see
> what the Servlet EG decides.
>
5. If someone is thinking about improved API,
a) I wonder whether ExtendedAccessLogValve calling of getParameter()
could be improved,
so that it does not trigger parameter parsing that includes reading
the body of a POST.
Or maybe do reading, but with a lower timeout. Essentially in the same
way as when skipping a body of a failed request.
It is not a job for an AccessLogValve to spend time on parameter parsing.
b) I wonder whether parameter parsing could be done asynchronously.
Best regards,
Konstantin Kolinko
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: Reduce default for maxParameterCount
Any more thoughts on this? There hasn't been much movement from the spec EG on this, so my current thinking is to revert this change for 10.1.x and earlier to wait and see what the Servlet EG decides. Mark On 15/03/2023 15:05, Mark Thomas wrote: On 15/03/2023 11:22, Konstantin Kolinko wrote: ср, 15 мар. 2023 г. в 13:29, Konstantin Kolinko : ср, 15 мар. 2023 г. в 13:15, Konstantin Kolinko : ср, 15 мар. 2023 г. в 12:07, Mark Thomas : On 14/03/2023 21:13, Christopher Schultz wrote: On 3/14/23 13:57, Mark Thomas wrote: On 09/03/2023 14:23, Christopher Schultz wrote: I would go for a 1000 limit for all currently-supported versions. It's *very* easy to raise the limit if it interferes with a specific application's functions. I *would* add an entry in the "notable changes" for each release e.g. https://tomcat.apache.org/migration-10.1.html#Tomcat_10.1.x_noteable_changes Makes sense. I'll do that. -1 unless the behaviour of "silently dropping extra parameters" is changed as well. Silent loss of data is not what I want to see in production. Fair point. Although I'll note that that is exactly what happens if the current limit is exceeded. I accept that, by lowering the limit, it is now more likely that limit will be exceeded. How much more likely I don't know and I don't think we have any reasonable way to determine. Also, the failure isn't completely silent. There will be an INFO log message the first time it happens in a 24 hour period. Proposals: 1. I think that maxParameterCount would better be configured per-Context. The count of parameters is a property of a specific web application. Makes sense. As an migration path for 10.1.x, 9.0.x and 8.5.x, do we want to make the Connector attribute the default to be used if a value is not explicitly set on the Context? That makes the new feature backwards compatible. We can remove the Connector setting in 11.0.x. 2. I wonder if we can make handling of the errors configurable. I think that the following options are possible: a) Drop parameters that exceeded the limit, or failed to decode. This is what we do now. b) If there is any error, ignore all parameters and behave as if none were provided. I'm wary of doing anything that will cause currently working applications to start breaking. c) Blow up by throwing a RuntimeException for any call to Request.getParameter() methods. It may be an IllegalStateException. This topic (error handling in parameters) is currently under discussion in the Servlet EG (https://github.com/jakartaee/servlet/issues/431). That discussion isn't particularly active but it is one of the current servlet issues on my TODO list so there will hopefully be some progress. My first thought was to go with c). I know that it contradicts with Servlet API JavaDoc, but if it is configurable then it is a possible option. I suppose that a web application should have error handling configured and should be able to deal with errors. If we go with c), it requires adding try/catch to safeguard getParameter() calls in the following classes of Tomcat: - org.apache.catalina.filters.FailedRequestFilter - org.apache.catalina.valves.ExtendedAccessLogValve (The ExtendedAccessLogValve can be configured to log the value of a parameter.) 3. I propose to change the default behaviour to b), "ignoring all parameters". The loss of data will be clearly visible to the applications. It would not go unnoted. In an ideal world, the Servlet spec would have opted for c) from the start. I wonder if it might not be better to revert this change for 10.1.x and earlier until the Servlet EG resolves #431 and then reconsider our options with (potentially) a new default behaviour in 11.0.x. If we don't revert then, of the current options: My concern with both b) and c) is that they could break applications that currently work. I don't like doing that if we don't have to in a point release. That leaves a). My main concern with a) is how to raise visibility of exceeding the limit. What if we changed the way UserDataHelper works (or introduced something new) that limited the number of log messages per period and thereby avoided the DoS risk via excessive logging but still generated enough log messages to raise awareness of the issue. 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: Reduce default for maxParameterCount
On 15/03/2023 11:22, Konstantin Kolinko wrote: ср, 15 мар. 2023 г. в 13:29, Konstantin Kolinko : ср, 15 мар. 2023 г. в 13:15, Konstantin Kolinko : ср, 15 мар. 2023 г. в 12:07, Mark Thomas : On 14/03/2023 21:13, Christopher Schultz wrote: On 3/14/23 13:57, Mark Thomas wrote: On 09/03/2023 14:23, Christopher Schultz wrote: I would go for a 1000 limit for all currently-supported versions. It's *very* easy to raise the limit if it interferes with a specific application's functions. I *would* add an entry in the "notable changes" for each release e.g. https://tomcat.apache.org/migration-10.1.html#Tomcat_10.1.x_noteable_changes Makes sense. I'll do that. -1 unless the behaviour of "silently dropping extra parameters" is changed as well. Silent loss of data is not what I want to see in production. Fair point. Although I'll note that that is exactly what happens if the current limit is exceeded. I accept that, by lowering the limit, it is now more likely that limit will be exceeded. How much more likely I don't know and I don't think we have any reasonable way to determine. Also, the failure isn't completely silent. There will be an INFO log message the first time it happens in a 24 hour period. Proposals: 1. I think that maxParameterCount would better be configured per-Context. The count of parameters is a property of a specific web application. Makes sense. As an migration path for 10.1.x, 9.0.x and 8.5.x, do we want to make the Connector attribute the default to be used if a value is not explicitly set on the Context? That makes the new feature backwards compatible. We can remove the Connector setting in 11.0.x. 2. I wonder if we can make handling of the errors configurable. I think that the following options are possible: a) Drop parameters that exceeded the limit, or failed to decode. This is what we do now. b) If there is any error, ignore all parameters and behave as if none were provided. I'm wary of doing anything that will cause currently working applications to start breaking. c) Blow up by throwing a RuntimeException for any call to Request.getParameter() methods. It may be an IllegalStateException. This topic (error handling in parameters) is currently under discussion in the Servlet EG (https://github.com/jakartaee/servlet/issues/431). That discussion isn't particularly active but it is one of the current servlet issues on my TODO list so there will hopefully be some progress. My first thought was to go with c). I know that it contradicts with Servlet API JavaDoc, but if it is configurable then it is a possible option. I suppose that a web application should have error handling configured and should be able to deal with errors. If we go with c), it requires adding try/catch to safeguard getParameter() calls in the following classes of Tomcat: - org.apache.catalina.filters.FailedRequestFilter - org.apache.catalina.valves.ExtendedAccessLogValve (The ExtendedAccessLogValve can be configured to log the value of a parameter.) 3. I propose to change the default behaviour to b), "ignoring all parameters". The loss of data will be clearly visible to the applications. It would not go unnoted. In an ideal world, the Servlet spec would have opted for c) from the start. I wonder if it might not be better to revert this change for 10.1.x and earlier until the Servlet EG resolves #431 and then reconsider our options with (potentially) a new default behaviour in 11.0.x. If we don't revert then, of the current options: My concern with both b) and c) is that they could break applications that currently work. I don't like doing that if we don't have to in a point release. That leaves a). My main concern with a) is how to raise visibility of exceeding the limit. What if we changed the way UserDataHelper works (or introduced something new) that limited the number of log messages per period and thereby avoided the DoS risk via excessive logging but still generated enough log messages to raise awareness of the issue. Mark - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: Reduce default for maxParameterCount
ср, 15 мар. 2023 г. в 13:15, Konstantin Kolinko : > > [...] > > -1 unless the behaviour of "silently dropping extra parameters" is > changed as well. > > Silent loss of data is not what I want to see in production. > > Documentation [1] says "Request parameters beyond this limit will be ignored." > [1] https://tomcat.apache.org/tomcat-8.5-doc/config/http.html > [OT] A bit of humour, regarding loss of data. A fragment of Mel Brooks' movie "History of the World, Part I" https://www.youtube.com/watch?v=w556vrpsy4w Best regards, Konstantin Kolinko - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: Reduce default for maxParameterCount
ср, 15 мар. 2023 г. в 13:29, Konstantin Kolinko :
>
> ср, 15 мар. 2023 г. в 13:15, Konstantin Kolinko :
> >
> > ср, 15 мар. 2023 г. в 12:07, Mark Thomas :
> > >
> > > On 14/03/2023 21:13, Christopher Schultz wrote:
> > > > Mark,
> > > >
> > > > On 3/14/23 13:57, Mark Thomas wrote:
> > > >> On 09/03/2023 14:23, Christopher Schultz wrote:
> > > >>> Mark,
> > > > >
> > > > > []
> > > >
> > > > I would go for a 1000 limit for all currently-supported versions. It's
> > > > *very* easy to raise the limit if it interferes with a specific
> > > > application's functions.
> > > >
> > > > I *would* add an entry in the "notable changes" for each release e.g.
> > > > https://tomcat.apache.org/migration-10.1.html#Tomcat_10.1.x_noteable_changes
> > >
> > > Makes sense.
> > >
> > > I'll do that.
> >
> > -1 unless the behaviour of "silently dropping extra parameters" is
> > changed as well.
> >
> > Silent loss of data is not what I want to see in production.
> >
> > Documentation [1] says "Request parameters beyond this limit will be
> > ignored."
> > [1] https://tomcat.apache.org/tomcat-8.5-doc/config/http.html
> >
> > More details shortly.
>
> First, some notes regarding the source code.
>
> (1) The limit is enforced in
> org.apache.tomcat.util.http.Parameters#addParameter().
>
> It throws an IllegalStateException which is either caught and retrown
> later, or caught and swallowed.
>
> (2) I see the following 4 code paths and cases:
>
> []
>
> Proposals shortly.
Additional notes:
(4) In general, there are the following kinds of errors:
- decoding errors (format errors),
- hitting limits (such as max parameter count),
- i/o errors (client disconnect).
Looking at org.apache.tomcat.util.http.Parameters.FailReason (used by
FailedRequestFilter), it is
public enum FailReason {
CLIENT_DISCONNECT,
MULTIPART_CONFIG_INVALID,
INVALID_CONTENT_TYPE,
IO_ERROR,
NO_NAME,
POST_TOO_LARGE,
REQUEST_BODY_INCOMPLETE,
TOO_MANY_PARAMETERS,
UNKNOWN,
URL_DECODING
}
(5) Skipping parameters due to decoding errors has potential for abuse,
similar to a known trick used by phishers,
combining a well-known site name in an URL with junk. E.g.
http://www.microsoft.com.blabla.phisher.site
http://www.microsoft@blabla.phisher.site
E.g.
http://foo.bar?par1=...&par2=safety
and one is able to trigger ignoring "par2", it may be abused.
Proposals:
1. I think that maxParameterCount would better be configured per-Context.
The count of parameters is a property of a specific web application.
2. I wonder if we can make handling of the errors configurable.
I think that the following options are possible:
a) Drop parameters that exceeded the limit, or failed to decode.
b) If there is any error, ignore all parameters and behave as if none
were provided.
c) Blow up by throwing a RuntimeException for any call to
Request.getParameter() methods.
It may be an IllegalStateException.
My first thought was to go with c). I know that it contradicts with
Servlet API JavaDoc, but if it is configurable then it is a possible
option. I suppose that a web application should have error handling
configured and should be able to deal with errors.
If we go with c), it requires adding try/catch to safeguard
getParameter() calls in the following classes of Tomcat:
- org.apache.catalina.filters.FailedRequestFilter
- org.apache.catalina.valves.ExtendedAccessLogValve
(The ExtendedAccessLogValve can be configured to log the value of a parameter.)
3. I propose to change the default behaviour to b), "ignoring all parameters".
The loss of data will be clearly visible to the applications. It would
not go unnoted.
Even if one has not configured a FailedRequestFilter (or implemented
similar login in their own way) in their web application.
In case if no web application is matched to a request (no Context
configured), ignoring all parameters is a good default, as they won't
be processed.
Best regards,
Konstantin Kolinko
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: Reduce default for maxParameterCount
ср, 15 мар. 2023 г. в 13:15, Konstantin Kolinko : > > ср, 15 мар. 2023 г. в 12:07, Mark Thomas : > > > > On 14/03/2023 21:13, Christopher Schultz wrote: > > > Mark, > > > > > > On 3/14/23 13:57, Mark Thomas wrote: > > >> On 09/03/2023 14:23, Christopher Schultz wrote: > > >>> Mark, > > >>> > > >>> On 3/9/23 05:56, Mark Thomas wrote: > > Hi all, > > > > In the context of CVE-2023-24998 (performance issues for large > > numbers of uploaded parts), I have been wondering about reducing the > > default value for maxParameterCount. > > > > The current default for maxParameterCount is 10,000. It was set > > based on it being low enough to mitigate CVE-2012-0022 (hash > > collisions in parameter names triggering performance issues) while > > being so high it was considered extremely unlikely to impact any web > > application. > > >>> > > >>> Also relevant: maxPostSize and maxHttpRequestHeaderSize which help to > > >>> limit the total size of a request, regardless of the number of > > >>> parameters. > > >> > > >> I don't think we can lower those any further by default. If anything, > > >> the trend is towards making them larger. > > >> > > The current default is sufficiently low to mitigate CVE-2023-24998. > > > > There isn't any reason I am aware of that means we need to reduce > > the default for maxParameterCount. My thinking is more along the > > lines that when we last thought about this default in 2012, it was > > considered from the perspective of "How high can we set this and > > still be sure applications aren't exposed to CVE-2012-0022 or > > something like it?". If we consider it from the perspective of "How > > low can we make this without breaking many / most / (nearly) all > > applications?" I think we'll choose a much lower number. > > >>> > > >>> +1 > > >>> > > Another benefit of a lower number is to harden Tomcat in advance > > against future vulnerabilities like CVE-2023-24998. > > > > I was wondering about a new default of 1000 or maybe even 500. > > > > This would certainly be for 11.0.x. I think it should be back-ported > > but maybe in stages (5000, 3000, 2000, 1000) and/or delayed so it is > > reduced in 10.1.x for a few releases before we reduce it in 9.0.x > > and the a few more releases before we reduce it in 8.5.x. > > > > Thoughts? > > >>> > > >>> +1 for 1000. 500 seems insane to me but I'm sure there is some > > >>> application out there which uses 1000 parameters instead of JSON, > > >>> etc. for some reason. > > >> > > >> I've reduced the default to 1,000 for 11.0.x. > > >> > > >> Thoughts on if/how to back-port this to 10.1.x and friends? > > >> > > >> Straight to 1000 for all older versions? > > >> Straight to 1000 for 10.1.x then wait a few releases for each further > > >> backport? > > >> Or more cautious and backport a gradual reduction? > > > > > > I would go for a 1000 limit for all currently-supported versions. It's > > > *very* easy to raise the limit if it interferes with a specific > > > application's functions. > > > > > > I *would* add an entry in the "notable changes" for each release e.g. > > > https://tomcat.apache.org/migration-10.1.html#Tomcat_10.1.x_noteable_changes > > > > Makes sense. > > > > I'll do that. > > -1 unless the behaviour of "silently dropping extra parameters" is > changed as well. > > Silent loss of data is not what I want to see in production. > > Documentation [1] says "Request parameters beyond this limit will be ignored." > [1] https://tomcat.apache.org/tomcat-8.5-doc/config/http.html > > More details shortly. First, some notes regarding the source code. (1) The limit is enforced in org.apache.tomcat.util.http.Parameters#addParameter(). It throws an IllegalStateException which is either caught and retrown later, or caught and swallowed. (2) I see the following 4 code paths and cases: a) Request.getParameter() was called, and request was of type "application/x-www-form-urlencoded". - Code path: org.apache.tomcat.util.http.Parameters#addParameter() is called from org.apache.tomcat.util.http.Parameters#processParameters(...) In Parameters#processParameters(...) the IllegalStateException and IOException are caught, logged with UserDataHelper and swallowed. So the error is logged. b) Request.getPart(), and request was "application/x-www-form-urlencoded". JavaDoc for HttpServletRequest.#getPart() says that this is not allowed, and the expected result is a ServletException. c) Request.getPart(), and request was "multipart/form-data". - Code path: org.apache.tomcat.util.http.Parameters#addParameter() is called from org.apache.catalina.connector.Request.parseParts(...) is called from org.apache.catalina.connector.Request.getPart(...) Request.parseParts(...) catches the ISE, stores it. Request.getPart(...) rethrows the caught ISE. The API of Request.getPart(...) allows rethrowing the
Re: Reduce default for maxParameterCount
ср, 15 мар. 2023 г. в 12:07, Mark Thomas : > > On 14/03/2023 21:13, Christopher Schultz wrote: > > Mark, > > > > On 3/14/23 13:57, Mark Thomas wrote: > >> On 09/03/2023 14:23, Christopher Schultz wrote: > >>> Mark, > >>> > >>> On 3/9/23 05:56, Mark Thomas wrote: > Hi all, > > In the context of CVE-2023-24998 (performance issues for large > numbers of uploaded parts), I have been wondering about reducing the > default value for maxParameterCount. > > The current default for maxParameterCount is 10,000. It was set > based on it being low enough to mitigate CVE-2012-0022 (hash > collisions in parameter names triggering performance issues) while > being so high it was considered extremely unlikely to impact any web > application. > >>> > >>> Also relevant: maxPostSize and maxHttpRequestHeaderSize which help to > >>> limit the total size of a request, regardless of the number of > >>> parameters. > >> > >> I don't think we can lower those any further by default. If anything, > >> the trend is towards making them larger. > >> > The current default is sufficiently low to mitigate CVE-2023-24998. > > There isn't any reason I am aware of that means we need to reduce > the default for maxParameterCount. My thinking is more along the > lines that when we last thought about this default in 2012, it was > considered from the perspective of "How high can we set this and > still be sure applications aren't exposed to CVE-2012-0022 or > something like it?". If we consider it from the perspective of "How > low can we make this without breaking many / most / (nearly) all > applications?" I think we'll choose a much lower number. > >>> > >>> +1 > >>> > Another benefit of a lower number is to harden Tomcat in advance > against future vulnerabilities like CVE-2023-24998. > > I was wondering about a new default of 1000 or maybe even 500. > > This would certainly be for 11.0.x. I think it should be back-ported > but maybe in stages (5000, 3000, 2000, 1000) and/or delayed so it is > reduced in 10.1.x for a few releases before we reduce it in 9.0.x > and the a few more releases before we reduce it in 8.5.x. > > Thoughts? > >>> > >>> +1 for 1000. 500 seems insane to me but I'm sure there is some > >>> application out there which uses 1000 parameters instead of JSON, > >>> etc. for some reason. > >> > >> I've reduced the default to 1,000 for 11.0.x. > >> > >> Thoughts on if/how to back-port this to 10.1.x and friends? > >> > >> Straight to 1000 for all older versions? > >> Straight to 1000 for 10.1.x then wait a few releases for each further > >> backport? > >> Or more cautious and backport a gradual reduction? > > > > I would go for a 1000 limit for all currently-supported versions. It's > > *very* easy to raise the limit if it interferes with a specific > > application's functions. > > > > I *would* add an entry in the "notable changes" for each release e.g. > > https://tomcat.apache.org/migration-10.1.html#Tomcat_10.1.x_noteable_changes > > Makes sense. > > I'll do that. -1 unless the behaviour of "silently dropping extra parameters" is changed as well. Silent loss of data is not what I want to see in production. Documentation [1] says "Request parameters beyond this limit will be ignored." [1] https://tomcat.apache.org/tomcat-8.5-doc/config/http.html More details shortly. Best regards, Konstantin Kolinko - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: Reduce default for maxParameterCount
On 14/03/2023 21:13, Christopher Schultz wrote: Mark, On 3/14/23 13:57, Mark Thomas wrote: On 09/03/2023 14:23, Christopher Schultz wrote: Mark, On 3/9/23 05:56, Mark Thomas wrote: Hi all, In the context of CVE-2023-24998 (performance issues for large numbers of uploaded parts), I have been wondering about reducing the default value for maxParameterCount. The current default for maxParameterCount is 10,000. It was set based on it being low enough to mitigate CVE-2012-0022 (hash collisions in parameter names triggering performance issues) while being so high it was considered extremely unlikely to impact any web application. Also relevant: maxPostSize and maxHttpRequestHeaderSize which help to limit the total size of a request, regardless of the number of parameters. I don't think we can lower those any further by default. If anything, the trend is towards making them larger. The current default is sufficiently low to mitigate CVE-2023-24998. There isn't any reason I am aware of that means we need to reduce the default for maxParameterCount. My thinking is more along the lines that when we last thought about this default in 2012, it was considered from the perspective of "How high can we set this and still be sure applications aren't exposed to CVE-2012-0022 or something like it?". If we consider it from the perspective of "How low can we make this without breaking many / most / (nearly) all applications?" I think we'll choose a much lower number. +1 Another benefit of a lower number is to harden Tomcat in advance against future vulnerabilities like CVE-2023-24998. I was wondering about a new default of 1000 or maybe even 500. This would certainly be for 11.0.x. I think it should be back-ported but maybe in stages (5000, 3000, 2000, 1000) and/or delayed so it is reduced in 10.1.x for a few releases before we reduce it in 9.0.x and the a few more releases before we reduce it in 8.5.x. Thoughts? +1 for 1000. 500 seems insane to me but I'm sure there is some application out there which uses 1000 parameters instead of JSON, etc. for some reason. I've reduced the default to 1,000 for 11.0.x. Thoughts on if/how to back-port this to 10.1.x and friends? Straight to 1000 for all older versions? Straight to 1000 for 10.1.x then wait a few releases for each further backport? Or more cautious and backport a gradual reduction? I would go for a 1000 limit for all currently-supported versions. It's *very* easy to raise the limit if it interferes with a specific application's functions. I *would* add an entry in the "notable changes" for each release e.g. https://tomcat.apache.org/migration-10.1.html#Tomcat_10.1.x_noteable_changes Makes sense. I'll do that. Mark - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: Reduce default for maxParameterCount
Mark, On 3/14/23 13:57, Mark Thomas wrote: On 09/03/2023 14:23, Christopher Schultz wrote: Mark, On 3/9/23 05:56, Mark Thomas wrote: Hi all, In the context of CVE-2023-24998 (performance issues for large numbers of uploaded parts), I have been wondering about reducing the default value for maxParameterCount. The current default for maxParameterCount is 10,000. It was set based on it being low enough to mitigate CVE-2012-0022 (hash collisions in parameter names triggering performance issues) while being so high it was considered extremely unlikely to impact any web application. Also relevant: maxPostSize and maxHttpRequestHeaderSize which help to limit the total size of a request, regardless of the number of parameters. I don't think we can lower those any further by default. If anything, the trend is towards making them larger. The current default is sufficiently low to mitigate CVE-2023-24998. There isn't any reason I am aware of that means we need to reduce the default for maxParameterCount. My thinking is more along the lines that when we last thought about this default in 2012, it was considered from the perspective of "How high can we set this and still be sure applications aren't exposed to CVE-2012-0022 or something like it?". If we consider it from the perspective of "How low can we make this without breaking many / most / (nearly) all applications?" I think we'll choose a much lower number. +1 Another benefit of a lower number is to harden Tomcat in advance against future vulnerabilities like CVE-2023-24998. I was wondering about a new default of 1000 or maybe even 500. This would certainly be for 11.0.x. I think it should be back-ported but maybe in stages (5000, 3000, 2000, 1000) and/or delayed so it is reduced in 10.1.x for a few releases before we reduce it in 9.0.x and the a few more releases before we reduce it in 8.5.x. Thoughts? +1 for 1000. 500 seems insane to me but I'm sure there is some application out there which uses 1000 parameters instead of JSON, etc. for some reason. I've reduced the default to 1,000 for 11.0.x. Thoughts on if/how to back-port this to 10.1.x and friends? Straight to 1000 for all older versions? Straight to 1000 for 10.1.x then wait a few releases for each further backport? Or more cautious and backport a gradual reduction? I would go for a 1000 limit for all currently-supported versions. It's *very* easy to raise the limit if it interferes with a specific application's functions. I *would* add an entry in the "notable changes" for each release e.g. https://tomcat.apache.org/migration-10.1.html#Tomcat_10.1.x_noteable_changes -chris - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: Reduce default for maxParameterCount
On 09/03/2023 14:23, Christopher Schultz wrote: Mark, On 3/9/23 05:56, Mark Thomas wrote: Hi all, In the context of CVE-2023-24998 (performance issues for large numbers of uploaded parts), I have been wondering about reducing the default value for maxParameterCount. The current default for maxParameterCount is 10,000. It was set based on it being low enough to mitigate CVE-2012-0022 (hash collisions in parameter names triggering performance issues) while being so high it was considered extremely unlikely to impact any web application. Also relevant: maxPostSize and maxHttpRequestHeaderSize which help to limit the total size of a request, regardless of the number of parameters. I don't think we can lower those any further by default. If anything, the trend is towards making them larger. The current default is sufficiently low to mitigate CVE-2023-24998. There isn't any reason I am aware of that means we need to reduce the default for maxParameterCount. My thinking is more along the lines that when we last thought about this default in 2012, it was considered from the perspective of "How high can we set this and still be sure applications aren't exposed to CVE-2012-0022 or something like it?". If we consider it from the perspective of "How low can we make this without breaking many / most / (nearly) all applications?" I think we'll choose a much lower number. +1 Another benefit of a lower number is to harden Tomcat in advance against future vulnerabilities like CVE-2023-24998. I was wondering about a new default of 1000 or maybe even 500. This would certainly be for 11.0.x. I think it should be back-ported but maybe in stages (5000, 3000, 2000, 1000) and/or delayed so it is reduced in 10.1.x for a few releases before we reduce it in 9.0.x and the a few more releases before we reduce it in 8.5.x. Thoughts? +1 for 1000. 500 seems insane to me but I'm sure there is some application out there which uses 1000 parameters instead of JSON, etc. for some reason. I've reduced the default to 1,000 for 11.0.x. Thoughts on if/how to back-port this to 10.1.x and friends? Straight to 1000 for all older versions? Straight to 1000 for 10.1.x then wait a few releases for each further backport? Or more cautious and backport a gradual reduction? Mark - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: Reduce default for maxParameterCount
Mark, On 3/9/23 05:56, Mark Thomas wrote: Hi all, In the context of CVE-2023-24998 (performance issues for large numbers of uploaded parts), I have been wondering about reducing the default value for maxParameterCount. The current default for maxParameterCount is 10,000. It was set based on it being low enough to mitigate CVE-2012-0022 (hash collisions in parameter names triggering performance issues) while being so high it was considered extremely unlikely to impact any web application. Also relevant: maxPostSize and maxHttpRequestHeaderSize which help to limit the total size of a request, regardless of the number of parameters. The current default is sufficiently low to mitigate CVE-2023-24998. There isn't any reason I am aware of that means we need to reduce the default for maxParameterCount. My thinking is more along the lines that when we last thought about this default in 2012, it was considered from the perspective of "How high can we set this and still be sure applications aren't exposed to CVE-2012-0022 or something like it?". If we consider it from the perspective of "How low can we make this without breaking many / most / (nearly) all applications?" I think we'll choose a much lower number. +1 Another benefit of a lower number is to harden Tomcat in advance against future vulnerabilities like CVE-2023-24998. I was wondering about a new default of 1000 or maybe even 500. This would certainly be for 11.0.x. I think it should be back-ported but maybe in stages (5000, 3000, 2000, 1000) and/or delayed so it is reduced in 10.1.x for a few releases before we reduce it in 9.0.x and the a few more releases before we reduce it in 8.5.x. Thoughts? +1 for 1000. 500 seems insane to me but I'm sure there is some application out there which uses 1000 parameters instead of JSON, etc. for some reason. A potential use-case for "large numbers of parameters" might be an application that uses something like a multi-select list and the number of choices is stupendously high. As in, when the application was designed, the designers said "we can use a multi-select list for this postal-code selector" and then 5-10 years later, someone said "hey let's dump all postal codes in the entire US into this multi-select" and there you have thousands of possibilities which the browser will happily package-off to the server. -chris - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Reduce default for maxParameterCount
Hi all, In the context of CVE-2023-24998 (performance issues for large numbers of uploaded parts), I have been wondering about reducing the default value for maxParameterCount. The current default for maxParameterCount is 10,000. It was set based on it being low enough to mitigate CVE-2012-0022 (hash collisions in parameter names triggering performance issues) while being so high it was considered extremely unlikely to impact any web application. The current default is sufficiently low to mitigate CVE-2023-24998. There isn't any reason I am aware of that means we need to reduce the default for maxParameterCount. My thinking is more along the lines that when we last thought about this default in 2012, it was considered from the perspective of "How high can we set this and still be sure applications aren't exposed to CVE-2012-0022 or something like it?". If we consider it from the perspective of "How low can we make this without breaking many / most / (nearly) all applications?" I think we'll choose a much lower number. Another benefit of a lower number is to harden Tomcat in advance against future vulnerabilities like CVE-2023-24998. I was wondering about a new default of 1000 or maybe even 500. This would certainly be for 11.0.x. I think it should be back-ported but maybe in stages (5000, 3000, 2000, 1000) and/or delayed so it is reduced in 10.1.x for a few releases before we reduce it in 9.0.x and the a few more releases before we reduce it in 8.5.x. Thoughts? Mark - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
