users@jersey.java.net

Re: [Jersey] SAX Feature error in Jersey 1.1.4.1

From: Phil Griffin <phil.griffin_at_oracle.com>
Date: Fri, 05 Mar 2010 08:36:03 -0700

Hi Paul,
Just to close out this thread from my perspective... I'm in general
agreement with you - the framework should default to secure settings but
allow overrides by users to disable those features. Also, as mentioned
in other threads Jersey should document these required/expected parser
features.

This Jersey behavior actually allowed me recognize and point out some
changes my parser factory needed.
Thanks for all the input,
Phil

On 3/1/2010 5:11 AM, Paul Sandoz wrote:
> Hi Phil,
>
> I really do want to avoid the case where developers deploy
> JAX-RS/Jersey apps that are by default insecure. Often developers are
> unaware or not knowledgeable about such vulnerabilities. So by default
> the framework should be as secure as possible.
>
> So logging a severe error IMHO is not suitable. The exception forces
> the developer or the integrator to focus on resolving security issues
> or the developer actively disabling it, thus explicitly taking
> responsibility for a potential vulnerability.
>
> The feature ".../secure-processing" is more vague in what it actually
> specifies:
>
> http://java.sun.com/j2se/1.5.0/docs/api/javax/xml/XMLConstants.html#FEATURE_SECURE_PROCESSING
>
> and mentions cases around DoS attacks based on limits which is less
> severe than that for external entities, which is why in this case i
> log a severe error rather than throwing an exception.
>
> If i would change this code i would take the conservative route of
> throwing an Exception for the second case as well, rather the logging
> a sever error for the first case :-)
>
> Like Tatu i am surprised that the XML parser does not recognize both
> properties.
>
> Paul.
>
> On Feb 20, 2010, at 5:31 PM, Phil Griffin wrote:
>
>> After a bit more debugging, this bit of Jersey code seems to be
>> causing my pain...more specifically, pain in the SAXParserFactory I'm
>> using.
>>
>> I'm not a SAX feature expert, but this snippet from
>> com.sun.jersey.core.impl.provider.xml.SAXParserContextProvider
>> doesn't make sense to me?
>> Since disableXmlSecurity is false by default, the effect of this code
>> is to disable "http://xml.org/sax/features/external-general-entities"
>> processing in my factory, resulting in the exception I originally
>> reported in this thread (which occurs during Jersey-driven
>> processing/parsing of a POST request).
>>
>> Also, my factory doesn't support feature
>> "http://javax.xml.XMLConstants/feature/secure-processing", but at
>> least that just gets logged.
>>
>> Paul/Tatu - can you comment on this?
>> Thanks,
>> Phil
>>
>> disableXmlSecurity =
>> fps.getFeature("com.sun.jersey.config.feature.DisableXmlSecurity");
>> protected SAXParserFactory getInstance()
>> {
>> SAXParserFactory f = SAXParserFactory.newInstance();
>> f.setNamespaceAware(true);
>> if(!disableXmlSecurity)
>> {
>> try
>> {
>>
>> f.setFeature("http://xml.org/sax/features/external-general-entities",
>> Boolean.FALSE.booleanValue());
>> }
>> catch(Exception ex)
>> {
>> throw new RuntimeException("Security features for the
>> SAX parser could not be enabled", ex);
>> }
>> try
>> {
>>
>> f.setFeature("http://javax.xml.XMLConstants/feature/secure-processing",
>> Boolean.TRUE.booleanValue());
>> }
>> catch(Exception ex)
>> {
>> LOGGER.log(Level.WARNING, "JAXP feature
>> XMLConstants.FEATURE_SECURE_PROCESSING cannot be set on a
>> SAXParserFactory. External general entity processing is disbaled but
>> other potential securty related features will not be enabled.", ex);
>> }
>> }
>> return f;
>> }
>>
>>
>> On 2/17/2010 3:02 PM, Phil Griffin wrote:
>>>
>>>
>>> On 2/17/2010 1:28 AM, Paul Sandoz wrote:
>>>>
>>>> On Feb 17, 2010, at 1:06 AM, Phil Griffin wrote:
>>>>
>>>>> Hi Paul,
>>>>> Thanks for the reply. It's a little hard to confirm what version
>>>>> the SAX parser is...looks like it could be Xerces 2.8.1?
>>>>> Is it likely the change in behavior occurred between Jersey 1.0.2
>>>>> and 1.1.4.1?
>>>>
>>>> Yes, i added support for setting the security settings on the JAXP
>>>> parsers in Jersey 1.0.3.1 and 1.1.4.
>>>>
>>>> Actually i went back and looked at the code and you can disable
>>>> this, see:
>>>>
>>>> https://jersey.dev.java.net/nonav/apidocs/latest/jersey/com/sun/jersey/core/util/FeaturesAndProperties.html#FEATURE_DISABLE_XML_SECURITY
>>> Thanks - yes adding this to web.xml is a workaround (while I'm
>>> confirming if the bundled Xerces I'm required to use can be updated?)
>>> <param-name>com.sun.jersey.config.feature.DisableXmlSecurity</param-name>
>>>
>>> <param-value>true</param-value>
>>>>
>>>>
>>>>> If so, what version of Xerces would be compatible?
>>>>>
>>>>
>>>> Not sure :-( but Tatu provides some more details in his email.
>>>>
>>>> Paul.
>>>>
>>>>> -Phil
>>>>>
>>>>> On 2/16/2010 2:15 PM, Paul Sandoz wrote:
>>>>>> Hi Phil,
>>>>>>
>>>>>> What is the implementation and version of the SAX parser you are
>>>>>> using?
>>>>>>
>>>>>> This warning is important because Jersey cannot configure the
>>>>>> parsing to protect against certain XML-based denial of service
>>>>>> attacks. So if you are building public-facing services that
>>>>>> consume XML your application could be at risk.
>>>>>>
>>>>>> Currently the only way to disable this is to disable JDK logging.
>>>>>>
>>>>>> If you really need this disabled can you log a enhancement and we
>>>>>> can had a feature to disable security-based configuration?
>>>>>>
>>>>>> Paul.
>>>>>>
>>>>>> On Feb 16, 2010, at 6:54 PM, Phil Griffin wrote:
>>>>>>
>>>>>>> I recently updated our Jersey jars to 1.1.4.1 and began getting
>>>>>>> a JAXP parser registry exception for a non-supported feature (in
>>>>>>> the factory I'm required to use). Is there a way to disable the
>>>>>>> com.sun.jersey.core.provider.jaxb.AbstractJAXBProvider or Jersey
>>>>>>> from expecting this feature?
>>>>>>>
>>>>>>> WebLogicSAXParser cannot be created.SAX feature
>>>>>>> /@ &#39;http://xml.org/sax/features/external-general-entities'
>>>>>>> <http://xml.org/sax/features/external-general-entities%27> not
>>>>>>> supported
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Phil
>>>>>>> /
>>>>>>
>>>>
>