From: "Mark Thomas" <[EMAIL PROTECTED]>
So the issues are: 1. AJP/1.3 compatibility 2. Potential DoS
As far as DoS goes, with the previous behaviour any parameters POSTed would be persisted in the session until the authentication was completed or the session timed out. Therefore, the same issue exists with both the old and new implementation. (a)
The size of the the POST is already limited by maxPostSize for both FORM and CLIENT-CERT auth. Is the proposal to add another parameter to the connector to optionally further limit the saved POST size when authenticating? (b)
The check on maxPostSize in the Request isn't applied to any 'chunked' POST body, and also not to any 'multipart/form-data'. I don't see any place else that checks it except when CLIENT-CERT auth saves the request body.
I stand corrected. This is easy to fix if it is agreed that this, or something similar to it, is the way forward.
Given (a), I don't see a significant difference in risk between the old and new behaviour. I am happy to mitigate this risk by implementing (b). As maxPostSize applies to any POST, including during CLIENT-CERT auth my own view is that the new parameter should apply only to the FormAuthenticator valve and should default to 0 (ie no data saved). -1 would mean use whatever value is specified for maxPostSize and any value
0 would be the limit unless maxPostSize was smaller in which case
maxPostSize would override the new parameter. The docs for this parameter would include a health warning about the risks of permitting the saving of POSTed data during FORM authentication.
No. Previously only the Parameters were saved, and limited by maxPostSize. Now you are saving off file-upload posts as well, and these aren't limited anywhere.
As above, putting the limit in is easy.
I obviously also need to look at AJP/1.3 compatibility. Any hints/tips gratefully received.
It should be something like: request.getCoyoteRequest().action(ActionCode.ACTION_SET_BODY_REPLAY, body);
but that won't work either unless Jk-Coyote gets cleaned up a bit (the ActionHook implementation is one of those "it's ugly but it works" things at the moment :). I could do the cleanup if the consensus is that this is the way to go.
Any help would be great. It took me a while to figure out how to get this far.
If these issues aren't resolved by the time of the next release, I'll revert the saving the raw data part of the patch.
Mark
--------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
