2 standalone servlet tests failing in jdk11

[alwin-joseph]

Describe the bug

2 standalone servlet TCK tests are failing when run in JDK11.
com/sun/ts/tests/servlet/spec/security/clientcert/Client.java#clientCertTest
com/sun/ts/tests/servlet/spec/security/clientcertanno/Client.java#clientCertTest

exception like
"[javatest.batch] 02-25-2021 08:22:59: ERROR: java.io.IOException: Server returned HTTP response code: 403 for URL: https://localhost:8181/clientcert_web/ServletSecTest
"

To Reproduce
Run the servlet tck with jdk11.
Download https://download.eclipse.org/ee4j/jakartaee-tck/jakartaee9-eftl/staged-910/jakarta-servlet-tck-5.0.0.zip
Use docker/servlettckrun.sh to run the tck tests.

servletlogs.txt

Complete test run logs at https://ci.eclipse.org/jakartaee-tck/blue/rest/organizations/jenkins/pipelines/jakartaee-tck/branches/master/runs/1243/nodes/81/steps/290/log/?start=0

[scottmarlow]

Test output is also copied here:

• tests/servlet/spec/security/clientcert/Client_clientCertTest.jtr
• tests/servlet/spec/security/clientcertanno/Client_clientCertTest.jtr

[markt-asf]

Strange. I've run the Servlet 5 TCK with Java 11 and Tomcat 10 in the past without any failures. I'll try and find some time to re-test with the latest TCK source.

[joakime]

Jetty 11 passes the Servlet 5 TCK with Java 11 as well.

The key to success for us is that the Server runs in Java 11, but the TCK runs in Java 8.

But Jetty only made the change to use Java 8 on the TCK as the HTTP2 test client is broken when run on Java 11.
This tree -> https://github.com/eclipse-ee4j/jakartaee-tck/tree/master/src/com/sun/ts/tests/servlet/spec/serverpush
That's the only thing that fails for us on when running the Servlet 5 TCK with Java 11.

[arjantijms]

Jetty 11 passes the Servlet 5 TCK with Java 11 as well. The key to success for us is that the Server runs in Java 11, but the TCK runs in Java 8.

If the client uses Java 8 and/or tls 1.2 everything is fine. If the client sends the certificate pre-emptively all is fine too. The problem is where the client does a request, the server detects client-cert authentication is needed, and then needs to ask for the client certificate. Maybe I’m missing something, but it seems truly bizar the protocol for asking this is not implemented in Java’s TLS 1.3.

But Jetty only made the change to use Java 8 on the TCK as the HTTP2 test client is broken when run on Java 11. This tree -> https://github.com/eclipse-ee4j/jakartaee-tck/tree/ master/src/com/sun/ts/tests/servlet/spec/serverpush That's the only thing that fails for us on when running the Servlet 5 TCK with Java 11.

Did you provide your own implementation of the TSHttpS-something protocol? I forgot the exact name, but the class that does the actual https request is pluggable in the TCK.

[arjantijms]

It looks like Tomcat has an explicit warning for this case:

org.apache.tomcat.util.net.SSLUtilBase : The JSSE TLS 1.3 implementation
does not support authentication after the initial handshake and is
therefore incompatible with optional client authentication

This SO question has some more info:
https://stackoverflow.com/questions/64182147/springboot-mvc-warning-org-apache-tomcat-util-net-sslutilbase-the-jsse-tls

[joakime]

Looking at the Jetty implementation, it seems that we rely on the Want/Need client auth SSL configurations, along with an javax.net.ssl.X509ExtendedTrustManager to allow us to handle Servlet Certificate/Auth Constraints when running the TCK on Java 11.

For us, only the HTTP/2 client behaviors are broken on Java 11, the rest of the TCK is a-ok.

[arjantijms]

The specific failure in the TCK seems to occur because of the difference in DN formatting between RFC 1779 and RFC 2253.

See https://eclipse.org/lists/glassfish-dev/msg00967.html

[scottmarlow]

I'm not sure if yesterdays servlet tck run included this fix or not so am running https://ci.eclipse.org/jakartaee-tck/job/9.1/job/eftl-standalonetck-build-run-910/25/ (https://ci.eclipse.org/jakartaee-tck/job/jakartaee-tck/job/master/1277/) for just the servlet tests to determine if this issue is addressed.

[scottmarlow]

https://ci.eclipse.org/jakartaee-tck/job/jakartaee-tck/job/master/1277/testReport/com.sun.ts.tests.servlet.spec.security.clientcert/Client/standalone_tck_run___servlet___clientCertTest/ still failed. I'll see if I can make any sense out of the https://ci.eclipse.org/jakartaee-tck/job/9.1/job/eftl-standalonetck-build-run-910/26/artifact/servlet-tck-results.tar.gz results.

