Return-Path: X-Original-To: apmail-cxf-commits-archive@www.apache.org Delivered-To: apmail-cxf-commits-archive@www.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id BFD83179B5 for ; Fri, 26 Sep 2014 16:05:19 +0000 (UTC) Received: (qmail 80385 invoked by uid 500); 26 Sep 2014 16:05:19 -0000 Delivered-To: apmail-cxf-commits-archive@cxf.apache.org Received: (qmail 80321 invoked by uid 500); 26 Sep 2014 16:05:19 -0000 Mailing-List: contact commits-help@cxf.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@cxf.apache.org Delivered-To: mailing list commits@cxf.apache.org Received: (qmail 80310 invoked by uid 99); 26 Sep 2014 16:05:19 -0000 Received: from tyr.zones.apache.org (HELO tyr.zones.apache.org) (140.211.11.114) by apache.org (qpsmtpd/0.29) with ESMTP; Fri, 26 Sep 2014 16:05:19 +0000 Received: by tyr.zones.apache.org (Postfix, from userid 65534) id 23C979A7B15; Fri, 26 Sep 2014 16:05:19 +0000 (UTC) Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: sergeyb@apache.org To: commits@cxf.apache.org Message-Id: <581d203c5206442682217df5a4743bbf@git.apache.org> X-Mailer: ASF-Git Admin Mailer Subject: git commit: [CXF-5944] Adding a JWS headers test on behalf of Luigi Lo Iacono with some modifications Date: Fri, 26 Sep 2014 16:05:19 +0000 (UTC) Repository: cxf Updated Branches: refs/heads/master 971d2199b -> b147a3d25 [CXF-5944] Adding a JWS headers test on behalf of Luigi Lo Iacono with some modifications Project: http://git-wip-us.apache.org/repos/asf/cxf/repo Commit: http://git-wip-us.apache.org/repos/asf/cxf/commit/b147a3d2 Tree: http://git-wip-us.apache.org/repos/asf/cxf/tree/b147a3d2 Diff: http://git-wip-us.apache.org/repos/asf/cxf/diff/b147a3d2 Branch: refs/heads/master Commit: b147a3d25dbb24a395faace4b3e23210810ea591 Parents: 971d219 Author: Sergey Beryozkin Authored: Fri Sep 26 17:05:03 2014 +0100 Committer: Sergey Beryozkin Committed: Fri Sep 26 17:05:03 2014 +0100 ---------------------------------------------------------------------- .../jose/AbstractJoseObjectReaderWriter.java | 42 ++++-- .../cxf/rs/security/jose/JoseHeaders.java | 31 +++- .../security/jose/jwe/JweCompactConsumer.java | 8 +- .../security/jose/jws/JwsCompactConsumer.java | 17 ++- .../security/jose/jws/JwsCompactHeaderTest.java | 147 +++++++++++++++++++ 5 files changed, 220 insertions(+), 25 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/cxf/blob/b147a3d2/rt/rs/security/oauth-parent/oauth2-jose/src/main/java/org/apache/cxf/rs/security/jose/AbstractJoseObjectReaderWriter.java ---------------------------------------------------------------------- diff --git a/rt/rs/security/oauth-parent/oauth2-jose/src/main/java/org/apache/cxf/rs/security/jose/AbstractJoseObjectReaderWriter.java b/rt/rs/security/oauth-parent/oauth2-jose/src/main/java/org/apache/cxf/rs/security/jose/AbstractJoseObjectReaderWriter.java index 3e43a94..d1cbcb7 100644 --- a/rt/rs/security/oauth-parent/oauth2-jose/src/main/java/org/apache/cxf/rs/security/jose/AbstractJoseObjectReaderWriter.java +++ b/rt/rs/security/oauth-parent/oauth2-jose/src/main/java/org/apache/cxf/rs/security/jose/AbstractJoseObjectReaderWriter.java @@ -104,18 +104,13 @@ public class AbstractJoseObjectReaderWriter { protected void fromJsonInternal(AbstractJoseObject jwt, String json) { String theJson = json.trim(); - Map values = readJwtObjectAsMap(theJson.substring(1, theJson.length() - 1)); - fromJsonInternal(jwt, values); + JoseObjectSettable joseObject = new JoseObjectSettable(jwt); + readJwtObjectAsMap(joseObject, theJson.substring(1, theJson.length() - 1)); } - protected void fromJsonInternal(AbstractJoseObject jwt, Map values) { - for (Map.Entry entry : values.entrySet()) { - jwt.setValue(entry.getKey(), entry.getValue()); - } - } - protected Map readJwtObjectAsMap(String json) { - Map values = new LinkedHashMap(); + + protected void readJwtObjectAsMap(Settable values, String json) { for (int i = 0; i < json.length(); i++) { if (isWhiteSpace(json.charAt(i))) { continue; @@ -133,7 +128,9 @@ public class AbstractJoseObjectReaderWriter { if (json.charAt(sepIndex + j) == '{') { int closingIndex = getClosingIndex(json, '{', '}', sepIndex + j); String newJson = json.substring(sepIndex + j + 1, closingIndex); - values.put(name, readJwtObjectAsMap(newJson)); + MapSettable nextMap = new MapSettable(); + readJwtObjectAsMap(nextMap, newJson); + values.put(name, nextMap.map); i = closingIndex + 1; } else if (json.charAt(sepIndex + j) == '[') { int closingIndex = getClosingIndex(json, '[', ']', sepIndex + j); @@ -151,7 +148,6 @@ public class AbstractJoseObjectReaderWriter { } } - return values; } protected List readJwtObjectAsList(String json) { List values = new LinkedList(); @@ -161,7 +157,9 @@ public class AbstractJoseObjectReaderWriter { } if (json.charAt(i) == '{') { int closingIndex = getClosingIndex(json, '{', '}', i); - values.add(readJwtObjectAsMap(json.substring(i + 1, closingIndex))); + MapSettable nextMap = new MapSettable(); + readJwtObjectAsMap(nextMap, json.substring(i + 1, closingIndex)); + values.add(nextMap.map); i = closingIndex + 1; } else { int commaIndex = getCommaIndex(json, i); @@ -208,7 +206,25 @@ public class AbstractJoseObjectReaderWriter { this.format = format; } - + private interface Settable { + void put(String key, Object value); + } + private static class MapSettable implements Settable { + private Map map = new LinkedHashMap(); + public void put(String key, Object value) { + map.put(key, value); + } + + } + private static class JoseObjectSettable implements Settable { + private AbstractJoseObject jose; + public JoseObjectSettable(AbstractJoseObject jose) { + this.jose = jose; + } + public void put(String key, Object value) { + jose.setValue(key, value); + } + } } http://git-wip-us.apache.org/repos/asf/cxf/blob/b147a3d2/rt/rs/security/oauth-parent/oauth2-jose/src/main/java/org/apache/cxf/rs/security/jose/JoseHeaders.java ---------------------------------------------------------------------- diff --git a/rt/rs/security/oauth-parent/oauth2-jose/src/main/java/org/apache/cxf/rs/security/jose/JoseHeaders.java b/rt/rs/security/oauth-parent/oauth2-jose/src/main/java/org/apache/cxf/rs/security/jose/JoseHeaders.java index a503665..3577669 100644 --- a/rt/rs/security/oauth-parent/oauth2-jose/src/main/java/org/apache/cxf/rs/security/jose/JoseHeaders.java +++ b/rt/rs/security/oauth-parent/oauth2-jose/src/main/java/org/apache/cxf/rs/security/jose/JoseHeaders.java @@ -19,6 +19,8 @@ package org.apache.cxf.rs.security.jose; +import java.util.Collections; +import java.util.LinkedHashMap; import java.util.List; import java.util.Map; @@ -26,7 +28,7 @@ import org.apache.cxf.helpers.CastUtils; import org.apache.cxf.rs.security.jose.jwk.JsonWebKey; public class JoseHeaders extends AbstractJoseObject { - + private Map headerUpdateCount; public JoseHeaders() { } @@ -91,7 +93,7 @@ public class JoseHeaders extends AbstractJoseObject { } public void setX509ThumbprintSHA256(String x509Thumbprint) { - super.setValue(JoseConstants.HEADER_X509_THUMBPRINT_SHA256, x509Thumbprint); + setHeader(JoseConstants.HEADER_X509_THUMBPRINT_SHA256, x509Thumbprint); } public String getX509ThumbprintSHA256() { @@ -107,7 +109,7 @@ public class JoseHeaders extends AbstractJoseObject { } public void setJsonWebKey(JsonWebKey key) { - setValue(JoseConstants.HEADER_JSON_WEB_KEY, key); + setHeader(JoseConstants.HEADER_JSON_WEB_KEY, key); } public JsonWebKey getJsonWebKey() { @@ -124,17 +126,29 @@ public class JoseHeaders extends AbstractJoseObject { return this; } + protected void setValue(String name, Object value) { + if (super.values.containsKey(name)) { + if (headerUpdateCount == null) { + headerUpdateCount = new LinkedHashMap(); + } + Integer count = headerUpdateCount.get(name); + count = count == null ? 2 : count++; + headerUpdateCount.put(name, count); + } + super.setValue(name, value); + } + public Object getHeader(String name) { return getValue(name); } public JoseHeaders setIntegerHeader(String name, Integer value) { - setValue(name, value); + setHeader(name, value); return this; } public Integer getIntegerHeader(String name) { - Object value = getValue(name); + Object value = getHeader(name); if (value != null) { return value instanceof Integer ? (Integer)value : Integer.parseInt(value.toString()); } else { @@ -142,16 +156,19 @@ public class JoseHeaders extends AbstractJoseObject { } } public JoseHeaders setLongHeader(String name, Long value) { - setValue(name, value); + setHeader(name, value); return this; } public Long getLongHeader(String name) { - Object value = getValue(name); + Object value = getHeader(name); if (value != null) { return value instanceof Long ? (Long)value : Long.parseLong(value.toString()); } else { return null; } } + public Map getHeaderUpdateCount() { + return headerUpdateCount == null ? null : Collections.unmodifiableMap(headerUpdateCount); + } } http://git-wip-us.apache.org/repos/asf/cxf/blob/b147a3d2/rt/rs/security/oauth-parent/oauth2-jose/src/main/java/org/apache/cxf/rs/security/jose/jwe/JweCompactConsumer.java ---------------------------------------------------------------------- diff --git a/rt/rs/security/oauth-parent/oauth2-jose/src/main/java/org/apache/cxf/rs/security/jose/jwe/JweCompactConsumer.java b/rt/rs/security/oauth-parent/oauth2-jose/src/main/java/org/apache/cxf/rs/security/jose/jwe/JweCompactConsumer.java index 8fc9805..7794102 100644 --- a/rt/rs/security/oauth-parent/oauth2-jose/src/main/java/org/apache/cxf/rs/security/jose/jwe/JweCompactConsumer.java +++ b/rt/rs/security/oauth-parent/oauth2-jose/src/main/java/org/apache/cxf/rs/security/jose/jwe/JweCompactConsumer.java @@ -23,6 +23,7 @@ import java.io.UnsupportedEncodingException; import org.apache.cxf.common.util.Base64Exception; import org.apache.cxf.common.util.Base64UrlUtility; +import org.apache.cxf.rs.security.jose.JoseHeaders; import org.apache.cxf.rs.security.jose.JoseHeadersReader; import org.apache.cxf.rs.security.jose.JoseHeadersReaderWriter; @@ -49,7 +50,12 @@ public class JweCompactConsumer { encryptedContent = Base64UrlUtility.decode(parts[3]); authTag = Base64UrlUtility.decode(parts[4]); - jweHeaders = new JweHeaders(reader.fromJsonHeaders(headersJson)); + JoseHeaders joseHeaders = reader.fromJsonHeaders(headersJson); + if (joseHeaders.getHeaderUpdateCount() != null) { + throw new SecurityException(); + } + jweHeaders = new JweHeaders(joseHeaders); + } catch (Base64Exception ex) { throw new SecurityException(ex); } http://git-wip-us.apache.org/repos/asf/cxf/blob/b147a3d2/rt/rs/security/oauth-parent/oauth2-jose/src/main/java/org/apache/cxf/rs/security/jose/jws/JwsCompactConsumer.java ---------------------------------------------------------------------- diff --git a/rt/rs/security/oauth-parent/oauth2-jose/src/main/java/org/apache/cxf/rs/security/jose/jws/JwsCompactConsumer.java b/rt/rs/security/oauth-parent/oauth2-jose/src/main/java/org/apache/cxf/rs/security/jose/jws/JwsCompactConsumer.java index 3449b8b..b3d0cbb 100644 --- a/rt/rs/security/oauth-parent/oauth2-jose/src/main/java/org/apache/cxf/rs/security/jose/jws/JwsCompactConsumer.java +++ b/rt/rs/security/oauth-parent/oauth2-jose/src/main/java/org/apache/cxf/rs/security/jose/jws/JwsCompactConsumer.java @@ -22,6 +22,7 @@ import java.io.UnsupportedEncodingException; import org.apache.cxf.common.util.Base64Exception; import org.apache.cxf.common.util.Base64UrlUtility; +import org.apache.cxf.rs.security.jose.JoseHeaders; import org.apache.cxf.rs.security.jose.JoseHeadersReader; import org.apache.cxf.rs.security.jose.JoseHeadersReaderWriter; import org.apache.cxf.rs.security.jose.jwk.JsonWebKey; @@ -78,13 +79,21 @@ public class JwsCompactConsumer { return encodedSignature.isEmpty() ? new byte[]{} : decode(encodedSignature); } public JwsHeaders getJwsHeaders() { - return new JwsHeaders(getReader().fromJsonHeaders(headersJson)); + JoseHeaders joseHeaders = reader.fromJsonHeaders(headersJson); + if (joseHeaders.getHeaderUpdateCount() != null) { + throw new SecurityException(); + } + return new JwsHeaders(joseHeaders); } public boolean verifySignatureWith(JwsSignatureVerifier validator) { - if (!validator.verify(getJwsHeaders(), getUnsignedEncodedPayload(), getDecodedSignature())) { - throw new SecurityException(); + try { + if (validator.verify(getJwsHeaders(), getUnsignedEncodedPayload(), getDecodedSignature())) { + return true; + } + } catch (SecurityException ex) { + // ignore } - return true; + return false; } public boolean verifySignatureWith(JsonWebKey key) { return verifySignatureWith(JwsUtils.getSignatureVerifier(key)); http://git-wip-us.apache.org/repos/asf/cxf/blob/b147a3d2/rt/rs/security/oauth-parent/oauth2-jose/src/test/java/org/apache/cxf/rs/security/jose/jws/JwsCompactHeaderTest.java ---------------------------------------------------------------------- diff --git a/rt/rs/security/oauth-parent/oauth2-jose/src/test/java/org/apache/cxf/rs/security/jose/jws/JwsCompactHeaderTest.java b/rt/rs/security/oauth-parent/oauth2-jose/src/test/java/org/apache/cxf/rs/security/jose/jws/JwsCompactHeaderTest.java new file mode 100644 index 0000000..0cc0a07 --- /dev/null +++ b/rt/rs/security/oauth-parent/oauth2-jose/src/test/java/org/apache/cxf/rs/security/jose/jws/JwsCompactHeaderTest.java @@ -0,0 +1,147 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.cxf.rs.security.jose.jws; + +import org.junit.Assert; +import org.junit.Test; + +public class JwsCompactHeaderTest extends Assert { + + /** + * JWS string, which lacks the "alg" header field. + * + * => Must be rejected by verification operation, since the spec declares + * that the "alg" header field must be present in the compact serialization. + */ + public static final String MISSING_ALG_HEADER_FIELD_IN_JWS = + "eyAiZ2xhIiA6ICJDQU1IIiB9.eyAibXNnIjogIllvdSBjYW4ndCB0b3VjaCB0aGlzISIgfQ" + + ".Sqd_AuwlPPqv4L1EV4zPuR-HfFJpe9kOfvc597RlcoE"; + + /** + * JWS string, which contains two "alg" header fields. Bogus "alg" header + * field first. + * + * => Must be rejected by verification operation, since the spec declares + * that the "alg" header field must be present once in the compact + * serialization. + */ + public static final String TWO_ALG_HEADER_FIELDS_IN_JWS_BOGUS_FIRST = + "eyAiYWxnIjogIkJvZ3VzIiwgImFsZyI6ICJIUzI1NiIgfQ.eyAibXNnIjogIllvdSBjYW4ndCB0b3VjaCB0aGlzISIgfQ" + + ".FIgpDi1Wp9iIxxXfBw8Zce2kiZ8gmqAaVYPduRFR8kU"; + + /** + * JWS string, which contains two "alg" header fields. Bogus "alg" header + * field last. + * + * => Must be rejected by verification operation, since the spec declares + * that the "alg" header field must be present once in the compact + * serialization. + */ + public static final String TWO_ALG_HEADER_FIELDS_IN_JWS_BOGUS_LAST = + "eyAiYWxnIjogIkhTMjU2IiwgImFsZyI6ICJCb2d1cyIgfQ.eyAibXNnIjogIllvdSBjYW4ndCB0b3VjaCB0aGlzISIgfQ" + + ".Ftwla-nAg0Nty8ILVhjlIETOy2Tw1JsD3bBq55AS0PU"; + + /** + * JWS string, which contains an invalid "alg" header field value. + * + * (1): Algorithm not supported/known + * + * => Must be rejected by verification operation, since the spec declares + * that the signature is not valid if the "alg" value does not represent a + * supported algorithm. "alg" values should either be registered in the IANA + * JSON Web Signature and Encryption Algorithms registry defined in JWA or + * be a value that contains a Collision-Resistant Name. + */ + public static final String INVALID_ALG_HEADER_VALUE_IN_JWS_1 = "tba"; + + /** + * JWS string, which contains an invalid "alg" header field value. + * + * (2): Wrong value encoding + * + * => Must be rejected by verification operation, since the spec declares + * that the "alg" value is a case-sensitive string containing a StringOrURI + * value. + */ + public static final String INVALID_ALG_HEADER_VALUE_IN_JWS_2 = "tba"; + + /** + * JWS string, which contains a "alg" header field value of "none". The + * signature has been generated with "HS256" and the signed JWS has been + * altered afterwards to the value "none". + * + * => Must be rejected by verification operation, since the "none" algorithm + * is considered harmful. + */ + public static final String ALG_HEADER_VALUE_HS256_IN_JWS = + "eyAiYWxnIjogIkhTMjU2IiB9" + + ".eyAibXNnIjogIllvdSBjYW4ndCB0b3VjaCB0aGlzISIgfQ" + + ".as_gclokwAmukh3zVF1X5sUCCfSc8TbjDdhdvk6C5c8"; + public static final String ALG_HEADER_VALUE_NONE_IN_JWS = + "eyAiYWxnIjogIm5vbmUiIH0" + + ".eyAibXNnIjogIllvdSBjYW4ndCB0b3VjaCB0aGlzISIgfQ" + + ".as_gclokwAmukh3zVF1X5sUCCfSc8TbjDdhdvk6C5c8"; + + + /** + * Support material (keys, etc.) + */ + private static final String ENCODED_MAC_KEY = "AyM1SysPpbyDfgZld3umj1qzKObwVMkoqQ-EstJQLr_T-1qS0gZH75" + + "aKtMN3Yj0iPS4hcgUuTwjAzZr1Z9CAow"; + + // JWS string, which contains crit header field + // JWS string, which contains more than three parts + // JWS string, which contains less than three parts + // JWS string, which contains null bytes padding + + @Test + public void verifyJwsWithMissingAlgHeaderField() throws Exception { + JwsCompactConsumer jwsConsumer = new JwsCompactConsumer(MISSING_ALG_HEADER_FIELD_IN_JWS); + + assertFalse(jwsConsumer.verifySignatureWith(new HmacJwsSignatureVerifier(ENCODED_MAC_KEY))); + } + + @Test + public void verifyJwsWithTwoAlgHeaderFieldsBogusFieldFirst() throws Exception { + JwsCompactConsumer jwsConsumer = new JwsCompactConsumer(TWO_ALG_HEADER_FIELDS_IN_JWS_BOGUS_FIRST); + + assertFalse(jwsConsumer.verifySignatureWith(new HmacJwsSignatureVerifier(ENCODED_MAC_KEY))); + } + + @Test + public void verifyJwsWithTwoAlgHeaderFieldsBogusFieldLast() throws Exception { + JwsCompactConsumer jwsConsumer = new JwsCompactConsumer(TWO_ALG_HEADER_FIELDS_IN_JWS_BOGUS_LAST); + + assertFalse(jwsConsumer.verifySignatureWith(new HmacJwsSignatureVerifier(ENCODED_MAC_KEY))); + } + + @Test + public void verifyJwsWithAlgHeaderValueNone() throws Exception { + JwsCompactConsumer jwsConsumerOriginal = new JwsCompactConsumer(ALG_HEADER_VALUE_HS256_IN_JWS); + + JwsCompactConsumer jwsConsumerAltered = new JwsCompactConsumer(ALG_HEADER_VALUE_NONE_IN_JWS); + + assertTrue(jwsConsumerOriginal.verifySignatureWith(new HmacJwsSignatureVerifier(ENCODED_MAC_KEY))); + + assertFalse(jwsConsumerAltered.verifySignatureWith(new HmacJwsSignatureVerifier(ENCODED_MAC_KEY))); + } + + +} +