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 temp directory used while resolving spring config #11632

Open
ovidijusnortal opened this issue Apr 8, 2024 · 6 comments
Open

Jetty temp directory used while resolving spring config #11632

ovidijusnortal opened this issue Apr 8, 2024 · 6 comments
Assignees
Labels

Comments

@ovidijusnortal
Copy link

Jetty Version
12.0.8

Jetty Environment
Spring Boot 3.2.4

Java Version
17

Question
After upgrading to jetty 12.0.8 we have an issue: when spring tries to resolve configuration DTO with field like this private Path apiTrustStore; instead of using value as is in our case /etc/xroad/ssl/center-admin-service.p12 we get this: /tmp/jetty-docbase.8085.8617281078912905990/etc/xroad/ssl/center-admin-service.p12.
Is this expected behavior? if yes is there a way to disable it?

@gregw
Copy link
Contributor

gregw commented Apr 8, 2024

It is not expected behaviour. We made some changes in #11568 to fix some issues with relative resource resolution. However, it looks like we have broken absolute resource resolution at the same time?

@joakime @janbartel @lorban Actually, whilst I think this is broken, I'm not sure how any of the changes in 12.0.8 could have affected it as they were about resources, whilst I see that SslConfiguration is doing direct Path manipulation to resolve the configuration:

    public static String resolvePath(String dir, String destPath)
    {
        if (StringUtil.isEmpty(dir) || StringUtil.isEmpty(destPath))
            return null;

        return Paths.get(dir).resolve(destPath).normalize().toString();
    }

Firstly, I think this is another mistaken use of Path in jetty-12 rather than just using the Resource APIs as intended....
but then I thought Path.resolve correctly handles absolute destinations.
Perhaps this is windows not see '/' as a path separator? @ovidijusnortal is this on windows or unix?

Note that this is invoked by the XML in jetty-ssl-context.xml:

        <Set name="TrustStorePath">
          <Call name="resolvePath" class="org.eclipse.jetty.xml.XmlConfiguration">
            <Arg><Property name="jetty.base"/></Arg>
            <Arg><Property name="jetty.sslContext.trustStorePath" deprecated="jetty.sslContext.trustStoreAbsolutePath,jetty.truststore" /></Arg>
          </Call>
        </Set>

So the invocation of the resolvePath method can be replaced in this XML to work around this issue. Or the setTrustStoreResource method used instead.

But let's investigate a bit more to see exactly what changed before we say exactly how to work around it.

@ovidijusnortal
Copy link
Author

@gregw found this on unix

@joakime
Copy link
Contributor

joakime commented Apr 9, 2024

@gregw I think that what you are referring to and what @ovidijusnortal is referring to are different things.

The OP is having issues with his project at https://github.com/nordic-institute/X-Road/

This was first noticed by OP's project when going from 12.0.6 to 12.0.8

The use of apiTrustStore in the OPs project is at ...

The apiTrustStore he has gets eventually loaded into something SSL related at ...

https://github.com/nordic-institute/X-Road/blob/develop/src/central-server/admin-service/api-client/src/main/java/org/niis/xroad/cs/admin/client/configuration/AdminServiceClientConfiguration.java#L117

Which is eventually used in a class org.apache.hc.core5.ssl.SSLContexts. (but by this point in time the apiTrustStore field is already prepared, and set by Spring configuration)

This doesn't seem to be the path that is of concern.

The OP shows us the full (presumably bad) path of /tmp/jetty-docbase.8085.8617281078912905990/etc/xroad/ssl/center-admin-service.p12

If we check the configuration of center-admin-service.p12 that's coming from something else.

https://github.com/nordic-institute/X-Road/blob/develop/src/central-server/admin-service/application/src/main/resources/application.yml#L36-L51

server:
  port: 4000
  ssl:
    key-store: /etc/xroad/ssl/center-admin-service.p12
    key-store-password: center-admin-service
    enabled: true
    ciphers: TLS_AES_128_GCM_SHA256,TLS_AES_256_GCM_SHA384,TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256,TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384,TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256,TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384
    protocol: TLS
    enabled-protocols: TLSv1.2,TLSv1.3
  compression:
    enabled: true
    mime-types: application/json,application/xml,text/html,text/xml,text/plain,application/javascript,text/css,image/jpeg
  servlet:
    session:
      cookie:
        same-site: Strict

That looks like standard spring-boot configuration for a server.

The project seems to be using spring-boot version 3.2.3 (at the time of the testing of that dependabot update from 12.0.6 to 12.0.8)

Looking at spring-boot project, they have not yet updated / tested on Jetty 12.0.8

@ovidijusnortal
Copy link
Author

same with spring boot 3.2.4 version. Spring boot right now is on 12.0.7 if I'm correct.
If I try to fetch that configuration value as a String then return value is /etc/xroad/ssl/center-admin-service.p12 as expected but when some spring magic is applied to make it as Path object then this /tmp/jetty-docbase.8085.8617281078912905990/etc/xroad/ssl/center-admin-service.p12 is result.

@joakime
Copy link
Contributor

joakime commented Apr 9, 2024

Of note, this behavior has been reported to spring-boot before.

Judging from the comments in those issues, it looks like if you specify the configuration using URI syntax it will work as intended ...

server:
  port: 4000
  ssl:
    key-store: file:/etc/xroad/ssl/center-admin-service.p12

@janbartel
Copy link
Contributor

@joakime @ovidijusnortal so it appears that this is a Spring issue, not a Jetty issue, thus can we close this issue?

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

No branches or pull requests

5 participants