Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[UNDERTOW-2033] Derrive SecurePredicate from connection encryption, r… #1587

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

baranowb
Copy link
Contributor

@baranowb baranowb added under verification Currently being verified (running tests, reviewing) before posting a review to contributor waiting CI check Ready to be merged but waiting for CI check waiting peer review PRs that edit core classes might require an extra review labels Apr 26, 2024
@freedge
Copy link

freedge commented May 4, 2024

this is how I tested this

Needed this to run the tests:

diff --git a/examples/src/main/java/io/undertow/examples/http2/Http2Server.java b/examples/src/main/java/io/undertow/examples/http2/Http2Server.java
index 4ac9b5e..79a2989 100644
--- a/examples/src/main/java/io/undertow/examples/http2/Http2Server.java
+++ b/examples/src/main/java/io/undertow/examples/http2/Http2Server.java
@@ -65,7 +65,7 @@ public class Http2Server {
     public static void main(final String[] args) throws Exception {
         String version = System.getProperty("java.version");
         System.out.println("Java version " + version);
-        if(version.charAt(0) == '1' && Integer.parseInt(version.charAt(2) + "") < 8 ) {
+        if(version.charAt(0) == '1' && version.charAt(1) == '.' && Integer.parseInt(version.charAt(2) + "") < 8 ) {
             System.out.println("This example requires Java 1.8 or later");
             System.out.println("The HTTP2 spec requires certain cyphers that are not present in older JVM's");
             System.out.println("See section 9.2.2 of the HTTP2 specification for details");
diff --git a/pom.xml b/pom.xml
index 36bb250..4ed6370 100644
--- a/pom.xml
+++ b/pom.xml
@@ -111,6 +111,8 @@
         <maven.javadoc.plugin.quiet>true</maven.javadoc.plugin.quiet>
         <maven.compiler.showDeprecation>false</maven.compiler.showDeprecation>
         <maven.compiler.showWarnings>false</maven.compiler.showWarnings>
+
+       <version.compiler.plugin>3.13.0</version.compiler.plugin>
     </properties>

     <modules>

I also prepared a new certificate as the one bundled here expired

#!/bin/bash

set -e
set -x
rm -f server.keystore client.keystore server.truststore client.truststore server.key server.p12 server.pem
openssl req -x509 -newkey ec -pkeyopt ec_paramgen_curve:secp384r1 -days 3650 \
    -nodes -keyout server.key -out server.pem -subj "/CN=localhost" \
    -addext "subjectAltName=DNS:localhost"
openssl pkcs12 -export -in server.pem -inkey server.key -name server -out server.p12 -passout file:<(echo pass)

keytool -destkeystore server.keystore -deststorepass:file <(echo password) -importkeystore -noprompt -srckeystore server.p12 -srcstorepass:file <(echo pass)
keytool -keystore client.keystore -storepass:file <(echo password) -importcert -noprompt -file server.pem
keytool -keystore server.truststore -storepass:file <(echo password) -importcert -noprompt -file server.pem
keytool -keystore client.truststore -storepass:file <(echo password) -importcert -noprompt -file server.pem
rm -f server.p12 server.key

With a recent version of curl you can actually provide the scheme through a header argument, no need for the haproxy headache.

Without the patch:

$ build/bin/curl https://localhost:8443 --cacert ~/GIT/undertow/examples/src/main/java/io/undertow/examples/http2/server.pem  -s -o /dev/null -w "%{http_code}\n"
200
$ build/bin/curl https://localhost:8443 --cacert ~/GIT/undertow/examples/src/main/java/io/undertow/examples/http2/server.pem  -s -o /dev/null -w "%{http_code}\n" -H ':scheme: http'
307
$ build/bin/curl http://localhost:8080 --http2-prior-knowledge  -s -o /dev/null -w "%{http_code}\n"
307
$ build/bin/curl http://localhost:8080 --http2-prior-knowledge  -s -o /dev/null -w "%{http_code}\n" -H ':scheme: https'
200

Using this patch

$ build/bin/curl https://localhost:8443 --cacert ~/GIT/undertow/examples/src/main/java/io/undertow/examples/http2/server.pem  -s -o /dev/null -w "%{http_code}\n"
200
$ build/bin/curl https://localhost:8443 --cacert ~/GIT/undertow/examples/src/main/java/io/undertow/examples/http2/server.pem  -s -o /dev/null -w "%{http_code}\n" -H ':scheme: http'
200
$ build/bin/curl http://localhost:8080 --http2-prior-knowledge  -s -o /dev/null -w "%{http_code}\n"
307
$ build/bin/curl http://localhost:8080 --http2-prior-knowledge  -s -o /dev/null -w "%{http_code}\n" -H ':scheme: https'
307
$

It fixes the issue indeed, thanks!

@baranowb baranowb requested a review from fl4via May 6, 2024 06:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
under verification Currently being verified (running tests, reviewing) before posting a review to contributor waiting CI check Ready to be merged but waiting for CI check waiting peer review PRs that edit core classes might require an extra review
Projects
None yet
2 participants