Use URL-safe Base64 encoder/decoder (cryostatio#419)

* Use URL-safe Base64 encoder/decoder Use encoder which produces, and decoder which expects, the URL-safe Base64 alphabet variant. The decoder is also tolerant of missing padding characters at the end, which the client will not send due to their illegality in the WebSocket Subprotocol header value * Update web-client to include base64url client-side changes
andrewazores · Apr 14, 2021 · f72053d · f72053d
1 parent d20d8af
commit f72053d
Show file tree

Hide file tree

Showing 7 changed files with 62 additions and 10 deletions.
diff --git a/src/main/java/com/redhat/rhjmc/containerjfr/net/BasicAuthManager.java b/src/main/java/com/redhat/rhjmc/containerjfr/net/BasicAuthManager.java
@@ -112,7 +112,7 @@ public Future<Boolean> validateHttpHeader(Supplier<String> headerProvider) {
         String b64 = matcher.group(1);
         try {
             String decoded =
-                    new String(Base64.getDecoder().decode(b64), StandardCharsets.UTF_8).trim();
+                    new String(Base64.getUrlDecoder().decode(b64), StandardCharsets.UTF_8).trim();
             return validateToken(() -> decoded);
         } catch (IllegalArgumentException e) {
             return CompletableFuture.completedFuture(false);
@@ -136,7 +136,7 @@ public Future<Boolean> validateWebSocketSubProtocol(Supplier<String> subProtocol
         String b64 = matcher.group(1);
         try {
             String decoded =
-                    new String(Base64.getDecoder().decode(b64), StandardCharsets.UTF_8).trim();
+                    new String(Base64.getUrlDecoder().decode(b64), StandardCharsets.UTF_8).trim();
             return validateToken(() -> decoded);
         } catch (IllegalArgumentException e) {
             return CompletableFuture.completedFuture(false);

diff --git a/src/main/java/com/redhat/rhjmc/containerjfr/net/OpenShiftAuthManager.java b/src/main/java/com/redhat/rhjmc/containerjfr/net/OpenShiftAuthManager.java
@@ -137,7 +137,7 @@ public Future<Boolean> validateWebSocketSubProtocol(Supplier<String> subProtocol
         String b64 = matcher.group(1);
         try {
             String decoded =
-                    new String(Base64.getDecoder().decode(b64), StandardCharsets.UTF_8).trim();
+                    new String(Base64.getUrlDecoder().decode(b64), StandardCharsets.UTF_8).trim();
             return validateToken(() -> decoded);
         } catch (IllegalArgumentException e) {
             return CompletableFuture.completedFuture(false);

diff --git a/.../java/com/redhat/rhjmc/containerjfr/net/web/http/AbstractAuthenticatedRequestHandler.java b/.../java/com/redhat/rhjmc/containerjfr/net/web/http/AbstractAuthenticatedRequestHandler.java
@@ -130,7 +130,7 @@ protected ConnectionDescriptor getConnectionDescriptorFromContext(RoutingContext
                     try {
                         c =
                                 new String(
-                                        Base64.getDecoder().decode(m.group("credentials")),
+                                        Base64.getUrlDecoder().decode(m.group("credentials")),
                                         StandardCharsets.UTF_8);
                     } catch (IllegalArgumentException iae) {
                         ctx.response().putHeader(JMX_AUTHENTICATE_HEADER, "Basic");

diff --git a/...main/java/com/redhat/rhjmc/containerjfr/net/web/http/api/v2/AbstractV2RequestHandler.java b/...main/java/com/redhat/rhjmc/containerjfr/net/web/http/api/v2/AbstractV2RequestHandler.java
@@ -146,7 +146,7 @@ protected ConnectionDescriptor getConnectionDescriptorFromParams(RequestParamete
                     try {
                         c =
                                 new String(
-                                        Base64.getDecoder().decode(m.group("credentials")),
+                                        Base64.getUrlDecoder().decode(m.group("credentials")),
                                         StandardCharsets.UTF_8);
                     } catch (IllegalArgumentException iae) {
                         params.getHeaders().set(JMX_AUTHENTICATE_HEADER, "Basic");

diff --git a/src/test/java/com/redhat/rhjmc/containerjfr/net/BasicAuthManagerTest.java b/src/test/java/com/redhat/rhjmc/containerjfr/net/BasicAuthManagerTest.java
@@ -308,6 +308,7 @@ void shouldPassKnownCredentials() throws Exception {
             Mockito.when(fs.exists(mockPath)).thenReturn(true);
             Mockito.when(fs.isRegularFile(mockPath)).thenReturn(true);
             Mockito.when(fs.isReadable(mockPath)).thenReturn(true);
+            // credentials: "user:pass"
             BufferedReader props =
                     new BufferedReader(
                             new StringReader(
@@ -319,6 +320,56 @@ void shouldPassKnownCredentials() throws Exception {
                             .get());
         }
 
+        @Test
+        void shouldPassKnownCredentialsWithPadding() throws Exception {
+            Path mockPath = Mockito.mock(Path.class);
+            Mockito.when(
+                            fs.pathOf(
+                                    System.getProperty("user.home"),
+                                    BasicAuthManager.USER_PROPERTIES_FILENAME))
+                    .thenReturn(mockPath);
+            Mockito.when(fs.exists(mockPath)).thenReturn(true);
+            Mockito.when(fs.isRegularFile(mockPath)).thenReturn(true);
+            Mockito.when(fs.isReadable(mockPath)).thenReturn(true);
+            // credentials: "user:pass1234"
+            BufferedReader props =
+                    new BufferedReader(
+                            new StringReader(
+                                    "user:bd94dcda26fccb4e68d6a31f9b5aac0b571ae266d822620e901ef7ebe3a11d4f"));
+            Mockito.when(fs.readFile(mockPath)).thenReturn(props);
+            Assertions.assertTrue(
+                    mgr.validateWebSocketSubProtocol(
+                                    () -> "basic.authorization.containerjfr.dXNlcjpwYXNzMTIzNA==")
+                            .get());
+        }
+
+        @Test
+        void shouldPassKnownCredentialsAndStrippedPadding() throws Exception {
+            Path mockPath = Mockito.mock(Path.class);
+            Mockito.when(
+                            fs.pathOf(
+                                    System.getProperty("user.home"),
+                                    BasicAuthManager.USER_PROPERTIES_FILENAME))
+                    .thenReturn(mockPath);
+            Mockito.when(fs.exists(mockPath)).thenReturn(true);
+            Mockito.when(fs.isRegularFile(mockPath)).thenReturn(true);
+            Mockito.when(fs.isReadable(mockPath)).thenReturn(true);
+            // credentials: "user:pass1234"
+            BufferedReader props =
+                    new BufferedReader(
+                            new StringReader(
+                                    "user:bd94dcda26fccb4e68d6a31f9b5aac0b571ae266d822620e901ef7ebe3a11d4f"));
+            Mockito.when(fs.readFile(mockPath)).thenReturn(props);
+            // the subprotocol token part here should be "dXNlcjpwYXNzMTIzNA==", but the '='s are
+            // padding and stripped out. The decoder should treat these as optional, and the client
+            // is likely not to send them since they are not permitted by the WebSocket
+            // specification for the Sec-WebSocket-Protocol header
+            Assertions.assertTrue(
+                    mgr.validateWebSocketSubProtocol(
+                                    () -> "basic.authorization.containerjfr.dXNlcjpwYXNzMTIzNA")
+                            .get());
+        }
+
         @Test
         void shouldFailUnknownCredentials() throws Exception {
             Path mockPath = Mockito.mock(Path.class);
@@ -330,6 +381,7 @@ void shouldFailUnknownCredentials() throws Exception {
             Mockito.when(fs.exists(mockPath)).thenReturn(true);
             Mockito.when(fs.isRegularFile(mockPath)).thenReturn(true);
             Mockito.when(fs.isReadable(mockPath)).thenReturn(true);
+            // credentials: "user:pass"
             BufferedReader props =
                     new BufferedReader(
                             new StringReader(

diff --git a/src/test/java/itest/InterleavedExternalTargetRequestsIT.java b/src/test/java/itest/InterleavedExternalTargetRequestsIT.java
@@ -168,7 +168,7 @@ private void createInMemoryRecordings() throws Exception {
                                 .putHeader(
                                         "X-JMX-Authorization",
                                         "Basic "
-                                                + Base64.getEncoder()
+                                                + Base64.getUrlEncoder()
                                                         .encodeToString(
                                                                 "admin:adminpass123".getBytes()))
                                 .sendForm(
@@ -198,7 +198,7 @@ private void verifyInMemoryRecordingsCreated() throws Exception {
                     .putHeader(
                             "X-JMX-Authorization",
                             "Basic "
-                                    + Base64.getEncoder()
+                                    + Base64.getUrlEncoder()
                                             .encodeToString("admin:adminpass123".getBytes()))
                     .send(
                             ar -> {
@@ -242,7 +242,7 @@ private void deleteInMemoryRecordings() throws Exception {
                                 .putHeader(
                                         "X-JMX-Authorization",
                                         "Basic "
-                                                + Base64.getEncoder()
+                                                + Base64.getUrlEncoder()
                                                         .encodeToString(
                                                                 "admin:adminpass123".getBytes()))
                                 .sendForm(
@@ -272,7 +272,7 @@ private void verifyInmemoryRecordingsDeleted() throws Exception {
                     .putHeader(
                             "X-JMX-Authorization",
                             "Basic "
-                                    + Base64.getEncoder()
+                                    + Base64.getUrlEncoder()
                                             .encodeToString("admin:adminpass123".getBytes()))
                     .send(
                             ar -> {

diff --git a/web-client b/web-client