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

Jetty ssl keystorePath doesn't work with absolute path #5689

Closed
shimon-cherny opened this issue Nov 19, 2020 · 11 comments · Fixed by #5867
Closed

Jetty ssl keystorePath doesn't work with absolute path #5689

shimon-cherny opened this issue Nov 19, 2020 · 11 comments · Fixed by #5867
Assignees

Comments

@shimon-cherny
Copy link

Hi,

After upgrading jetty from 9.4.24 to 9.4.34 i have noticed that my server failed to start if this property at jetty-ssl-context.xml contained absolute path.
and only works with relative path.

Jetty fails to start as it appends a prefix of base path to the configuration and fails.

@sbordet
Copy link
Contributor

sbordet commented Nov 19, 2020

@shimon-cherny the jetty.sslContext.keyStorePath property is relative $JETTY_BASE and it has been so for all the Jetty 9.4.x series.

Perhaps there is something else that is making your test fail to start?

Can you detail the error, stack trace, etc.?

@sbordet
Copy link
Contributor

sbordet commented Nov 19, 2020

@gregw what do you think of changing this for 10?

We can support maybe expansion of properties.

From:

<Set name="KeyStorePath"><Property name="jetty.base" default="." />/<Property name="jetty.sslContext.keyStorePath" default="etc/keystore.p12" /></Set>

to:

<Set name="KeyStorePath"><Property name="jetty.sslContext.keyStorePath" default="${jetty.base}/etc/keystore.p12" /></Set>

Thoughts?

@gregw
Copy link
Contributor

gregw commented Nov 20, 2020

I think we need to protect backwards compatibility so we'd need a new property... either for the directory or for the full path:

<Set name="KeyStorePath">
 <Property name="jetty.sslContext.keyStorePathAbsolute">
  <default>
    <Property name="jetty.base" default="." />/<Property name="jetty.sslContext.keyStorePath" default="etc/keystore.p12" />
  </default>
 </Property>
</Set>

@shimon-cherny
Copy link
Author

This is the log from the previous jetty version (see that it works just fine). There is some change here....

