[java-oidc-common] 01/01: JCOMOIDC-90 - Add CSP protection to views
Phil Smart
philip.smart at jisc.ac.uk
Thu Nov 2 09:38:34 UTC 2023
This is an automated email from the git hooks/post-receive script.
philsmart pushed a commit to branch dev/JCOMOIDC-90
in repository java-oidc-common.
View the commit online:
http://git.shibboleth.net/view/?p=java-oidc-common.git;a=commit;h=14e0c1a1754c42df7a071e655600f88930fb4569
commit 14e0c1a1754c42df7a071e655600f88930fb4569
Author: Phil Smart <philip.smart at jisc.ac.uk>
AuthorDate: Thu Nov 2 09:38:26 2023 +0000
JCOMOIDC-90 - Add CSP protection to views
- Added the cspDigester and cspNonceGenerator to the post encoder
- Added the digester and generator to the velocity context if available
- Updated tests
https://shibboleth.atlassian.net/browse/JCOMOIDC-90
---
.../encoding/impl/AbstractOIDCMessageEncoder.java | 2 +-
.../encoding/impl/HTTPPostAuthnRequestEncoder.java | 75 ++++++++++++++++++----
.../impl/HTTPRedirectAuthnRequestEncoder.java | 3 +-
.../impl/HTTPPostAuthnRequestEncoderTest.java | 33 +++++++++-
.../resources/templates/oidc-request-form-post.vm | 14 +++-
5 files changed, 108 insertions(+), 19 deletions(-)
diff --git a/oidc-common-profile-impl/src/main/java/net/shibboleth/oidc/profile/encoding/impl/AbstractOIDCMessageEncoder.java b/oidc-common-profile-impl/src/main/java/net/shibboleth/oidc/profile/encoding/impl/AbstractOIDCMessageEncoder.java
index 2e48004..c46faa3 100644
--- a/oidc-common-profile-impl/src/main/java/net/shibboleth/oidc/profile/encoding/impl/AbstractOIDCMessageEncoder.java
+++ b/oidc-common-profile-impl/src/main/java/net/shibboleth/oidc/profile/encoding/impl/AbstractOIDCMessageEncoder.java
@@ -26,7 +26,6 @@ import javax.annotation.Nullable;
import org.opensaml.messaging.encoder.MessageEncodingException;
import org.opensaml.messaging.encoder.servlet.AbstractHttpServletResponseMessageEncoder;
import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
import com.nimbusds.openid.connect.sdk.claims.ACR;
@@ -37,6 +36,7 @@ import net.shibboleth.oidc.profile.encoding.OIDCMessageEncoder;
import net.shibboleth.shared.collection.Pair;
import net.shibboleth.shared.logic.PredicateSupport;
import net.shibboleth.shared.net.URLBuilder;
+import net.shibboleth.shared.primitive.LoggerFactory;
/**
* Base class for OIDC message encoders.
diff --git a/oidc-common-profile-impl/src/main/java/net/shibboleth/oidc/profile/encoding/impl/HTTPPostAuthnRequestEncoder.java b/oidc-common-profile-impl/src/main/java/net/shibboleth/oidc/profile/encoding/impl/HTTPPostAuthnRequestEncoder.java
index 0a2c899..b472ef7 100644
--- a/oidc-common-profile-impl/src/main/java/net/shibboleth/oidc/profile/encoding/impl/HTTPPostAuthnRequestEncoder.java
+++ b/oidc-common-profile-impl/src/main/java/net/shibboleth/oidc/profile/encoding/impl/HTTPPostAuthnRequestEncoder.java
@@ -16,6 +16,7 @@ package net.shibboleth.oidc.profile.encoding.impl;
import java.io.OutputStreamWriter;
import java.io.Writer;
+import java.net.URI;
import java.nio.charset.StandardCharsets;
import java.util.List;
@@ -28,15 +29,17 @@ import org.opensaml.messaging.context.MessageContext;
import org.opensaml.messaging.encoder.MessageEncoder;
import org.opensaml.messaging.encoder.MessageEncodingException;
import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
import jakarta.servlet.http.HttpServletResponse;
import net.shibboleth.oidc.profile.core.OIDCAuthenticationRequest;
import net.shibboleth.oidc.profile.oauth2.config.OAuth2AuthorizationProfileConfiguration.HttpRequestMethod;
import net.shibboleth.shared.annotation.constraint.NotEmpty;
import net.shibboleth.shared.codec.HTMLEncoder;
+import net.shibboleth.shared.codec.StringDigester;
import net.shibboleth.shared.collection.Pair;
import net.shibboleth.shared.logic.Constraint;
+import net.shibboleth.shared.primitive.LoggerFactory;
+import net.shibboleth.shared.security.IdentifierGenerationStrategy;
import net.shibboleth.shared.servlet.HttpServletSupport;
/**
@@ -57,6 +60,36 @@ public class HTTPPostAuthnRequestEncoder extends AbstractOIDCMessageEncoder {
/** ID of the Velocity template used when using FORM POST response mode. */
@Nonnull @NotEmpty private String velocityTemplateId = DEFAULT_TEMPLATE_ID;
+ /** CSP digester for generating CSP hashes. */
+ @Nullable private StringDigester cspDigester;
+
+ /** CSP nonce generator. */
+ @Nullable private IdentifierGenerationStrategy cspNonceGenerator;
+
+ /**
+ * Sets a {@link StringDigester} to use in computing CSP digests in views.
+ *
+ * @param digester digester to set
+ *
+ * @since 3.1.0
+ */
+ public void setCSPDigester(@Nullable final StringDigester digester) {
+ checkSetterPreconditions();
+ cspDigester = digester;
+ }
+
+ /**
+ * Sets an {@link IdentifierGenerationStrategy} to use in computing CSP nonces in views.
+ *
+ * @param strategy nonce strategy
+ *
+ * @since 3.1.0
+ */
+ public void setCSPNonceGenerator(@Nullable final IdentifierGenerationStrategy strategy) {
+ checkSetterPreconditions();
+ cspNonceGenerator = strategy;
+ }
+
/**
* Set the Velocity template id.
*
@@ -67,8 +100,7 @@ public class HTTPPostAuthnRequestEncoder extends AbstractOIDCMessageEncoder {
* @param newVelocityTemplateId the new Velocity template id
*/
public void setVelocityTemplateId(@Nonnull @NotEmpty final String newVelocityTemplateId) {
- ifInitializedThrowUnmodifiabledComponentException();
- ifDestroyedThrowDestroyedComponentException();
+ checkSetterPreconditions();
velocityTemplateId =
Constraint.isNotEmpty(newVelocityTemplateId, "Velocity template id must not not be null or empty");
}
@@ -79,14 +111,16 @@ public class HTTPPostAuthnRequestEncoder extends AbstractOIDCMessageEncoder {
* @param newVelocityEngine the new VelocityEngine instane
*/
public void setVelocityEngine(@Nonnull final VelocityEngine newVelocityEngine) {
- ifInitializedThrowUnmodifiabledComponentException();
- ifDestroyedThrowDestroyedComponentException();
+ checkSetterPreconditions();
velocityEngine = Constraint.isNotNull(newVelocityEngine, "Velocity engine can not be null");
}
@Override
- public boolean test(@Nonnull final HttpRequestMethod requestMethod) {
+ public boolean test(@Nullable final HttpRequestMethod requestMethod) {
+ if (requestMethod == null) {
+ return false;
+ }
return requestMethod == HttpRequestMethod.POST;
}
@@ -94,24 +128,38 @@ public class HTTPPostAuthnRequestEncoder extends AbstractOIDCMessageEncoder {
* Construct form POST.
*
* @param request the authentication request.
+ * @param httpResponse the httpResponse.
* @return response message as velocity context.
*
* @throws MessageEncodingException on error building the parameters
*/
- private VelocityContext doPostEncode(@Nonnull final OIDCAuthenticationRequest request)
- throws MessageEncodingException {
+ private VelocityContext doPostEncode(@Nonnull final OIDCAuthenticationRequest request,
+ @Nonnull final HttpServletResponse httpResponse) throws MessageEncodingException {
+
final VelocityContext context = new VelocityContext();
final List<Pair<String, String>> params = createParametersFromRequest(request);
params.forEach(param -> context.put(param.getFirst(),HTMLEncoder.encodeForHTML(param.getSecond())));
- context.put("action", HTMLEncoder.encodeForHTMLAttribute(request.getEndpointURI().toString()));
- log.trace("Velocity context {}", params);
+ final URI endpoint = request.getEndpointURI();
+ if (endpoint == null) {
+ throw new MessageEncodingException("Endpoint URI for form action is null");
+ }
+ context.put("response", httpResponse);
+ if (cspDigester != null) {
+ context.put("cspDigester", cspDigester);
+ }
+ if (cspNonceGenerator != null) {
+ context.put("cspNonce", cspNonceGenerator);
+ }
+ context.put("action", HTMLEncoder.encodeForHTMLAttribute(endpoint.toString()));
+ log.trace("Velocity context OIDC parameters: {}", params);
return context;
}
@Override
protected void doEncode() throws MessageEncodingException {
- if (velocityEngine == null) {
+ final VelocityEngine velocityEngineLocal = velocityEngine;
+ if (velocityEngineLocal == null) {
throw new MessageEncodingException("VelocityEngine must be supplied for form post request mode");
}
@@ -124,12 +172,13 @@ public class HTTPPostAuthnRequestEncoder extends AbstractOIDCMessageEncoder {
}
try {
final HttpServletResponse response = getHttpServletResponse();
+ assert response != null;
HttpServletSupport.addNoCacheHeaders(response);
HttpServletSupport.setUTF8Encoding(response);
HttpServletSupport.setContentType(response, "text/html");
- final VelocityContext context = doPostEncode((OIDCAuthenticationRequest) outboundMessage);
+ final VelocityContext context = doPostEncode((OIDCAuthenticationRequest) outboundMessage, response);
try (final Writer out = new OutputStreamWriter(response.getOutputStream(), StandardCharsets.UTF_8)) {
- velocityEngine.mergeTemplate(velocityTemplateId, "UTF-8", context, out);
+ velocityEngineLocal.mergeTemplate(velocityTemplateId, "UTF-8", context, out);
out.flush();
}
} catch (final Exception e) {
diff --git a/oidc-common-profile-impl/src/main/java/net/shibboleth/oidc/profile/encoding/impl/HTTPRedirectAuthnRequestEncoder.java b/oidc-common-profile-impl/src/main/java/net/shibboleth/oidc/profile/encoding/impl/HTTPRedirectAuthnRequestEncoder.java
index 8918633..76b51aa 100644
--- a/oidc-common-profile-impl/src/main/java/net/shibboleth/oidc/profile/encoding/impl/HTTPRedirectAuthnRequestEncoder.java
+++ b/oidc-common-profile-impl/src/main/java/net/shibboleth/oidc/profile/encoding/impl/HTTPRedirectAuthnRequestEncoder.java
@@ -23,12 +23,12 @@ import org.opensaml.messaging.context.MessageContext;
import org.opensaml.messaging.encoder.MessageEncoder;
import org.opensaml.messaging.encoder.MessageEncodingException;
import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
import jakarta.servlet.http.HttpServletResponse;
import net.shibboleth.oidc.profile.core.OIDCAuthenticationRequest;
import net.shibboleth.oidc.profile.oauth2.config.OAuth2AuthorizationProfileConfiguration.HttpRequestMethod;
import net.shibboleth.shared.net.URLBuilder;
+import net.shibboleth.shared.primitive.LoggerFactory;
import net.shibboleth.shared.servlet.HttpServletSupport;
/**
@@ -60,6 +60,7 @@ public class HTTPRedirectAuthnRequestEncoder extends AbstractOIDCMessageEncoder
final String redirectURL = buildRedirectURL(messageContext, (OIDCAuthenticationRequest)outboundMessage);
final HttpServletResponse response = getHttpServletResponse();
+ assert response != null;
HttpServletSupport.addNoCacheHeaders(response);
HttpServletSupport.setUTF8Encoding(response);
HttpServletSupport.setContentType(response, "application/x-www-form-urlencoded");
diff --git a/oidc-common-profile-impl/src/test/java/net/shibboleth/oidc/profile/encoding/impl/HTTPPostAuthnRequestEncoderTest.java b/oidc-common-profile-impl/src/test/java/net/shibboleth/oidc/profile/encoding/impl/HTTPPostAuthnRequestEncoderTest.java
index bf06347..01e49b3 100644
--- a/oidc-common-profile-impl/src/test/java/net/shibboleth/oidc/profile/encoding/impl/HTTPPostAuthnRequestEncoderTest.java
+++ b/oidc-common-profile-impl/src/test/java/net/shibboleth/oidc/profile/encoding/impl/HTTPPostAuthnRequestEncoderTest.java
@@ -26,7 +26,6 @@ import org.springframework.mock.web.MockHttpServletResponse;
import org.testng.annotations.BeforeMethod;
import org.testng.annotations.Test;
-import com.nimbusds.jwt.JWT;
import com.nimbusds.jwt.JWTClaimsSet;
import com.nimbusds.jwt.PlainJWT;
import com.nimbusds.oauth2.sdk.ResponseType;
@@ -35,11 +34,14 @@ import com.nimbusds.openid.connect.sdk.OIDCClaimsRequest;
import com.nimbusds.openid.connect.sdk.assurance.claims.VerifiedClaimsSetRequest;
import net.shibboleth.oidc.profile.core.OIDCAuthenticationRequest;
+import net.shibboleth.shared.codec.StringDigester;
+import net.shibboleth.shared.codec.StringDigester.OutputFormat;
import net.shibboleth.shared.component.UninitializedComponentException;
import net.shibboleth.shared.servlet.impl.HttpServletRequestResponseContext;
import net.shibboleth.shared.servlet.impl.ThreadLocalHttpServletResponseSupplier;
import net.shibboleth.shared.testing.VelocityEngine;
+
/** Test for the HTTPPostAuthnRequestEncoder.*/
public class HTTPPostAuthnRequestEncoderTest {
@@ -94,6 +96,35 @@ public class HTTPPostAuthnRequestEncoderTest {
}
+ @Test
+ public void testSuccesfullEncoding_WithClaims_And_CspComputed() throws Exception {
+ final OIDCClaimsRequest requestedClaims = new OIDCClaimsRequest()
+ .withIDTokenClaimsRequest(new VerifiedClaimsSetRequest().add("given_name"))
+ .withUserInfoClaimsRequest(new VerifiedClaimsSetRequest().add("family_name"));
+ request.setRequestedClaims(requestedClaims);
+ request.setProviderSupportsClaimsParameter(true);
+
+ encoder.setCSPDigester(new StringDigester("SHA-256", OutputFormat.HEX_LOWER));
+
+ encoder.initialize();
+ encoder.encode();
+ final String response = mockResponse.getContentAsString();
+ assertNotNull(response);
+
+ final String csp = mockResponse.getHeader("Content-Security-Policy");
+ assertNotNull(csp);
+ assertTrue(csp != null &&
+ csp.contains("script-src-attr 'unsafe-hashes' 'sha256-78f9e25449128af5ff73b5d604669faa1f2d4a9891aca8aa61ea9b1bb3754ce1"));
+
+ // These are all required
+ assertTrue(response.contains("client_id"));
+ assertTrue(response.contains("response_type"));
+ assertTrue(response.contains("client_id"));
+ assertTrue(response.contains("scope"));
+ assertTrue(response.contains("claims"));
+
+ }
+
@Test
public void testSuccesfullEncoding_WithRequestObject() throws Exception {
diff --git a/oidc-common-profile-impl/src/test/resources/templates/oidc-request-form-post.vm b/oidc-common-profile-impl/src/test/resources/templates/oidc-request-form-post.vm
index ff3955f..eb445e6 100644
--- a/oidc-common-profile-impl/src/test/resources/templates/oidc-request-form-post.vm
+++ b/oidc-common-profile-impl/src/test/resources/templates/oidc-request-form-post.vm
@@ -1,7 +1,13 @@
##
## Velocity Template for OIDC Form Post response mode.
+## cspDigester - Calculates base64-encoded SHA-2 hashes (call apply). Can be null for backward compatibility.
+## cspNonce - Calculates secure nonces (call generateIdentifier). Can be null for backward compatibility.
##
##
+#set ($onLoad = "document.forms[0].submit()")
+#if($cspDigester)
+ $response.addHeader("Content-Security-Policy","object-src 'none'; script-src 'none'; script-src-attr 'unsafe-hashes' 'sha256-$cspDigester.apply($onLoad)'")
+#end
<!DOCTYPE html>
<html>
@@ -9,7 +15,7 @@
<meta charset="utf-8" />
</head>
-<body onload="document.forms[0].submit()">
+<body onload="$onLoad">
<noscript>
<p>
<strong>Note:</strong> Since your browser does not support JavaScript, you must press the Continue button once to proceed.
@@ -17,7 +23,7 @@
</noscript>
<form action="${action}" method="post">
- <div>
+ <div>
#if($client_id)
<input type="hidden" name="client_id" value="${client_id}" />#end #if($scope)
@@ -33,7 +39,9 @@
<input type="hidden" name="prompt" value="${prompt}" />#end #if($request)
- <input type="hidden" name="request" value="${request}" />#end #if($claims)
+ <input type="hidden" name="request" value="${request}" />#end #if($acr_values)
+
+ <input type="hidden" name="acr_values" value="${acr_values}" />#end #if($claims)
<input type="hidden" name="claims" value="${claims}" />#end #if($nonce)
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
More information about the commits
mailing list