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

endpointIdentificationAlgorithm enabled by default in SSLEngine #4726

Closed
andy-xian-hai opened this issue Mar 27, 2020 · 2 comments
Closed

endpointIdentificationAlgorithm enabled by default in SSLEngine #4726

andy-xian-hai opened this issue Mar 27, 2020 · 2 comments

Comments

@andy-xian-hai
Copy link

andy-xian-hai commented Mar 27, 2020

Jetty version
jetty-9.4.27.v20200227
Java version
java 1.8
OS type/version
Windows
Description
In commit e4d7860, the default _endpointIdentificationAlgorithm changed from null to HTTPS, this can break existing application code.

I know issue #3464 tries to solve this case. However, I find it misses following case. I will just past the stack trace here for your reference:

Thread [main] (Suspended (breakpoint at line 1958 in SslContextFactory))	
	owns: Object  (id=2995)	
	owns: Object  (id=2996)	
	owns: Object  (id=2997)	
	owns: Object  (id=2998)	
	SslContextFactory.customize(SSLParameters) line: 1958	
	SslContextFactory.customize(SSLEngine) line: 1947	
	SslContextFactory.checkConfiguration() line: 253	
	SslContextFactory.doStart() line: 247	
	SslContextFactory(AbstractLifeCycle).start() line: 72	
	SslConnectionFactory(ContainerLifeCycle).start(LifeCycle) line: 169	
	SslConnectionFactory(ContainerLifeCycle).doStart() line: 117	
	SslConnectionFactory.doStart() line: 97	
	SslConnectionFactory(AbstractLifeCycle).start() line: 72	
	WebServiceHttpsConnector(ContainerLifeCycle).start(LifeCycle) line: 169	
	WebServiceHttpsConnector(ContainerLifeCycle).doStart() line: 117	
	WebServiceHttpsConnector(AbstractConnector).doStart() line: 321	
	WebServiceHttpsConnector(AbstractNetworkConnector).doStart() line: 81	
	WebServiceHttpsConnector(ServerConnector).doStart() line: 231	
	WebServiceHttpsConnector(AbstractLifeCycle).start() line: 72	
	Server.doStart() line: 385	
	Server(AbstractLifeCycle).start() line: 72	
	MockWebServer.<init>(int, String, MockWebServer$HostingOptions, SchemaDirectory, String, String) line: 281	

From the stack trace, I have two questions:

  1. Is the member initialization: "private String _endpointIdentificationAlgorithm = "HTTPS";" necessary? There are actually three code paths for _endpointIdentificationAlgorithm:

    1. Server mode, _endpointIdentificationAlgorithm would be HTTPS.
    2. Client mode, _endpointIdentificationAlgorithm would be null.
    3. For all other cases, _endpointIdentificationAlgorithm should be null in my opinion.
  2. Can you provide a way to skip customize method like "SslContextFactory.customize(SSLEngine) line: 1947". " customize method does not respect sslParameter value set in application layer and just replace those parameters silently. This really surprises application deliberately set those values by themselves because their settings are ignored. It would be nice to have a way not to set those values.

@joakime
Copy link
Contributor

joakime commented Mar 27, 2020

Sounds like WebServiceHttpsConnector isn't updated to use SslContextFactory$Server.

The baseline SslContextFactory (not Server specific, not Client specific) can only be neutral (support both server and client) and secure (industry recommended security configuration is default).
Adjusting the baseline SslContextFactory to support specifically either Server or Client will break the other sides usage.

Historically, before both the linked issue #3464 and commit e4d7860 (that you mentioned in this issue), we used to have client specific code where the client was initialized (commit ce6bc23) , but that broke other users.

Note that in Jetty 10.0.0 the baseline SslContextFactory is abstract and cannot be instantiated.
https://github.com/eclipse/jetty.project/blob/jetty-10.0.0.alpha2/jetty-util/src/main/java/org/eclipse/jetty/util/ssl/SslContextFactory.java#L103

The only safe choice is to have your WebServiceHttpsConnector updated to use specifically the SslContextFactory$Server implementation.

@andy-xian-hai
Copy link
Author

Tks. It solves my problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants