[GitHub] [wicket] andruhon commented on issue #374: WICKET-6682 add CSP nonce support: initial commit
andruhon commented on issue #374: WICKET-6682 add CSP nonce support: initial commit URL: https://github.com/apache/wicket/pull/374#issuecomment-511157387 Thanks @martin-g ! There's another PR for the same issue https://github.com/apache/wicket/pull/376 . Sven recommended to try different approach above. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [wicket] martin-g commented on a change in pull request #374: WICKET-6682 add CSP nonce support: initial commit
martin-g commented on a change in pull request #374: WICKET-6682 add CSP nonce
support: initial commit
URL: https://github.com/apache/wicket/pull/374#discussion_r303218028
##
File path:
wicket-core/src/main/java/org/apache/wicket/markup/head/nonce/AbstractNonceStrategy.java
##
@@ -0,0 +1,44 @@
+/*
+ * 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.wicket.markup.head.nonce;
+
+import org.apache.wicket.MetaDataKey;
+import org.apache.wicket.markup.head.INonceStrategy;
+
+import java.security.SecureRandom;
+import java.util.Base64;
+
+/**
+ * {@inheritDoc}
+ */
+public abstract class AbstractNonceStrategy implements INonceStrategy {
+
+private static final int NONCE_LENGTH = 24;
+
+MetaDataKey NONCE_KEY = new MetaDataKey() {};
+
+private static final SecureRandom RND = new SecureRandom();
Review comment:
Is this thread safe ? It would be better to use `ThreadLocalRandom` instead
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [wicket] martin-g commented on a change in pull request #374: WICKET-6682 add CSP nonce support: initial commit
martin-g commented on a change in pull request #374: WICKET-6682 add CSP nonce
support: initial commit
URL: https://github.com/apache/wicket/pull/374#discussion_r303217930
##
File path:
wicket-core/src/main/java/org/apache/wicket/markup/head/INonceStrategy.java
##
@@ -0,0 +1,37 @@
+/*
+ * 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.wicket.markup.head;
+
+/**
+ * Nonce generation strategy. Implementation should define how and when the
nonce is generated.
+ *
+ * Strategy Should be configured in the Application.
Review comment:
`The` `s`trategy should be configured in the application's `{@link
SecuritySettings resource settings}`
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [wicket] martin-g commented on a change in pull request #374: WICKET-6682 add CSP nonce support: initial commit
martin-g commented on a change in pull request #374: WICKET-6682 add CSP nonce
support: initial commit
URL: https://github.com/apache/wicket/pull/374#discussion_r303218087
##
File path:
wicket-core/src/main/java/org/apache/wicket/markup/head/nonce/CspUtils.java
##
@@ -0,0 +1,58 @@
+/*
+ * 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.wicket.markup.head.nonce;
+
+import org.apache.wicket.Application;
+import org.apache.wicket.markup.head.INonceStrategy;
+
+import java.util.Optional;
+
+/**
+ * Provide some helpers for CSP security.
+ * {@see https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Security-Policy/script-src#Unsafe_inline_script;>https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Security-Policy/script-src#Unsafe_inline_script}
Review comment:
The text content could be just `CSP Unsafe inline script`. No need to use
the full url again.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [wicket] martin-g commented on a change in pull request #374: WICKET-6682 add CSP nonce support: initial commit
martin-g commented on a change in pull request #374: WICKET-6682 add CSP nonce
support: initial commit
URL: https://github.com/apache/wicket/pull/374#discussion_r303218202
##
File path:
wicket-core/src/main/java/org/apache/wicket/markup/head/nonce/PerSessionNonceStrategy.java
##
@@ -0,0 +1,50 @@
+/*
+ * 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.wicket.markup.head.nonce;
+
+import org.apache.wicket.Session;
+import org.apache.wicket.util.lang.Args;
+
+/**
+ * Per session Nonce generation strategy.
+ * Generates a new nonce for each new session.
+ */
+public class PerSessionNonceStrategy extends AbstractNonceStrategy {
+
+@Override
+public String getNonce() {
+Session session = Session.get();
+String nonce = session.getMetaData(NONCE_KEY);
+if (nonce == null) {
+nonce = generateNonce();
+saveNonce(session, nonce);
+}
+return nonce;
+}
+
+@Override
+public void invalidate() {
+saveNonce(Session.get(), generateNonce());
+}
+
+private void saveNonce(Session session, String nonce) {
+Args.notNull(session, "session");
+Args.notNull(nonce, "csp");
+session.setMetaData(NONCE_KEY, nonce);
Review comment:
This should also bind the Session. Otherwise the Wicket Session could stay
temporary for the request cycle and a new one will be created for the next
request.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [wicket] martin-g commented on a change in pull request #374: WICKET-6682 add CSP nonce support: initial commit
martin-g commented on a change in pull request #374: WICKET-6682 add CSP nonce
support: initial commit
URL: https://github.com/apache/wicket/pull/374#discussion_r303218212
##
File path:
wicket-core/src/main/java/org/apache/wicket/settings/SecuritySettings.java
##
@@ -243,4 +247,26 @@ public SecuritySettings setAuthenticationStrategy(final
IAuthenticationStrategy
authenticationStrategy = strategy;
return this;
}
+
+ /**
+* Get CSP nonce strategy.
+*
+* @return returns the CSP nonce strategy. Null if not configured.
+*/
+ public INonceStrategy getNonceStrategy() {
Review comment:
get`Csp`NonceStrategy() ?!
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [wicket] martin-g commented on a change in pull request #374: WICKET-6682 add CSP nonce support: initial commit
martin-g commented on a change in pull request #374: WICKET-6682 add CSP nonce
support: initial commit
URL: https://github.com/apache/wicket/pull/374#discussion_r303217903
##
File path:
wicket-core/src/main/java/org/apache/wicket/core/util/string/JavaScriptUtils.java
##
@@ -152,7 +153,9 @@ public static void writeJavaScriptUrl(final Response
response, final CharSequenc
public static void writeJavaScriptUrl(final Response response, final
CharSequence url,
final String id, boolean defer, String charset, boolean async)
{
-
[GitHub] [wicket] martin-g commented on a change in pull request #374: WICKET-6682 add CSP nonce support: initial commit
martin-g commented on a change in pull request #374: WICKET-6682 add CSP nonce support: initial commit URL: https://github.com/apache/wicket/pull/374#discussion_r303217943 ## File path: wicket-core/src/main/java/org/apache/wicket/markup/head/INonceStrategy.java ## @@ -0,0 +1,37 @@ +/* + * 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.wicket.markup.head; + +/** + * Nonce generation strategy. Implementation should define how and when the nonce is generated. + * + * Strategy Should be configured in the Application. + * The nonce strategy itself is only responsible for nonce generation. Review comment: ... for `CSP nonce generation. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
Re: FYI
Hi, I've committed a change to Wicket's Buildbot config that adds JAVA_HOME for the Java 11, 13 and 14 builds. Hopefully this will fix the problem! On Fri, Jul 12, 2019 at 5:01 PM Andrea Del Bene wrote: > Problem with javadoc-maven-plugin 3.1.x and CI: > > https://issues.apache.org/jira/browse/INFRA-18741 > > -- > Andrea Del Bene. > Apache Wicket committer. >
[GitHub] [wicket] svenmeier commented on a change in pull request #376: WICKET-6682 add CSP nonce support: initial commit
svenmeier commented on a change in pull request #376: WICKET-6682 add CSP nonce support: initial commit URL: https://github.com/apache/wicket/pull/376#discussion_r303195834 ## File path: wicket-core/src/main/java/org/apache/wicket/core/util/string/JavaScriptUtils.java ## @@ -152,28 +155,48 @@ public static void writeJavaScriptUrl(final Response response, final CharSequenc public static void writeJavaScriptUrl(final Response response, final CharSequence url, Review comment: Ah yes, that's what I was looking for :). This one can be deprecated and should no longer be called. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [wicket] andruhon commented on a change in pull request #376: WICKET-6682 add CSP nonce support: initial commit
andruhon commented on a change in pull request #376: WICKET-6682 add CSP nonce support: initial commit URL: https://github.com/apache/wicket/pull/376#discussion_r303195348 ## File path: wicket-core/src/main/java/org/apache/wicket/core/util/string/JavaScriptUtils.java ## @@ -152,28 +155,48 @@ public static void writeJavaScriptUrl(final Response response, final CharSequenc public static void writeJavaScriptUrl(final Response response, final CharSequence url, Review comment: There's a new method just after this one. I suppose we can mark this one as Deprecated and get rid of all usages in the wicket code. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [wicket] svenmeier commented on a change in pull request #376: WICKET-6682 add CSP nonce support: initial commit
svenmeier commented on a change in pull request #376: WICKET-6682 add CSP nonce support: initial commit URL: https://github.com/apache/wicket/pull/376#discussion_r303195128 ## File path: wicket-core/src/main/java/org/apache/wicket/core/util/string/JavaScriptUtils.java ## @@ -152,28 +155,48 @@ public static void writeJavaScriptUrl(final Response response, final CharSequenc public static void writeJavaScriptUrl(final Response response, final CharSequence url, Review comment: Well, if we continue what we have done in the past, we have to add yet another parameter "nonce". IMHO it's better to change it to receive a valueMap instead, and let the caller fill it. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [wicket] andruhon commented on a change in pull request #376: WICKET-6682 add CSP nonce support: initial commit
andruhon commented on a change in pull request #376: WICKET-6682 add CSP nonce
support: initial commit
URL: https://github.com/apache/wicket/pull/376#discussion_r303193907
##
File path:
wicket-core/src/main/java/org/apache/wicket/markup/head/filter/CspNonceHeaderResponse.java
##
@@ -0,0 +1,348 @@
+package org.apache.wicket.markup.head.filter;
+
+import org.apache.wicket.Application;
+import org.apache.wicket.core.request.handler.IPartialPageRequestHandler;
+import org.apache.wicket.core.util.string.CssUtils;
+import org.apache.wicket.core.util.string.JavaScriptUtils;
+import org.apache.wicket.markup.head.*;
+import org.apache.wicket.markup.html.DecoratingHeaderResponse;
+import org.apache.wicket.request.Response;
+import org.apache.wicket.request.cycle.RequestCycle;
+import org.apache.wicket.request.mapper.parameter.PageParameters;
+import org.apache.wicket.request.resource.ResourceReference;
+import org.apache.wicket.util.lang.Args;
+import org.apache.wicket.util.string.Strings;
+import org.apache.wicket.util.value.HeaderItemAttribute;
+import org.apache.wicket.util.value.HeaderItemAttributeMap;
+
+/**
+ * Add CSP nonce to all relevant JavaScript and CSS header items
+ *
+ * Note: please don't forget to wrap with {@link ResourceAggregator}
+ * when setting it up with {@link Application#setHeaderResponseDecorator},
+ * otherwise dependencies will not be rendered
+ */
+public abstract class CspNonceHeaderResponse extends DecoratingHeaderResponse {
+
+public CspNonceHeaderResponse(IHeaderResponse real) {
+super(real);
+}
+
+@Override
+public void render(HeaderItem item) {
+while (item instanceof IWrappedHeaderItem) {
+item = ((IWrappedHeaderItem)item).getWrapped();
+}
+
+final String nonce = getNonce();
+
+if (item instanceof JavaScriptContentHeaderItem) {
+item = new JavaScriptContentWithNonceHeaderItem(
+((JavaScriptContentHeaderItem) item).getJavaScript(),
+((JavaScriptContentHeaderItem) item).getId()
+).setNonce(nonce);
+} if (item instanceof JavaScriptReferenceHeaderItem) {
+JavaScriptReferenceHeaderItem headerItem =
(JavaScriptReferenceHeaderItem) item;
+item = new JavaScriptReferenceWithNonceHeaderItem(
+headerItem.getReference(),
+headerItem.getPageParameters(),
+headerItem.getId(),
+headerItem.isDefer(),
+headerItem.getCharset()
+).setNonce(nonce);
+} else if (item instanceof JavaScriptUrlReferenceHeaderItem) {
+JavaScriptUrlReferenceHeaderItem headerItem =
(JavaScriptUrlReferenceHeaderItem) item;
+item = new JavaScriptUrlReferenceWithNonceHeaderItem(
+headerItem.getUrl(),
+headerItem.getId(),
+headerItem.isDefer(),
+headerItem.getCharset()
+).setNonce(nonce);
+} else if (item instanceof OnDomReadyHeaderItem) {
+OnDomReadyHeaderItem headerItem = (OnDomReadyHeaderItem) item;
+item = new
OnDomReadyHeaderWithNonceItem(headerItem.getJavaScript()).setNonce(nonce);
+} else if (item instanceof OnLoadHeaderItem) {
+OnLoadHeaderItem headerItem = (OnLoadHeaderItem) item;
+item = new
OnLoadWithNonceHeaderItem(headerItem.getJavaScript()).setNonce(nonce);
+} else if (item instanceof CssContentHeaderItem) {
+CssContentHeaderItem headerItem = (CssContentHeaderItem) item;
+item = new CssContentHeaderWithNonceItem(headerItem.getCss(),
headerItem.getId()).setNonce(nonce);
+} else if (item instanceof CssReferenceHeaderItem) {
+CssReferenceHeaderItem headerItem = (CssReferenceHeaderItem) item;
+item = new CssReferenceWithNonceHeaderItem(
+headerItem.getReference(),
+headerItem.getPageParameters(),
+headerItem.getMedia(),
+headerItem.getRel()
+).setNonce(nonce);
+} else if (item instanceof CssUrlReferenceHeaderItem) {
+CssUrlReferenceHeaderItem headerItem = (CssUrlReferenceHeaderItem)
item;
+item = new CssUrlReferenceWithNonceHeaderItem(
+headerItem.getUrl(),
+headerItem.getMedia(),
+headerItem.getRel()
+).setNonce(nonce);
+}
+
+super.render(item);
+}
+
+protected abstract String getNonce();
+
+protected static void renderScriptReferenceHeaderItem(
+Response response,
+final CharSequence url,
+final String id,
+boolean defer,
+String charset,
+boolean async,
+String nonce
+) {
+Args.notEmpty(url, "url");
+Args.notEmpty(nonce, "nonce");
+