2020/11/19 13:55:52 | 2020-11-19 13:55:52.631:INFO:oejus.SslContextFactory:WrapperSimpleAppMain: x509=X509@67724b(almoctane.net,h=[swinfra.net],w=[swinfra.net]) for Server@670025db[provider=null,keyStore=file:///opt/octane/conf/keystore.jks,trustStore=file:///opt/octane/conf/keystore.jks]
2020/11/19 13:55:52 | 2020-11-19 13:55:52.644:INFO:oejs.AbstractConnector:WrapperSimpleAppMain: Started ServerConnector@2d8666a2{SSL,[ssl, http/1.1]}{0.0.0.0:8443}
2020/11/19 13:55:52 | 2020-11-19 13:55:52.644:INFO:oejs.Server:WrapperSimpleAppMain: Started @76611ms
2020/11/19 13:55:52 |
2020/11/19 13:55:52 |
2020/11/19 13:55:52 | *************************************************************************
2020/11/19 13:55:52 | *************************************************************************
2020/11/19 13:55:52 | **
2020/11/19 13:55:52 | ** Server is ready! (Boot time 74 seconds)
2020/11/19 13:55:52 | **
2020/11/19 13:55:52 | *************************************************************************
2020/11/19 13:55:52 | *************************************************************************

[root@188e391ae721 octane]# grep store /opt/octane/server/conf/jetty-ssl-context.xml

<Set name="KeyStorePath"><Property name="jetty.base" default="." />/<Property name="jetty.sslContext.keyStorePath" deprecated="jetty.keystore" default="/opt/octane/conf/keystore.jks"/></Set>

@joakime
Copy link
Contributor

joakime commented Nov 22, 2020

[root@188e391ae721 octane]# grep store /opt/octane/server/conf/jetty-ssl-context.xml

<Set name="KeyStorePath"><Property name="jetty.base" default="." />/<Property name="jetty.sslContext.keyStorePath" deprecated="jetty.keystore" default="/opt/octane/conf/keystore.jks"/></Set>

Wait. What?

What is this file?
That directory .../server/conf/jetty-ssl-context.xml is not a known location for finding XML files per the jetty start mechanism.

Also, why are you editing the XML files?? That's strongly discouraged.

2020/11/19 13:55:52 | 2020-11-19 13:55:52.644:INFO:oejs.Server:WrapperSimpleAppMain: Started @76611ms

Huh? This isn't the jetty start.jar mechanism for sure.
Yeah, all bets are off for the XML files then.

This is essentially a custom Embedded Jetty with custom XML files.

@shimon-cherny
Copy link
Author

shimon-cherny commented Nov 22, 2020

Not sure i understood your point.
Yes, we are using embedded jetty and starting it with tanuki wrapper and these configurations is updated on our application startup.

I am pretty sure in both versions the same file has been loaded in jetty. While at 9.4.24 it worked as is and at 9.4.34 it doesn't

Any thing i can do besides roll back to previous version or writing an upgrader for this configuration file?

@joakime
Copy link
Contributor

joakime commented Nov 22, 2020

The directory /conf/ is not a valid directory for Jetty start.jar.

Nothing in jetty's start.jar will look for content in a directory called /conf/.

The main class oejs.Server looks like the normal Jetty org.eclipse.jetty.server.Server, that's good.

The WrapperSimpleAppMain appears to be a thread name.
Is that what the tanuki wrapper does?

Now, why are you using jetty-ssl-context.xml from a /conf/ directory?

I suspect this is a tankui wrapper specific directory, if so, then you cannot directly reference the XML from jetty-home (or the older jetty-distribution) like that.

The XMLs have a strict order (depending on the modules you have enabled).
You should not reference a raw XML like that.
You should not edit an XML file.

To use the XML files from jetty-home (or the older jetty-distribution) they must be used properly for it to function properly.
Did you create a separate jetty.base directory?
Did you reference the modules you want enabled?
Did you configure those modules using only properties?
If you did this properly, you'll have a jetty.base directory with no (jetty default) XML files in it, suitable for running your instance with.

@shimon-cherny
Copy link
Author

shimon-cherny commented Nov 22, 2020

We are deploying jetty as separate directory called server which consists of 'conf' folder (where all jetty xml configurations files are being deployed) and 'lib' folder (where all jetty jars are being deployed).

Then we have a custom jetty launcher which configures jetty server to look into this conf directory and reads from there start.ini and configures jetty server to activate relevant modules.

So this file is being loaded for jetty. It simply stopped doing what it has been doing so far.

@gregw
Copy link
Contributor

gregw commented Nov 23, 2020

@shimon-cherny
As a work around, since you have edited the XML anyway, then you can just edit it to be:

<Set name="KeyStorePath">/opt/octane/conf/keystore.jks"</Set>

@joakime I don't think there is anything wrong with being able to configure absolute paths for such things which are outside of jetty-base and jetty-home. I have no idea how this could have worked in 9.4.24 as the XML is the same.... but regardless we should make sure we provide an option so it can be done.

@shimon-cherny As your startup does appear a little bit outside of norms, it makes it a bit harder for us to assist. It does look like you are partly using our start mechanism, but not fully. Ideally you would not have to edit the XML at all in a normal usage and you should strive to achieve that. However in this case, I don't think it would work... unless you can use a symlink from jetty-base to that keystore.

@shimon-cherny
Copy link
Author

Thank you for all the comments.

@gregw
So instead of setting properties

<Set name="KeyStorePath"><Property name="jetty.base" default="." /><Property name="jetty.sslContext.keyStorePath" deprecated="jetty.keystore" default="/opt/octane/conf/keystore.jks"/></Set>

Just put the configuration as a value like this?

<Set name="KeyStorePath">/opt/octane/conf/keystore.jks"</Set>

BTW This is what jetty throws now (same file - different jetty version) - it concatenates jetty base url to the keystore path.

2020/11/23 12:54:49 | WrapperSimpleApp Error: java.lang.IllegalStateException: /opt/octane/server/opt/octane/conf/keystore.jks is not a valid keystore
2020/11/23 12:54:49 | WrapperSimpleApp Error: at org.eclipse.jetty.util.security.CertificateUtils.getKeyStore(CertificateUtils.java:50)
2020/11/23 12:54:49 | WrapperSimpleApp Error: at org.eclipse.jetty.util.ssl.SslContextFactory.loadKeyStore(SslContextFactory.java:1197)
2020/11/23 12:54:49 | WrapperSimpleApp Error: at org.eclipse.jetty.util.ssl.SslContextFactory.load(SslContextFactory.java:321)
2020/11/23 12:54:49 | WrapperSimpleApp Error: at org.eclipse.jetty.util.ssl.SslContextFactory.doStart(SslContextFactory.java:243)
2020/11/23 12:54:49 | WrapperSimpleApp Error: at org.eclipse.jetty.util.component.AbstractLifeCycle.start(AbstractLifeCycle.java:73)
2020/11/23 12:54:49 | WrapperSimpleApp Error: at org.eclipse.jetty.util.component.ContainerLifeCycle.start(ContainerLifeCycle.java:169)
2

@sirmspencer
Copy link

You could also create a symlink to a location inside jetty.base

@sbordet sbordet added this to To do in Jetty 9.4.36 via automation Jan 8, 2021
@sbordet sbordet self-assigned this Jan 8, 2021
sbordet added a commit that referenced this issue Jan 12, 2021
…ute_path

Fixes #5689 - Jetty ssl keystorePath doesn't work with absolute path.
Jetty 9.4.36 automation moved this from To do to Done Jan 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

Successfully merging a pull request may close this issue.

5 participants