Ron Monzillo <ron.monzillo_at_oracle.com> wrote:
>Mark,
>
>you once wrote:
Please try and keep conversations in the correct thread. Having gone to the trouble to separate the two issues, you've just mixed them back up.
>> In summary, I agree that there is a problem but this proposal seems
>to
>> be trading one problem for a potentially more complex one. I think it
>is
>> worth considering if there is a simpler solution to the problem.
>>
>> Alternatively, we could treat this purely as a user education issue
>that
>> needs a clear explanation of the potential pitfall in the spec and
>some
>> guidance on how to avoid it.
>I don't think documentation alone can fix this problem; especially
>where
>developers get their information
>from lots of places, but rarely from the Servlet spec, and where there
>is already a significant population
>of mis-configured applications that are in need of remediation.
>
>While I agree that the proposal would create more work for container
>vendors, its objective was to make it easy
>for application developers and deployers to repair existing
>misconfigured applications, and to create new applications
>in which there are no uncovered http methods.
I don't see the proposal as making things easier for anyone - container developers or application developers. I have read your proposal several times and I am still not sure I fully understand it. Hence, I am far from convinced that developers will understand what the option does, let alone use it.
If it wasn't for annotations, mis-configured applications could be fixed by simply deleting the <http-method> elements in web.xml. Annotations complicate things rather in this case. There are ways around this but they aren't as simple.
>With the switch enabled, no changes would be made to the constraint
>configuration of an application with no
>uncovered methods. This would be consistent with a recommendation that
>all http methods be covered
>at all of an application's constrained url patterns.
>
>I would like to be able to require that the constraint combining system
>
>always report the existence of uncovered methods, which
>would serve to educate developers on the proper configuration of
>constraints. When the flag is set it would cause the
>additional effect of locking down all the uncovered methods.
>
>I don't see how this would in any way make the system more difficult
>for
>the application developer, or the deployer; and as I
>said, I think it is going to take more than documentation to resolve
>this problem.
>
>Beyond simply providing better documentation and examples, what would
>you think a simpler solution for developers
>would look like?
A configuration setting (configuration details TBD) that provided three options:
a) do nothing - for those developers that know what they are doing
b) log use of <http-method> and equivalent as a warning that they should be reviewed
c) same as b) plus any <http-method> elements and equivalents are ignored.
Plus better examples and documentation of course.
What would be useful (to me at least to help understand your proposal) would be an explanation (with full examples) of any cases where the end result of your proposal and option c) above differ.
Mark
>
>Ron
>
>ps: fwiw, I don't think that we can ignore the verb tampering
>vulnerabilities that Jeff cited. I don't think we own them, but
>I think we can do more to ensure that apps deployed on our platforms
>are
>not susceptible to them.
I agree there is work for the JSP EG /JSP spec implementers. I haven't yet seen anything that convinces me there is anything the Servlet EG should/could do in this area.
>On 9/14/12 1:01 PM, Mark Thomas wrote:
>> On 13/09/2012 20:16, Jeff Williams wrote:
>>>> For "verb tampering" to be an issue an application/framework has to
>do
>>> the following:
>>>> 1) Override one of the service() methods of the default HttpServlet
>>> and add support for the new verb.
>>>> 2) Implement that verb by calling one of the other doXXX methods.
>>>> 3) Protect the XXX verb with a security constraint but leave the
>new
>>> verb unprotected.
>>>
>>>> Regarding 1), the Javadoc makes it very clear that this is almost
>>> certainly a bad idea.
>>>
>>> Unfortunately, there's a certain amount of overlap between the
>verbs.
>>> To implement the HEAD request the container will execute a GET and
>then
>>> strip off the HTTP response body that was generated. This confuses
>the
>>> issue of which methods to "allow" as an attacker can now send a HEAD
>to
>>> bypass the constraint.
>> I'm not concerned about that. If a developer protects GET and not
>HEAD
>> then that is their responsibility. Given the definition of HEAD, I
>don't
>> see an issue with an implementation that calls doGet() in doHead().
>>
>>> For new verbs, the Javadoc is also, unfortunately, irrelevant
>because
>>> JSPs respond to any verb. Many JSPs have interesting business logic
>in
>>> them (unfortunately), and others disclose valuable information.
>That
>>> makes it easy to bypass any security-constraints that include
>methods.
>> That is a problem with the JSP implementations, not the Servlet spec.
>> The JSP spec certainly suggests using service() rather than doXXX()
>but
>> it doesn't require it. Neither could I see any discussion of HTTP
>> methods in the JSP spec. It looks to be possible for JSP
>implementations
>> to respond to a subset of verbs (GET, HEAD, POST?) and reject the
>rest.
>> That list of verbs needs to be configurable. If this approach is
>viable,
>> then a useful addition to the JSP spec would be a way a defining
>which
>> verbs a JSP should respond to. Further discussion of this point
>really
>> belongs with the JSP EG.
>>
>> I'm not sure of the current status of the JSP EG. Does anyone here
>know?
>>
>>>> Regarding 2), one has to ask why on earth the new verb was
>introduced
>>> in the first place.
>>>
>>> A developer probably wouldn't, but an attacker certainly would.
>That's
>>> why we called it tampering.
>> Excluding the JSP issue which is out of scope for the Servlet EG, the
>> developer has to do something to introduce support for the verb.
>Hence
>> my view that they need to take responsibility for that.
>>
>>
>>>> Regarding 3), given the hoops a developer as to jump through and
>the
>>> warnings they
>>>> have to have ignored to reach this point then responsibility for
>>> failing to secure the
>>>> result properly lies squarely with the developer.
>>> I just don't see any real blocks to making a security mistake with
>this.
>>> The only possibility is that they read and understand the spec,
>which is
>>> demonstrably confusing.
>> Then argue for clarifying the text and the examples, not for making
>the
>> rules even more complex.
>>
>>>> I am strongly against making any changes to the Servlet
>specification.
>>>> The previous EGs have done more than enough to make clear the
>dangers
>>> of this approach.
>>>> If application/framework developers insist on ignoring that advice
>>> then they have to take responsibility.
>>>
>>> As I've mentioned, I would strongly prefer the approach where the
>>> methods listed are the only ones allowed.
>> This is supported in web.xml. Folks just have to use it.
>>
>> We could remove support for the deny option although that won't be
>> backwards compatible so I suspect it will never happen.
>>
>> In the meantime, use of deny could be strongly discouraged and
>> containers are free to log warnings if users use the deny rather than
>> allow approach.
>>> I believe that makes sense
>>> and is what developers expect. However, I think Ron's proposal is
>>> better than leaving in place the confusing AND insecure approach in
>the
>>> current spec.
>> On that, we disagree.
>>
>> Mark
>>
>>> --Jeff
>>>
>>>
>>> -----Original Message-----
>>> From: Mark Thomas [mailto:markt_at_apache.org]
>>> Sent: Thursday, September 13, 2012 2:35 PM
>>> To: jsr340-experts_at_servlet-spec.java.net
>>> Cc: Jeff Williams
>>> Subject: Re: [servlet-spec users] [jsr340-experts] Verb tampering
>Was:
>>> Digest for list jsr340-experts_at_servlet-spec.java.net
>>>
>>> On 11/09/2012 03:14, Jeff Williams wrote:
>>>> The paper about verb tampering and why you should care is here.
>>>>
>https://www.aspectsecurity.com/wp-content/plugins/download-monitor/dow
>>>> nl
>>>> oad.php?id=18.
>>>>
>>>> For those who just want a quick summary, verb tampering is a big
>deal.
>>>> In our security testing, we have used it to force access to
>critical
>>>> administrative functionality. Literally the only thing protecting
>>>> these functions is the secrecy of the URL, which is frequently
>>>> guessable or brute forceable. To access a "protected" JSP, all the
>>>> attacker has to do is change the verb to JEFF. To invoke a
>"protected"
>>>> servlet, they can just send a HEAD request. For example, at
>attacker
>>>> could forge a request to a bank admin page to change a user's
>>>> password, then drain their bank account.
>>>>
>>>> To me, leaving this unfixed is unthinkable. My initial suggestion
>to
>>>> Ron was to change to a default deny semantic. But he eventually
>>>> convinced me that the expert group would never go for it because it
>>>> would likely break many apps. Leaving this a documentation issue
>>>> simply ensures that many applications will remain vulnerable.
>>> This is absolutely not a issue for the Servlet specification. It may
>be
>>> an issue for an implementation but I would be very surprised if any
>of
>>> the implementations have done something sufficiently stupid for this
>to
>>> be an problem.
>>>
>>> The default HttpServlet implementation has a separate method for
>each
>>> HTTP verb.
>>>
>>> The default implementation of every single one of those methods is
>to
>>> return a 405 response to HTTP/1.1 requests and 400 for HTTP/1.0 and
>>> HTTP/0.9 requests.
>>>
>>> Unrecognised HTTP verbs receive a 501 response.
>>>
>>> For "verb tampering" to be an issue an application/framework has to
>do
>>> the following:
>>> 1) Override one of the service() methods of the default HttpServlet
>and
>>> add support for the new verb.
>>>
>>> 2) Implement that verb by calling one of the other doXXX methods.
>>>
>>> 3) Protect the XXX verb with a security constraint but leave the new
>>> verb unprotected.
>>>
>>> Regarding 1), the Javadoc makes it very clear that this is almost
>>> certainly a bad idea.
>>>
>>> Regarding 2), one has to ask why on earth the new verb was
>introduced in
>>> the first place.
>>>
>>> Regarding 3), given the hoops a developer as to jump through and the
>>> warnings they have to have ignored to reach this point then
>>> responsibility for failing to secure the result properly lies
>squarely
>>> with the developer.
>>>
>>> I am strongly against making any changes to the Servlet
>specification.
>>> The previous EGs have done more than enough to make clear the
>dangers of
>>> this approach. If application/framework developers insist on
>ignoring
>>> that advice then they have to take responsibility.
>>>
>>> Mark
>>>