bpontarelli/tls fixes (#17)

Fixing TLS issues with a rewrite of the SSLEngine code. This now breaks apart handshaking and body processing into separate steps and using different buffers for consistency and clarity. This still doesn't handle mid-stream cipher renegotiations, but it works for most request/response transactions that are not large.
FusionAuth · Feb 7, 2024 · cfa80b6 · cfa80b6
1 parent a8fb48b
commit cfa80b6
Show file tree

Hide file tree

Showing 12 changed files with 571 additions and 267 deletions.
diff --git a/build.savant b/build.savant
@@ -62,12 +62,9 @@ release = loadPlugin(id: "org.savantbuild.plugin:release-git:2.0.0-RC.6")
 pom = loadPlugin(id: "org.savantbuild.plugin:pom:2.0.0-RC.6")
 
 java.settings.javaVersion = "17"
-java
-    .settings
-    .compilerArguments = "--add-exports java.base/sun.security.x509=ALL-UNNAMED --add-exports java.base/sun.security.util=ALL-UNNAMED -XDignore.symbol.file"
+java.settings.compilerArguments = "--add-exports java.base/sun.security.x509=ALL-UNNAMED --add-exports java.base/sun.security.util=ALL-UNNAMED -XDignore.symbol.file"
 javaTestNG.settings.javaVersion = "17"
-javaTestNG
-    .settings.jvmArguments = "--add-exports java.base/sun.security.x509=ALL-UNNAMED --add-exports java.base/sun.security.util=ALL-UNNAMED"
+javaTestNG.settings.jvmArguments = "--add-exports java.base/sun.security.x509=ALL-UNNAMED --add-exports java.base/sun.security.util=ALL-UNNAMED"
 javaTestNG.settings.testngArguments = "-listener io.fusionauth.http.BaseTest\$TestListener"
 
 target(name: "clean", description: "Cleans the build directory") {

diff --git a/java-http.ipr b/java-http.ipr
@@ -1385,6 +1385,31 @@
   <component name="ProjectRootManager" version="2" languageLevel="JDK_17" default="true" project-jdk-name="Java 17" project-jdk-type="JavaSDK">
     <output url="file://$PROJECT_DIR$/out" />
   </component>
+  <component name="ProjectRunConfigurationManager">
+    <configuration default="true" type="TestNG">
+      <shortenClasspath name="NONE" />
+      <useClassPathOnly />
+      <option name="SUITE_NAME" value="" />
+      <option name="PACKAGE_NAME" value="" />
+      <option name="MAIN_CLASS_NAME" value="" />
+      <option name="GROUP_NAME" value="" />
+      <option name="TEST_OBJECT" value="CLASS" />
+      <option name="VM_PARAMETERS" value="-ea --add-exports java.base/sun.security.x509=ALL-UNNAMED --add-exports java.base/sun.security.util=ALL-UNNAMED" />
+      <option name="PARAMETERS" value="" />
+      <option name="OUTPUT_DIRECTORY" value="" />
+      <option name="TEST_SEARCH_SCOPE">
+        <value defaultName="moduleWithDependencies" />
+      </option>
+      <option name="PROPERTIES_FILE" value="" />
+      <properties />
+      <listeners>
+        <listener class="io.fusionauth.http.BaseTest$TestListener" />
+      </listeners>
+      <method v="2">
+        <option name="Make" enabled="true" />
+      </method>
+    </configuration>
+  </component>
   <component name="VcsDirectoryMappings">
     <mapping directory="$PROJECT_DIR$" vcs="Git" />
   </component>

diff --git a/src/main/java/io/fusionauth/http/log/AccumulatingLogger.java b/src/main/java/io/fusionauth/http/log/AccumulatingLogger.java
@@ -27,7 +27,7 @@
 public class AccumulatingLogger extends BaseLogger {
   private final List<String> messages = new ArrayList<>();
 
-  public void reset() {
+  public synchronized void reset() {
     messages.clear();
   }
 
@@ -37,7 +37,7 @@ public String toString() {
   }
 
   @Override
-  protected void handleMessage(String message) {
+  protected synchronized void handleMessage(String message) {
     messages.add(message);
   }
 }
diff --git a/src/main/java/io/fusionauth/http/server/HTTP11Processor.java b/src/main/java/io/fusionauth/http/server/HTTP11Processor.java
@@ -181,6 +181,7 @@ public ProcessorState read(ByteBuffer buffer) throws IOException {
       state = ProcessorState.Write;
     }
 
+    logger.trace("(RR)");
     return state;
   }
 

diff --git a/src/main/java/io/fusionauth/http/server/HTTPRequestProcessor.java b/src/main/java/io/fusionauth/http/server/HTTPRequestProcessor.java
@@ -66,6 +66,7 @@ public ByteBuffer bodyBuffer() {
 
   public RequestState processBodyBytes() {
     bodyProcessor.processBuffer(inputStream);
+    logger.trace("(BODY) {} {}", bodyProcessor.currentBuffer(), bodyProcessor.totalBytesProcessed());
 
     if (bodyProcessor.isComplete()) {
       inputStream.signalDone();
@@ -115,7 +116,7 @@ public RequestState processPreambleBytes(ByteBuffer buffer) {
 
           int size = Math.max(buffer.remaining(), bufferSize);
           if (contentLength != null) {
-            logger.debug("Handling body using Content-Length header");
+            logger.debug("Handling body using Content-Length header {}", contentLength);
             bodyProcessor = new ContentLengthBodyProcessor(size, contentLength);
           } else {
             logger.debug("Handling body using Chunked data");

diff --git a/src/main/java/io/fusionauth/http/server/HTTPResponseProcessor.java b/src/main/java/io/fusionauth/http/server/HTTPResponseProcessor.java
@@ -70,7 +70,7 @@ public synchronized ByteBuffer[] currentBuffer() {
 
       // Construct the preamble if needed and return it if there is any bytes left
       if (preambleBuffers == null) {
-        logger.debug("The worker thread has bytes to write or has closed the stream, but the preamble hasn't been sent yet. Generating preamble");
+        logger.debug("The server (via a worker thread or the server due to an Expect request) has bytes to write or has closed the stream, but the preamble hasn't been sent yet. Generating preamble");
         int maxHeadLength = configuration.getMaxHeadLength();
         if (state == ResponseState.Preamble) {
           fillInHeaders();