com/sun/ts/tests/servlet/spec/security/clientcertanno/Client.java#clientCertTest also failed.

[arjantijms]

@scottmarlow Seemingly it didn't just fail again, but fails in a different way:

"java.io.IOException: Server returned HTTP response code: 400 for URL: "

In the previous run it was a 403. A 400 is sent when no certificates at all have been obtained, which is very likely the case here:

if (certs == null || certs.length < 1) {

    if (debug >= 1)
        log(rb.getString(NO_CERTIFICATE_INCLUDED_INFO));
            
    // BEGIN S1AS 4878272
    hres.sendError(SC_BAD_REQUEST);
    response.setDetailMessage(rb.getString(NO_CLIENT_CERTIFICATE_CHAIN));
    // END S1AS 4878272
       
    return false;
}

SC_BAD_REQUEST = 400.

I guess the TLSv1.2 setting somehow did not take effect, as if that would happen, this would be the exact error case.

[scottmarlow]

Hmm, I can try to build and test again just to verify that we are building with the change.

https://ci.eclipse.org/jakartaee-tck/job/9.1/job/eftl-standalonetck-build-run-910/30 will run the Standalone Server TCK.

[scottmarlow]

I searched for references to securedWebServicePort and a few other places:
src/com/sun/ts/lib/deliverable/jaspic/JaspicJakartaEEPropertyManager.java
src/com/sun/ts/lib/deliverable/jaspic/JaspicPropertyManager.java
src/com/sun/ts/lib/deliverable/cts/CTSPropertyManager.java
src/com/sun/ts/lib/harness/TSRuntimeConfiguration.java

Perhaps client.cert.test.jdk.tls.client.protocols needs to be added to ^

[scottmarlow]

I'll look a bit deeper into these property managers.

[scottmarlow]

It looks the Servlet
com.sun.ts.lib.deliverable.AbstractPropertyManager#getTestSpecificProperties that is used for the servlet tests needs the client.cert.test.jdk.tls.client.protocols property to be copied around into the test properties. I'm guessing the change to handle that belongs in com.sun.ts.lib.deliverable.servlet.ServletPropertyManager#getTestSpecificProperties + com.sun.ts.lib.deliverable.cts.CTSPropertyManager#getTestSpecificProperties.

@RohitKumarJain @gurunrao do you agree that for any added ts.jte properties for servlet testing, that we need to update CTSPropertyManager#getTestSpecificProperties + ServletPropertyManager#getTestSpecificProperties to ensure the added (client.cert.test.jdk.tls.client.protocols) property is available to the TCK test?

[arjantijms]

Perhaps client.cert.test.jdk.tls.client.protocols needs to be added to ^

Oh indeed. I noticed these, but hoped just adding to the .jte file was enough. Apparently this was not the case.

[RohitKumarJain]

It seems this issue is fixed and we don't see any failures for servlet tck. CI job reference can be found here: https://ci.eclipse.org/jakartaee-tck/job/9.1/job/eftl-standalonetck-build-run-910/35/

[hussainnm]

Changing the principal name to match the current format is making the test to fail in GF 6.0.

[gurunrao]

Failures are not see with 9.1 Paltform TCK + JDK11 + GF 6.1 latest run here - https://ci.eclipse.org/jakartaee-tck/job/jakartaee-tck/job/master/1314/#showFailuresLink

[arjantijms]

Changing the principal name to match the current format is making the test to fail in GF 6.0.

That's technically correct. The mapping file is specific for a server, and in this case specific for GF 6.1. So in its current configuration, the test can only be run on GF 6.1, not GF 6.0.

What should really be done is having a porting kit, so this file is not at all part of the TCK itself. In absence of this, a quick fix could be to introduce a second role, and then changing the security of the Servlet that's being tested to accept that one as well.

e.g.

<security-role-mapping>
     <role-name>AdministratorOld</role-name>
     <principal-name>CN=CTS, OU=Java Software, O=Sun Microsystems Inc., L=Burlington, ST=MA, C=US</principal-name>
</security-role-mapping>

WDYT?

[hussainnm]

I tried by adding another principal-name and it passed with GF 6.0

<sun-web-app>
    <context-root>clientcert_web</context-root>
    <security-role-mapping>
        <role-name>Administrator</role-name>
        <principal-name>CN=CTS,OU=Java Software,O=Sun Microsystems Inc.,L=Burlington,ST=MA,C=US</principal-name>
        <principal-name>CN=CTS, OU=Java Software, O=Sun Microsystems Inc., L=Burlington, ST=MA, C=US</principal-name>	
    </security-role-mapping>
</sun-web-app>

[arjantijms]

@hussainnm I was just about to respond with that ;) Indeed, we can just as well map several principals to that role. That's easier to change. Thanks!

(eventually that file should disappear from the TCK, but for now your proposal is the best)