Skip to content

[DO NOT MERGE] Test with trusting self-signed certificates #5946

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.

Sign up for GitHub

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

Jump to bottom
Draft
chaoran-chen wants to merge 8 commits into
base: main
Choose a base branch
from
Draft

[DO NOT MERGE] Test with trusting self-signed certificates #5946

chaoran-chen wants to merge 8 commits into from

Conversation

@chaoran-chen
Copy link
Member

@chaoran-chen chaoran-chen commented Feb 5, 2026 •
edited by loculus-bot
Loading

resolves #

Screenshot

PR Checklist

  • All necessary documentation has been adapted.
  • The implemented feature is covered by appropriate, automated tests.
  • Any manual testing that has been done is documented (i.e. what exactly was tested?)

🚀 Preview: Add preview label to enable

@claude claude bot added the backend related to the loculus backend component label Feb 5, 2026
github-advanced-security[bot]
github-advanced-security bot found potential problems Feb 5, 2026
View reviewed changes
backend/src/main/kotlin/org/loculus/backend/service/files/S3Service.kt
)

val sslContext = SSLContext.getInstance("TLS")
sslContext.init(null, trustAllCerts, java.security.SecureRandom())

Check failure

Code scanning / CodeQL

`TrustManager` that accepts all certificates High

This uses
TrustManager
Loading
, which is defined in
S3Service$
Loading
and trusts any certificate.

Copilot Autofix

AI 4 days ago

In general, the right fix is to stop using a TrustManager that blindly trusts all certificates and to stop disabling hostname verification. Instead, rely on the default JVM trust store or explicitly configure a KeyStore/trust store containing only the certificates (or CAs) you want to trust, and use a standard TrustManagerFactory and default hostname verification.

In this specific file, the minimal, safe change that preserves existing functionality as much as possible is:

  1. Remove the custom X509TrustManager that accepts all certificates.
  2. Stop creating a custom SSLContext and SSLConnectionSocketFactory with NoopHostnameVerifier.
  3. Let the AWS SDK’s ApacheHttpClient builder create a default HTTP client (ApacheHttpClient.builder().build()), which uses the platform’s default trust configuration and proper hostname verification.

If the application truly must talk to a service with a self‑signed certificate, the correct solution would be to configure the JVM’s trust store or an Apache HTTP client trust store with that specific certificate; however, that setup lies outside the shown snippet and cannot be implemented safely here without additional configuration details. Therefore, within backend/src/main/kotlin/org/loculus/backend/service/files/S3Service.kt, the best contained fix is to simplify createTrustAllHttpClient() so it no longer disables TLS checks, and to remove now‑unused TLS‑related imports.

Concretely:

  • In S3Service.kt, update createTrustAllHttpClient() (lines 228–245) to:
private fun createTrustAllHttpClient(): SdkHttpClient {
    return ApacheHttpClient.builder().build()
}
  • Remove the unused imports related to the insecure TLS customization: NoopHostnameVerifier, SSLConnectionSocketFactory, X509Certificate, SSLContext, TrustManager, and X509TrustManager.

No other behavior of S3Client construction (endpoint override, region, credentials, S3 configuration) is changed.

Suggested changeset 1
backend/src/main/kotlin/org/loculus/backend/service/files/S3Service.kt

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/backend/src/main/kotlin/org/loculus/backend/service/files/S3Service.kt b/backend/src/main/kotlin/org/loculus/backend/service/files/S3Service.kt
--- a/backend/src/main/kotlin/org/loculus/backend/service/files/S3Service.kt
+++ b/backend/src/main/kotlin/org/loculus/backend/service/files/S3Service.kt
@@ -1,7 +1,5 @@
 package org.loculus.backend.service.files
 
-import org.apache.http.conn.ssl.NoopHostnameVerifier
-import org.apache.http.conn.ssl.SSLConnectionSocketFactory
 import org.loculus.backend.config.S3BucketConfig
 import org.loculus.backend.config.S3Config
 import org.loculus.backend.controller.BadRequestException
@@ -32,11 +30,7 @@
 import software.amazon.awssdk.services.s3.presigner.model.PutObjectPresignRequest
 import software.amazon.awssdk.services.s3.presigner.model.UploadPartPresignRequest
 import java.net.URI
-import java.security.cert.X509Certificate
 import java.time.Duration
-import javax.net.ssl.SSLContext
-import javax.net.ssl.TrustManager
-import javax.net.ssl.X509TrustManager
 
 private const val PRESIGNED_URL_EXPIRY_SECONDS = 60 * 30
 
@@ -226,21 +219,7 @@
         .build()
 
     private fun createTrustAllHttpClient(): SdkHttpClient {
-        val trustAllCerts = arrayOf<TrustManager>(
-            object : X509TrustManager {
-                override fun checkClientTrusted(chain: Array<out X509Certificate>?, authType: String?) {}
-                override fun checkServerTrusted(chain: Array<out X509Certificate>?, authType: String?) {}
-                override fun getAcceptedIssuers(): Array<X509Certificate> = arrayOf()
-            },
-        )
-
-        val sslContext = SSLContext.getInstance("TLS")
-        sslContext.init(null, trustAllCerts, java.security.SecureRandom())
-
-        val socketFactory = SSLConnectionSocketFactory(sslContext, NoopHostnameVerifier.INSTANCE)
-
         return ApacheHttpClient.builder()
-            .socketFactory(socketFactory)
             .build()
     }
 
EOF
@@ -1,7 +1,5 @@
package org.loculus.backend.service.files

import org.apache.http.conn.ssl.NoopHostnameVerifier
import org.apache.http.conn.ssl.SSLConnectionSocketFactory
import org.loculus.backend.config.S3BucketConfig
import org.loculus.backend.config.S3Config
import org.loculus.backend.controller.BadRequestException
@@ -32,11 +30,7 @@
import software.amazon.awssdk.services.s3.presigner.model.PutObjectPresignRequest
import software.amazon.awssdk.services.s3.presigner.model.UploadPartPresignRequest
import java.net.URI
import java.security.cert.X509Certificate
import java.time.Duration
import javax.net.ssl.SSLContext
import javax.net.ssl.TrustManager
import javax.net.ssl.X509TrustManager

private const val PRESIGNED_URL_EXPIRY_SECONDS = 60 * 30

@@ -226,21 +219,7 @@
.build()

private fun createTrustAllHttpClient(): SdkHttpClient {
val trustAllCerts = arrayOf<TrustManager>(
object : X509TrustManager {
override fun checkClientTrusted(chain: Array<out X509Certificate>?, authType: String?) {}
override fun checkServerTrusted(chain: Array<out X509Certificate>?, authType: String?) {}
override fun getAcceptedIssuers(): Array<X509Certificate> = arrayOf()
},
)

val sslContext = SSLContext.getInstance("TLS")
sslContext.init(null, trustAllCerts, java.security.SecureRandom())

val socketFactory = SSLConnectionSocketFactory(sslContext, NoopHostnameVerifier.INSTANCE)

return ApacheHttpClient.builder()
.socketFactory(socketFactory)
.build()
}

Copilot is powered by AI and may make mistakes. Always verify output.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

backend related to the loculus backend component

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@chaoran-chen