Repository: shapesecurity/salvation (java)

Issue:
Adjust base64-value validation error reporting to new base64-value grammar
> base64-value  = 1*( ALPHA / DIGIT / "+" / "/" / "-" / "_" )*2( "=" ) 

Note that length recommendation is still valid:
>  The generated value SHOULD be at least 128 bits long (before encoding), and SHOULD be generated via a cryptographically secure random number generator in order to ensure that the value is difficult for an attacker to predict.

Pull request description:
fixes #139
- do not warn if base64 value matches either table 1 or table 2 grammar of RFC4648
- do not mention RFC4648 in warning messages

Patch:
diff --git a/src/main/java/com/shapesecurity/salvation/data/Base64Value.java b/src/main/java/com/shapesecurity/salvation/data/Base64Value.java
index 94fe42e..ade6f97 100644
--- a/src/main/java/com/shapesecurity/salvation/data/Base64Value.java
+++ b/src/main/java/com/shapesecurity/salvation/data/Base64Value.java
@@ -24,7 +24,7 @@ public class Base64Value implements Show {
 
         if (chars.length % 4 != 0) {
             throw new IllegalArgumentException("Invalid base64-value (should be multiple of 4 bytes: " + chars.length
-                + "). Consider using RFC4648 compliant base64 encoding implementation.");
+                + ").");
         }
 
         int i;
@@ -34,17 +34,17 @@ public class Base64Value implements Show {
             }
             if (!isBase64Char(chars[i])) {
                 throw new IllegalArgumentException(
-                    "Invalid base64-value (characters are not in the base64-value grammar). Consider using RFC4648 compliant base64 encoding implementation.");
+                    "Invalid base64-value (characters are not in the base64-value grammar).");
             }
         }
         if (i < chars.length - 2) {
             throw new IllegalArgumentException(
-                "Invalid base64-value (bad padding). Consider using RFC4648 compliant base64 encoding implementation.");
+                "Invalid base64-value (bad padding).");
         }
         for (; i < chars.length; i++) {
             if (chars[i] != '=') {
                 throw new IllegalArgumentException(
-                    "Invalid base64-value padding (illegal characters). Consider using RFC4648 compliant base64 encoding implementation.");
+                    "Invalid base64-value padding (illegal characters).");
             }
         }
 
