Got it. It seems to make sense to me to add this, then.
Jason, any thoughts?
On 18 Dec 2012, at 20:22, Ron Monzillo wrote:
> On 12/18/12 6:37 AM, Pete Muir wrote:
>> Hi Bill,
>>
>> In general, it would be really helpful if you could upgrade the examples to show what effect the new element would have. IOW, provide an example of the problem today, and then how the new element fixes it.
>>
> Hi Pete,
>
> When the flag, is set an additional constraint would be generated to apply
> a DENY semantic to the (presumably) uncovered methods.
>
> Example 1:
>
> original constraint:
> <security-constraint>
> <web-resource-collection>
> <url-pattern>/a/b/c/*</url-pattern>
> <http-method>GET</http-method>
> <http-method>POST</http-method>
> </web-resource-collection>
> <auth-constraint>
> <role-name>user</role-name>
> </auth-constraint>
> </security-constraint>
>
> additional constraint (to deny all methods except GET and POST)
> <security-constraint>
> <web-resource-collection>
> <url-pattern>
> /a/b/c/
> *</url-pattern>
> <http-method-omission>GET</http-method-omission>
> <http-method-omission>POST</http-method-omission>
> </web-resource-collection>
> <auth-constraint/>
> </security-constraint>
>
> Example 2:
>
> original constraint
>
> <security-constraint>
> <web-resource-collection>
> <url-pattern>/d/e/f/*</url-pattern>
> <http-method-omission>GET</http-method-omission>
> <http-method-omission>POST</http-method-omission>
> </web-resource-collection>
> <auth-constraint/>
> </security-constraint>
>
> additional constraint (to allow GET and POST with no protection requirements)
>
> <security-constraint>
> <web-resource-collection>
> <url-pattern>/d/e/f/*</url-pattern>
> <http-method>GET</http-method>
> <http-method>POST</http-method>
> </web-resource-collection>
> <auth-constraint/>
> </security-constraint>
>
> Example 3 is analogous to example 1
> Example 4 is analogous to example 2
>>
>> So, I may be wrong about the below, but I think it's what is proposed....
>>
> you are correct
>>
>> What I find very odd about the proposal is the way it affects "by omission" approach. In fact, I think this may make the proposed solution essentially useless. If I did:
>>
>> <security-constraint>
>> <web-resource-collection>
>> <url-pattern>/d/e/f/*</url-pattern>
>> <http-method-omission>GET</http-method-omission>
>> <http-method-omission>POST</http-method-omission>
>> </web-resource-collection>
>> <auth-constraint/>
>> </security-constraint>
>>
>> IMO I would expect that the intention of the developer would be that GET and POST are uncovered.
>>
> The intention of the developer is that all but GET and POST are to be denied.
> This constraint says nothing definitive about how GET and POST should be treated.
> The proposal is our response to the realization that developers are generally not aware that
> uncovered methods are not protected.
>
>> If under the proposal I then enabled "deny-uncovered-http-methods" both GET and POST become covered, which was not my intention, and I have no way to tell the container "no really, I do want these methods uncovered"
> you could and should define a constraint that covers GET and POST (to provide
> whatever level of protection including none, that you wish to apply to GET and POST).
> Doing so will ensure that all methods are being treated according to your specification.
>
> for example (to allow GET and POST without requiring protection)
> <security-constraint>
> <web-resource-collection>
> <url-pattern>/d/e/f/*</url-pattern>
> <http-method>GET</http-method>
> <http-method>POST</http-method>
> </web-resource-collection>
> </security-constraint>
>
> When constraints are defined on individual methods, we are basically saying that you should
> be explicit about all methods. If you prefer the "benefit" of the shorthands, then
> you have the option of deploying without setting flag.
>
> independent of support for the flag, we are advocating the following use model
>
> 1. (unless there is some reason why you must apply different security
> properties to different HTTP methods at a common pattern) don't declare
> HTTP methods in security constraints
>
> 2. if HTTP methods are declared in a security constraint, make sure you
> define additional security constraints to cover all of the other methods
> at all of the url-patterns for which HTTP methods were declared.
>
> Ron
>
>>
>> So, i would suggest modifying the proposal so that it only covers resources that aren't explicitly uncovered as above (in other words not changing example 3, and the equivalent in annotations).
>>
>> HTH
>>
>>
>>
>> On 18 Dec 2012, at 05:14, Bill Shannon wrote:
>>
>>
>>> An issue came up in the Servlet expert group that I'd like to escalate
>>> to this group.
>>>
>>> There's a potential security issue in the Servlet spec in the way that
>>> permissions are associated with HTTP methods. This issue was raised by
>>> Jeff Williams of Aspect Security and OWASP (
>>> http://www.owasp.og
>>> ). Ron
>>> Monzillo on our security team worked with Jeff to propose a solution to
>>> the Servlet expert group. The reaction from the Servlet expert group was
>>> (paraphrasing) that this was not an important issue and that people who
>>> understood the Servlet spec would not make this mistake. They seemed to
>>> think the solution was worse than the problem and rejected Ron's proposal.
>>> (If any of you are also members of the Servlet expert group, I'd like to
>>> hear your perspective.)
>>>
>>> I'd prefer not to be in the situation where an independent group has
>>> pointed out a valid security concern with the Java EE platform and
>>> we've chosen not to fix it. I'd like to get your feedback...
>>>
>>> - Is this a valid concern?
>>>
>>> - Should we provide a "fix" in the Servlet spec?
>>>
>>> - Is the proposed fix acceptable?
>>>
>>> I've included Ron's original description of the problem and the
>>> proposed fix below. If there's support in this group for fixing
>>> this problem in the Servlet spec, we'll take this back to the Servlet
>>> expert group and ask them to reconsider. Those of you with reps on
>>> the Servlet expert group may need to help.
>>>
>>> I'm looking forward to your feedback.
>>>
>>> Thanks.
>>>
>>>
>>> -----
>>>
>>> Experts,
>>>
>>> This proposal was developed in response to a recommendation made by Jeff
>>> Williams of Aspect and OWASP.
>>>
>>> The following article from Aspect Security describes the issue.
>>>
>>>
>>> https://www.aspectsecurity.com/wp-content/plugins/download-monitor/download.php?id=18
>>>
>>>
>>> The Servlet constraint grammar provides the ability to enumerate
>>> (including by omission) the HTTP methods to which the protection
>>> requirements defined in a constraint apply. When HTTP methods are
>>> enumerated within a constraint definition, the protections defined
>>> by the constraint apply only to the methods established by the
>>> enumeration. We refer to the HTTP methods that are not established
>>> by the enumeration as "uncovered" methods.
>>>
>>> When HTTP methods are not enumerated within a constraint, the protections
>>> defined by the constraint apply to the complete set of HTTP (extension)
>>> methods. In that case, there are no uncovered HTTP methods at the
>>> url-patterns to which the constraint applies.
>>>
>>> Servlet requires that a default permit semantic be associated with
>>> uncovered methods (and with unconstrained url-patterns).
>>>
>>> Developers need to understand these rules in order to use the
>>> security-constraint grammar effectively. Unfortunately it is easy to find
>>> examples of security-constraint specifications that, as a result of HTTP
>>> method enumeration, are vulnerable as described in the cited reference.
>>>
>>> To address this issue, we are proposing that a new element be defined for
>>> use in web.xml that when specified, would cause a deny semantic to be
>>> applied to uncovered HTTP methods.
>>>
>>> This new element would be defined within the web-common_3_0.xsd.xml
>>> schema (with version corrected appropriately) as follows:
>>>
>>> <xsd:complexType name="web-appType">
>>> <xsd:choice minOccurs="0" maxOccurs="unbounded">
>>> <xsd:element name="module-name" type="javaee:string" minOccurs="0"/>
>>> <xsd:group ref="javaee:web-commonType"/>
>>> <xsd:element name="absolute-ordering" type="javaee:absoluteOrderingType"/>
>>> <xsd:element name="deny-uncovered-http-methods" type="javaee:emptyType">
>>> <xsd:annotation>
>>> <xsd:documentation>
>>>
>>> When specified, this element causes uncovered http methods
>>> to be denied. For every url-pattern that is the target of a
>>> security-constrant, this element causes all HTTP methods that
>>> are not covered (by a security constraint) at the url-pattern
>>> to be denied.
>>>
>>> </xsd:documentation>
>>> </xsd:annotation>
>>> </xsd:element>
>>> </xsd:choice>
>>> <xsd:attributeGroup ref="javaee:web-common-attributes"/>
>>> </xsd:complexType>
>>>
>>> This element would only be available via the top level deployment descriptor,
>>> web.xml. When specified in web.xml, this element would apply to all security
>>> constraints defined for the web application; including any defined in fragments.
>>> This element would apply to uncovered methods resulting from the specification
>>> of security constraints via the web.xml and web-fragment deployment
>>> descriptors, from the use of the @ServletSecurity annotation, and from the use
>>> of the setServletSecurity method of the ServletRegistration interface.
>>>
>>> This flag would provide a means to ensure that uncovered methods could not be
>>> the cause of a security breach.
>>>
>>> The examples that follow demonstrate how methods may be left uncovered. The
>>> determination of whether methods are uncovered is made after all the
>>> constraints that apply to the pattern have been combined as described
>>> in "Section 13.8.1 Combining Constraints"
>>>
>>> To implement this flag it will be necessary for it to effect the processing
>>> of the @ServletSecurity annotation and the ServletSecurityElement such
>>> that methods uncovered in these forms will retain their uncovered status
>>> into constraint combination, after which a deny semantic can be associated
>>> with any remaining uncovered methods.
>>>
>>> When this flag has not been specified the existing semantics will be retained.
>>>
>>> Comments?
>>>
>>> Ron
>>>
>>> More details
>>>
>>> EXAMPLES OF UNCOVERED METHODS
>>>
>>> Example 1: Security-constraint that enumerates HTTP methods using http-method
>>> elements, All HTTP methods except GET and POST are uncovered (at the
>>> url-patterns to which the constraint applies)
>>>
>>> [<security-constraint>
>>> <web-resource-collection>
>>> <url-pattern>/a/b/c/*</url-pattern>
>>> <http-method>GET</http-method>
>>> <http-method>POST</http-method>
>>> </web-resource-collection>
>>> <auth-constraint>
>>> <role-name>user</role-name>
>>> </auth-constraint>
>>> </security-constraint>
>>>
>>> Example 2: Security-constraint that enumerates HTTP methods using
>>> http-method-omission elements, The ommitted HTTP methods, GET and POST, are
>>> uncovered (at the url-patterns to which the constraint applies)
>>>
>>> <security-constraint>
>>> <web-resource-collection>
>>> <url-pattern>/d/e/f/*</url-pattern>
>>> <http-method-omission>GET</http-method-omission>
>>> <http-method-omission>POST</http-method-omission>
>>> </web-resource-collection>
>>> <auth-constraint/>
>>> </security-constraint>
>>>
>>> Example 3: @ServletSecurity annotation that contains one or more HTTP
>>> method-specific @HttpMethodConstraint elements and that either implicitly
>>> or explicitly establishes a default HTTP method-independent @HttpConstraint
>>> (value = EmptyRoleSemantic.PERMIT, rolesAllowed = {}, and
>>> TransportGuarantee = NONE) to be applied to all other HTTP methods. Like
>>> Example 1, all methods except those that are the target of an
>>> @HttpMethodConstraint are uncovered (at the url-patterns to which the
>>> constraint applies).
>>>
>>> [3a] implicit method-independent @HttpConstraint
>>> (value = EmptyRoleSemantic.PERMIT, rolesAllowed = {}, and
>>> TransportGuarantee = NONE)
>>>
>>> @ServletSecurity(
>>> httpMethodConstraints = {
>>> @HttpMethodConstraint(value = "GET", rolesAllowed = "user"),
>>> @HttpMethodConstraint(value = "POST", rolesAllowed = "user"),
>>> })
>>>
>>> [3b] explicit method-independent @HttpConstraint that establishes a default
>>> HTTP method-independent @HttpConstraint (value = EmptyRoleSemantic.PERMIT,
>>> rolesAllowed = {}, and TransportGuarantee = NONE). Note that there are some
>>> other explicit ways to specify the equivalent of the default HTTP
>>> method-indpendent @HttpConstraint.
>>>
>>> @ServletSecurity(
>>> value = @HttpConstraint(value = EmptyRoleSemantic.PERMIT,
>>> rolesAllowed = {}, transportGuarantee = TransportGuarantee.NONE),
>>> httpMethodConstraints = {
>>> @HttpMethodConstraint(value = "GET", rolesAllowed = "user"),
>>> @HttpMethodConstraint(value = "POST", rolesAllowed = "user"),
>>> })
>>>
>>> Example 4: use of the setServletSecurity method of the ServletRegistration
>>> interface with a ServletSecurityElement parameter that either implicitly
>>> or explicitly establishes a default HTTP method-indpendent @HttpConstraint
>>> value (as described in example 3 above). All methods except those that are
>>> the target of an HttpMethodConstraint element are uncovered (at the
>>> url-patterns to which the constraint applies).
>>>
>>> EFFECT OF ANNOTATION TRANSLATION RULES
>>>
>>> Section "13.4.1.2 Mapping @ServletSecurity to security-constraint" defines
>>> the mapping of the @ServletSecurity annotation and the ServletSecurityElement
>>> into corresponding web.xml style security-constraints. As defined in
>>> section 13.4.1.2:
>>>
>>> "The @ServletSecurity annotation is used to define one method-independent
>>> @HttpConstraint followed by a list of zero or more @HttpMethodConstraint
>>> specifications. The method-independent constraint is applied to all HTTP
>>> methods for which no HTTP method-specific constraint has been defined.
>>>
>>> When no @HttpMethodConstraint elements are included, a @ServletSecurity
>>> annotation corresponds to a single security-constraint element containing a
>>> web-resource-collection that contains no http-method elements, and thus
>>> pertains to all HTTP methods."
>>>
>>> The security-constraint elements that result from this mapping combine to
>>> leave no HTTP methods uncovered (at the corresponding url-patterns).
>>> Following the mapping, any HTTP methods that were uncovered prior to
>>> the mapping (as shown in Examples 3 and 4 above) would no longer be
>>> distinguishable as such. In other words, after the mapping to
>>> security-constraints, it will be too late to associate a different
>>> semantic with such methods.
>>>
>>> As such, the deny-uncovered-http-methods flag must effect the mapping
>>> of annotations to security-constraints such that a method-independent
>>> constraint is NOT created when at least one protected HttpMethodConstraint
>>> is defined within the @ServletSecurity annotation (or ServletSecurityElement)
>>> and the value of the @HttpConstraint annotation (or HttpConstraintElement)
>>> is equivalent to the default @HttpConstraint
>>> (i.e., value = EmptyRoleSemantic.PERMIT, rolesAllowed = {}, and
>>> TransportGuarantee = NONE). An HttpMethodConstraint is protected if it's
>>> protection value differs from that of the default @HTTPConstraint.
>>>
>>> In summary, to implement a flag to be used to change the semantic applied
>>> to uncovered HTTP methods, it will be necessary for it to effect the
>>> interpretation of the @ServletSecurity and ServletSecurityElement such that
>>> methods uncovered in these forms will retain their uncovered status into
>>> constraint combination, after which time a deny semantic can be associated
>>> with any remaining uncovered methods.
>>>
>