Mailing-List: contact issues-help@cxf.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@cxf.apache.org
Date: Tue, 8 Sep 2015 16:14:45 +0000 (UTC)
From: "Sergey Beryozkin (JIRA)" <jira@apache.org>
To: issues@cxf.apache.org
Message-ID: <JIRA.12861939.1441397762000.280139.1441728885905@Atlassian.JIRA>
In-Reply-To: <JIRA.12861939.1441397762000@Atlassian.JIRA>
References: <JIRA.12861939.1441397762000@Atlassian.JIRA>
 <JIRA.12861939.1441397762300@arcas>
Subject: [jira] [Comment Edited] (CXF-6579) Inflated tokens can be corrupted
 if compression ratio is greater than 2:1
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit


    [ https://issues.apache.org/jira/browse/CXF-6579?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14735047#comment-14735047 ] 

Sergey Beryozkin edited comment on CXF-6579 at 9/8/15 4:13 PM:
---------------------------------------------------------------

If I replace the code as suggested I have these two DeflateEncoderDecoderTest tests failing:

testInvalidContent
testInvalidContentAfterBase64

Note I added those tests in response to:

https://issues.apache.org/jira/browse/CXF-5390

This all revolves around the existing code that gets the token inflated and therefore one option is just to drop these 2 tests if we replace the code as you suggested.

However, clearly "invalid_grant".getBytes() is not a valid deflated sequence and as such why exactly the code you suggested does not throw DataFormatException ? 

If the existing inflation code does not work for higher compression rates then why can't we fix it as opposed to completely remove it.

I'm sure that directly working with Inflater is also viable in a sense that it should support various compression rates, therefore IMHO we need to tune the existing code to ensure we have all the rates supported but also have an early failure in case of invalid sequences.

Can you please review the existing code and see how it can be fixed ? 
 

was (Author: sergey_beryozkin):
If I replace the code as suggested I have these two DataFormatException tests failing:

testInvalidContent
testInvalidContentAfterBase64

Note I added those tests in response to:

https://issues.apache.org/jira/browse/CXF-5390

This all revolves around the existing code that gets the token inflated and therefore one option is just to drop these 2 tests if we replace the code as you suggested.

However, clearly "invalid_grant".getBytes() is not a valid deflated sequence and as such why exactly the code you suggested does not throw DataFormatException ? 

If the existing inflation code does not work for higher compression rates then why can't we fix it as opposed to completely remove it.

I'm sure that directly working with Inflater is also viable in a sense that it should support various compression rates, therefore IMHO we need to tune the existing code to ensure we have all the rates supported but also have an early failure in case of invalid sequences.

Can you please review the existing code and see how it can be fixed ? 
 

> Inflated tokens can be corrupted if compression ratio is greater than 2:1
> -------------------------------------------------------------------------
>
>                 Key: CXF-6579
>                 URL: https://issues.apache.org/jira/browse/CXF-6579
>             Project: CXF
>          Issue Type: Bug
>          Components: Core, JAX-RS Security
>    Affects Versions: 3.0.6, 2.7.17, 3.1.2
>            Reporter: Phillip Klinefelter
>            Assignee: Sergey Beryozkin
>            Priority: Critical
>             Fix For: 3.1.3, 2.7.18, 3.0.7
>
>
> DeflateEncoderDecoder/CompressionUtils inflate method assumes that the compression ratio will be 2:1.  That assumption is not true for SAML tokens with many similar attribute statements.  The inflated token will be corrupted with a portion of the token replaced with null characters.
> https://github.com/apache/cxf/blob/cxf-2.7.17/rt/rs/security/xml/src/main/java/org/apache/cxf/rs/security/saml/DeflateEncoderDecoder.java#L34
> https://github.com/apache/cxf/blob/cxf-3.0.6/core/src/main/java/org/apache/cxf/common/util/CompressionUtils.java#L41
> https://github.com/apache/cxf/blob/cxf-3.1.2/core/src/main/java/org/apache/cxf/common/util/CompressionUtils.java#L41
> {code}
>     @Test
>     public void testInflateDeflateWithTokenDuplication() throws Exception {
>         String token = "valid_grant valid_grant valid_grant valid_grant valid_grant valid_grant";
>         DeflateEncoderDecoder deflateEncoderDecoder = new DeflateEncoderDecoder();
>         byte[] deflatedToken = deflateEncoderDecoder.deflateToken(token.getBytes());
>         String cxfInflatedToken = IOUtils
>                 .toString(deflateEncoderDecoder.inflateToken(deflatedToken));
>         String streamInflatedToken = IOUtils.toString(
>                 new InflaterInputStream(new ByteArrayInputStream(deflatedToken),
>                         new Inflater(true)));
>         assertThat(streamInflatedToken, is(token));
>         assertThat(cxfInflatedToken, is(token));
>     }
> {code}
> The stream inflated token is correct but the CXF inflated token is invalid.
> {code}
> java.lang.AssertionError: 
> Expected: is "valid_grant valid_grant valid_grant valid_grant valid_grant valid_grant"
>      got: "t valid_grant valid_grant valid_grant"
> {code}


--
This message was sent by Atlassian JIRA
(v6.3.4#6332)