From 68ba11ba7b9c7edd63e850e4b5b9c4683d825870 Mon Sep 17 00:00:00 2001 From: Rob Winch Date: Thu, 7 Apr 2011 21:29:28 -0500 Subject: [PATCH] SEC-1711: Support verifying that attribute exchange parameters were signed --- .../security/openid/OpenID4JavaConsumer.java | 28 +++- .../SignedAxMessageExtensionFactory.java | 118 +++++++++++++++ .../openid/OpenID4JavaConsumerTests.java | 22 ++- .../SignedAxMessageExtensionFactoryTests.java | 139 ++++++++++++++++++ 4 files changed, 305 insertions(+), 2 deletions(-) create mode 100644 openid/src/main/java/org/springframework/security/openid/SignedAxMessageExtensionFactory.java create mode 100644 openid/src/test/java/org/springframework/security/openid/SignedAxMessageExtensionFactoryTests.java diff --git a/openid/src/main/java/org/springframework/security/openid/OpenID4JavaConsumer.java b/openid/src/main/java/org/springframework/security/openid/OpenID4JavaConsumer.java index 75542c2955..d278c0f7e9 100644 --- a/openid/src/main/java/org/springframework/security/openid/OpenID4JavaConsumer.java +++ b/openid/src/main/java/org/springframework/security/openid/OpenID4JavaConsumer.java @@ -39,6 +39,7 @@ import org.openid4java.message.ax.AxMessage; import org.openid4java.message.ax.FetchRequest; import org.openid4java.message.ax.FetchResponse; import org.springframework.beans.factory.DisposableBean; +import org.springframework.beans.factory.InitializingBean; import org.springframework.util.StringUtils; @@ -48,7 +49,7 @@ import org.springframework.util.StringUtils; * @author Rob Winch */ @SuppressWarnings("unchecked") -public class OpenID4JavaConsumer implements OpenIDConsumer, DisposableBean { +public class OpenID4JavaConsumer implements OpenIDConsumer, DisposableBean, InitializingBean { private static final String DISCOVERY_INFO_KEY = DiscoveryInformation.class.getName(); private static final String ATTRIBUTE_LIST_KEY = "SPRING_SECURITY_OPEN_ID_ATTRIBUTES_FETCH_LIST"; @@ -59,6 +60,7 @@ public class OpenID4JavaConsumer implements OpenIDConsumer, DisposableBean { private final ConsumerManager consumerManager; private final AxFetchListFactory attributesToFetchFactory; private boolean skipShutdownConnectionManager; + private boolean skipSignedAxMessageRegistration; //~ Constructors =================================================================================================== @@ -242,6 +244,22 @@ public class OpenID4JavaConsumer implements OpenIDConsumer, DisposableBean { this.skipShutdownConnectionManager = skipShutdownConnectionManager; } + /** + * If false {@link SignedAxMessageExtensionFactory} is + * registered with openid4java library in {@link #afterPropertiesSet()}. The + * effect is that all attributes from attribute exchange are guaranteed to + * be signed. If the value is true, then + * {@link SignedAxMessageExtensionFactory} is not registered. The default + * value is false. + * + * @param skipAxMessageRegistration + * the new value to determine if + * {@link SignedAxMessageExtensionFactory} is registered or not. + */ + public void setSkipSignedAxMessageRegistration(boolean skipAxMessageRegistration) { + this.skipSignedAxMessageRegistration = skipAxMessageRegistration; + } + public void destroy() throws Exception { if(!skipShutdownConnectionManager) { MultiThreadedHttpConnectionManager.shutdownAll(); @@ -250,4 +268,12 @@ public class OpenID4JavaConsumer implements OpenIDConsumer, DisposableBean { + "Note this could cause memory leaks if the resources are not cleaned up else where."); } } + + public void afterPropertiesSet() throws Exception { + if(!skipSignedAxMessageRegistration) { + Message.addExtensionFactory(SignedAxMessageExtensionFactory.class); + }else { + logger.debug("Skipping SignedAxMessageExtensionFactory. Attributes from attribute exchange are not guaranteed to be signed."); + } + } } diff --git a/openid/src/main/java/org/springframework/security/openid/SignedAxMessageExtensionFactory.java b/openid/src/main/java/org/springframework/security/openid/SignedAxMessageExtensionFactory.java new file mode 100644 index 0000000000..7bfdb3ace1 --- /dev/null +++ b/openid/src/main/java/org/springframework/security/openid/SignedAxMessageExtensionFactory.java @@ -0,0 +1,118 @@ +/* Copyright 2011 to the original author or authors. + * + * Licensed 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.springframework.security.openid; + +import org.openid4java.message.MessageException; +import org.openid4java.message.MessageExtension; +import org.openid4java.message.MessageExtensionFactory; +import org.openid4java.message.ParameterList; +import org.openid4java.message.ax.AxMessage; +import org.openid4java.message.ax.FetchRequest; +import org.openid4java.message.ax.FetchResponse; +import org.openid4java.message.ax.StoreRequest; +import org.openid4java.message.ax.StoreResponse; + +/** + *

+ * An extension to the openid4java library which when registered with the Message class will guarantee that attribute + * exchange attributes are signed. + *

+ *

+ * WARNING The scope of this class must be public because it is required in order for + * it to be registered as MessageExtensionFactory. However, this class is meant for internal use only. + *

+ * + * @since 3.1 + * @author Rob Winch + */ +public final class SignedAxMessageExtensionFactory implements MessageExtensionFactory { + + //~ Methods ======================================================================================================= + + public String getTypeUri() { + return AxMessage.OPENID_NS_AX; + } + + public MessageExtension getExtension(ParameterList parameterList, + boolean isRequest) throws MessageException { + String axMode = parameterList.getParameterValue("mode"); + + if ("fetch_request".equals(axMode)) { + return new SignedFetchRequest(parameterList); + } else if ("fetch_response".equals(axMode)) { + return new SignedFetchResponse(parameterList); + } else if ("store_request".equals(axMode)) { + return new SignedStoreRequest(parameterList); + } else if ("store_response_success".equals(axMode) || + "store_response_failure".equals(axMode)) { + return new SignedStoreResponse(parameterList); + } + throw new MessageException("Invalid value for attribute exchange mode: " + + axMode); + } + + //~ Inner Classes ================================================================================================= + + private static class SignedFetchRequest extends FetchRequest { + public SignedFetchRequest(ParameterList params) throws MessageException { + super(params); + if(!isValid()) { + throw new MessageException("Invalid parameters for a fetch request"); + } + } + @Override + public boolean signRequired() { + return true; + } + } + + private static class SignedFetchResponse extends FetchResponse { + public SignedFetchResponse(ParameterList params) throws MessageException { + super(params); + if(!isValid()) { + throw new MessageException("Invalid parameters for a fetch response"); + } + } + @Override + public boolean signRequired() { + return true; + } + } + + private static class SignedStoreRequest extends StoreRequest { + public SignedStoreRequest(ParameterList params) throws MessageException { + super(params); + if(!isValid()) { + throw new MessageException("Invalid parameters for a store request"); + } + } + @Override + public boolean signRequired() { + return true; + } + } + + private static class SignedStoreResponse extends StoreResponse { + public SignedStoreResponse(ParameterList params) throws MessageException { + super(params); + // validate the params + StoreResponse.createStoreResponse(params); + } + @Override + public boolean signRequired() { + return true; + } + } +} diff --git a/openid/src/test/java/org/springframework/security/openid/OpenID4JavaConsumerTests.java b/openid/src/test/java/org/springframework/security/openid/OpenID4JavaConsumerTests.java index 2db95fb7dc..83534f987b 100644 --- a/openid/src/test/java/org/springframework/security/openid/OpenID4JavaConsumerTests.java +++ b/openid/src/test/java/org/springframework/security/openid/OpenID4JavaConsumerTests.java @@ -39,7 +39,7 @@ import org.springframework.mock.web.MockHttpServletRequest; * @author Rob Winch */ @RunWith(PowerMockRunner.class) -@PrepareForTest(MultiThreadedHttpConnectionManager.class) +@PrepareForTest({MultiThreadedHttpConnectionManager.class,Message.class}) public class OpenID4JavaConsumerTests { List attributes = Arrays.asList(new OpenIDAttribute("a","b"), new OpenIDAttribute("b","b", Arrays.asList("c"))); @@ -208,6 +208,26 @@ public class OpenID4JavaConsumerTests { new OpenID4JavaConsumer(attributes); } + @Test + public void afterPropertiesSetRegister() throws Exception { + mockStatic(Message.class); + new OpenID4JavaConsumer().afterPropertiesSet(); + + verifyStatic(); + Message.addExtensionFactory(SignedAxMessageExtensionFactory.class); + } + + @Test + public void afterPropertiesSetSkipRegister() throws Exception { + mockStatic(Message.class); + OpenID4JavaConsumer consumer = new OpenID4JavaConsumer(); + consumer.setSkipSignedAxMessageRegistration(true); + consumer.afterPropertiesSet(); + + verifyStatic(never()); + Message.addExtensionFactory(SignedAxMessageExtensionFactory.class); + } + @Test public void destroyInvokesShutdownAll() throws Exception { mockStatic(MultiThreadedHttpConnectionManager.class); diff --git a/openid/src/test/java/org/springframework/security/openid/SignedAxMessageExtensionFactoryTests.java b/openid/src/test/java/org/springframework/security/openid/SignedAxMessageExtensionFactoryTests.java new file mode 100644 index 0000000000..bd5077d4b3 --- /dev/null +++ b/openid/src/test/java/org/springframework/security/openid/SignedAxMessageExtensionFactoryTests.java @@ -0,0 +1,139 @@ +/* Copyright 2011 to the original author or authors. + * + * Licensed 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.springframework.security.openid; + +import static junit.framework.Assert.assertEquals; +import static junit.framework.Assert.assertTrue; + +import java.util.HashMap; +import java.util.Map; + +import org.junit.Before; +import org.junit.Test; +import org.openid4java.message.MessageException; +import org.openid4java.message.MessageExtension; +import org.openid4java.message.ParameterList; +import org.openid4java.message.ax.AxMessage; + +public class SignedAxMessageExtensionFactoryTests { + private Map params; + private SignedAxMessageExtensionFactory factory; + + @Before + public void setUp() { + factory = new SignedAxMessageExtensionFactory(); + params = new HashMap(); + } + + @Test + public void getTypeUri() { + assertEquals(AxMessage.OPENID_NS_AX, factory.getTypeUri()); + } + + @Test + public void fetchRequestSigned() throws Exception { + params.put("mode", "fetch_request"); + params.put("value.email","email@example.com"); + params.put("type.email","http://axschema.org/contact/email"); + params.put("required", "email"); + MessageExtension ext = factory.getExtension(new ParameterList(params), true); + assertTrue(ext.signRequired()); + } + + @Test(expected=MessageException.class) + public void fetchRequestInvalid() throws Exception { + params.put("mode", "fetch_request"); + params.put("type.email","http://axschema.org/contact/email"); + MessageExtension ext = factory.getExtension(new ParameterList(params), true); + assertTrue(ext.signRequired()); + } + + @Test + public void fetchResponseSigned() throws Exception { + params.put("mode", "fetch_response"); + params.put("value.email","email@example.com"); + params.put("type.email","http://axschema.org/contact/email"); + params.put("required", "email"); + MessageExtension ext = factory.getExtension(new ParameterList(params), false); + assertTrue(ext.signRequired()); + } + + @Test(expected=MessageException.class) + public void fetchResponseInvalid() throws Exception { + params.put("mode", "fetch_response"); + params.put("type.email","http://axschema.org/contact/email"); + MessageExtension ext = factory.getExtension(new ParameterList(params), true); + assertTrue(ext.signRequired()); + } + + @Test + public void storeRequestSigned() throws Exception { + params.put("mode", "store_request"); + params.put("value.email","email@example.com"); + params.put("type.email","http://axschema.org/contact/email"); + params.put("required", "email"); + MessageExtension ext = factory.getExtension(new ParameterList(params), true); + assertTrue(ext.signRequired()); + } + + @Test(expected=MessageException.class) + public void storeRequestInvalid() throws Exception { + params.put("mode", "store_request"); + params.put("type.email","http://axschema.org/contact/email"); + MessageExtension ext = factory.getExtension(new ParameterList(params), true); + assertTrue(ext.signRequired()); + } + + @Test + public void storeResponseSuccessSigned() throws Exception { + params.put("mode", "store_response_success"); + MessageExtension ext = factory.getExtension(new ParameterList(params), false); + assertTrue(ext.signRequired()); + } + + @Test(expected=MessageException.class) + public void storeResponseSuccessInvalid() throws Exception { + params.put("mode", "store_response_success"); + params.put("invalid","value"); + MessageExtension ext = factory.getExtension(new ParameterList(params), true); + assertTrue(ext.signRequired()); + } + + @Test + public void storeResponseFailureSigned() throws Exception { + params.put("mode", "store_response_failure"); + MessageExtension ext = factory.getExtension(new ParameterList(params), false); + assertTrue(ext.signRequired()); + } + + @Test(expected=MessageException.class) + public void storeResponseFailureInvalid() throws Exception { + params.put("mode", "store_response_failure"); + params.put("value.email","email@example.com"); + MessageExtension ext = factory.getExtension(new ParameterList(params), true); + assertTrue(ext.signRequired()); + } + + @Test(expected=MessageException.class) + public void nullMode() throws Exception { + factory.getExtension(new ParameterList(params), true); + } + + @Test(expected=MessageException.class) + public void invalidMode() throws Exception { + params.put("mode", "invalid"); + factory.getExtension(new ParameterList(params), true); + } +}