@@ -57,7 +57,8 @@ public class Base64Value implements Show {
         return '0' <= ch && ch <= '9' ||
             'A' <= ch && ch <= 'Z' ||
             'a' <= ch && ch <= 'z' ||
-            ch == '+' || ch == '/';
+            ch == '+' || ch == '/' ||
+            ch == '-' || ch == '_';
     }
 
     public int size() {
diff --git a/src/main/java/com/shapesecurity/salvation/directiveValues/NonceSource.java b/src/main/java/com/shapesecurity/salvation/directiveValues/NonceSource.java
index 4f76437..911f87b 100644
--- a/src/main/java/com/shapesecurity/salvation/directiveValues/NonceSource.java
+++ b/src/main/java/com/shapesecurity/salvation/directiveValues/NonceSource.java
@@ -23,9 +23,10 @@ public class NonceSource implements SourceExpression, MatchesNonce {
             String safeValue = this.value.replace('-', '+').replace('_', '/');
             base64Value = new Base64Value(safeValue);
             // warn if value is not RFC4648
-            if (this.value.contains("-") || this.value.contains("_")) {
+            if ((this.value.contains("-") || this.value.contains("_")) &&
+                (this.value.contains("+") || this.value.contains("/"))) {
                 errors.add(
-                    "Invalid base64-value (characters are not in the base64-value grammar). Consider using RFC4648 compliant base64 encoding implementation.");
+                    "Invalid base64-value. Must use either RFC4648 \"base64\" characters (including + and /) or RFC4648 \"base64url\" characters (including - and _), but not both.");
             }
         } catch (IllegalArgumentException e) {
             errors.add(e.getMessage());


Test patch:
diff --git a/src/test/java/com/shapesecurity/salvation/Base64ValueTest.java b/src/test/java/com/shapesecurity/salvation/Base64ValueTest.java
index d005dc5..1290be7 100644
--- a/src/test/java/com/shapesecurity/salvation/Base64ValueTest.java
+++ b/src/test/java/com/shapesecurity/salvation/Base64ValueTest.java
@@ -4,6 +4,8 @@ import com.shapesecurity.salvation.data.Notice;
 import org.junit.Test;
 
 import java.util.ArrayList;
+import java.util.Base64;
+import java.util.Random;
 
 import static org.junit.Assert.assertEquals;
 
@@ -43,7 +45,7 @@ public class Base64ValueTest {
         Parser.parse("script-src 'self' https://example.com 'nonce-abc'", "https://origin", notices);
         assertEquals(1, notices.size());
         assertEquals(
-            "Invalid base64-value (should be multiple of 4 bytes: 3). Consider using RFC4648 compliant base64 encoding implementation.",
+            "Invalid base64-value (should be multiple of 4 bytes: 3).",
             notices.get(0).show());
     }
 
@@ -54,15 +56,37 @@ public class Base64ValueTest {
             notices);
         assertEquals(1, notices.size());
         assertEquals(
-            "Invalid base64-value (characters are not in the base64-value grammar). Consider using RFC4648 compliant base64 encoding implementation.",
+            "Invalid base64-value (characters are not in the base64-value grammar).",
             notices.get(0).show());
 
         notices.clear();
         Parser.parse("script-src 'self' https://example.com 'nonce-1^=='", "https://origin", notices);
         assertEquals(1, notices.size());
         assertEquals(
-            "Invalid base64-value (characters are not in the base64-value grammar). Consider using RFC4648 compliant base64 encoding implementation.",
+            "Invalid base64-value (characters are not in the base64-value grammar).",
+            notices.get(0).show());
+
+        notices.clear();
+        Parser.parse("script-src 'self' https://example.com 'nonce-12_/-+=='", "https://origin", notices);
+        assertEquals(2, notices.size());
+        assertEquals(
+            "Invalid base64-value. Must use either RFC4648 \"base64\" characters (including + and /) or RFC4648 \"base64url\" characters (including - and _), but not both.",
             notices.get(0).show());
+        assertEquals(
+            "CSP specification recommends nonce-value to be at least 128 bits long (before encoding).",
+            notices.get(1).show());
+
+        Parser.parse("script-src 'self' https://example.com 'nonce-12-+/'", "https://origin", notices);
+        assertEquals(3, notices.size());
+        assertEquals(
+            "Invalid base64-value. Must use either RFC4648 \"base64\" characters (including + and /) or RFC4648 \"base64url\" characters (including - and _), but not both.",
+            notices.get(0).show());
+        assertEquals(
+            "CSP specification recommends nonce-value to be at least 128 bits long (before encoding).",
+            notices.get(1).show());
+        assertEquals(
+            "Invalid base64-value (should be multiple of 4 bytes: 5).",
+            notices.get(2).show());
     }
 
     @Test public void testIllegalPadding() {
@@ -71,14 +95,14 @@ public class Base64ValueTest {
         Parser.parse("script-src 'self' https://example.com 'nonce-12=+'", "https://origin", notices);
         assertEquals(1, notices.size());
         assertEquals(
-            "Invalid base64-value padding (illegal characters). Consider using RFC4648 compliant base64 encoding implementation.",
+            "Invalid base64-value padding (illegal characters).",
             notices.get(0).show());
 
         notices.clear();
         Parser.parse("script-src 'self' https://example.com 'nonce-1==='", "https://origin", notices);
         assertEquals(1, notices.size());
         assertEquals(
-            "Invalid base64-value (bad padding). Consider using RFC4648 compliant base64 encoding implementation.",
+            "Invalid base64-value (bad padding).",
             notices.get(0).show());
     }
 
@@ -86,12 +110,36 @@ public class Base64ValueTest {
         ArrayList<Notice> notices = new ArrayList<>();
 
         Parser.parse("script-src 'self' https://example.com 'nonce-31231asda_dsdsxc'", "https://origin", notices);
-        assertEquals(2, notices.size());
-        assertEquals(
-            "Invalid base64-value (characters are not in the base64-value grammar). Consider using RFC4648 compliant base64 encoding implementation.",
-            notices.get(0).show());
+        assertEquals(1, notices.size());
         assertEquals("CSP specification recommends nonce-value to be at least 128 bits long (before encoding).",
-            notices.get(1).show());
+            notices.get(0).show());
+
+    }
+
+    @Test public void testValid() {
+
+        for (int i = 16; i <= 32; i++) {
+            byte[] b = new byte[i];
+            new Random().nextBytes(b);
+            String encoded = Base64.getEncoder().encodeToString(b);
+            ArrayList<Notice> notices = new ArrayList<>();
+
+            Parser.parse("script-src 'self' https://example.com 'nonce-" + encoded + "'",
+                "https://origin", notices);
+            assertEquals(0, notices.size());
+        }
+
+        for (int i = 16; i <= 32; i++) {
+            byte[] b = new byte[i];
+            new Random().nextBytes(b);
+            String encoded = Base64.getUrlEncoder().encodeToString(b);
+            ArrayList<Notice> notices = new ArrayList<>();
+
+            Parser.parse("script-src 'self' https://example.com 'nonce-" + encoded + "'",
+                "https://origin", notices);
+            assertEquals(0, notices.size());
+
+        }
 
     }
 }
diff --git a/src/test/java/com/shapesecurity/salvation/LocationTest.java b/src/test/java/com/shapesecurity/salvation/LocationTest.java
index 18c984c..0435279 100644
--- a/src/test/java/com/shapesecurity/salvation/LocationTest.java
+++ b/src/test/java/com/shapesecurity/salvation/LocationTest.java
@@ -417,10 +417,10 @@ public class LocationTest extends CSPTest {
         assertEquals(2, notices.size());
         Notice notice = notices.get(0);
         assertEquals(
-            "1:28: Invalid base64-value (should be multiple of 4 bytes: 3). Consider using RFC4648 compliant base64 encoding implementation.",
+            "1:28: Invalid base64-value (should be multiple of 4 bytes: 3).",
             notice.show());
         assertEquals(
-            "Warning: Invalid base64-value (should be multiple of 4 bytes: 3). Consider using RFC4648 compliant base64 encoding implementation.",
+            "Warning: Invalid base64-value (should be multiple of 4 bytes: 3).",
             notice.toString());
         notice = notices.get(1);
         assertEquals(
diff --git a/src/test/java/com/shapesecurity/salvation/ParserTest.java b/src/test/java/com/shapesecurity/salvation/ParserTest.java
index 89715a2..1a96d36 100644
--- a/src/test/java/com/shapesecurity/salvation/ParserTest.java
+++ b/src/test/java/com/shapesecurity/salvation/ParserTest.java
@@ -479,7 +479,7 @@ public class ParserTest extends CSPTest {
         assertEquals(0, p.getDirectives().size());
         assertEquals(1, notices.size());
         assertEquals(
-            "Invalid base64-value (should be multiple of 4 bytes: 43). Consider using RFC4648 compliant base64 encoding implementation.",
+            "Invalid base64-value (should be multiple of 4 bytes: 43).",
             notices.get(0).message);
 
         assertEquals("directive-name, directive-value",