-
Notifications
You must be signed in to change notification settings - Fork 186
[FEATURE] Add an option to turn on and off the certificate validation of llm connectors #4394
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
base: main
Are you sure you want to change the base?
Changes from 1 commit
2f5e82f
3027319
564d401
de4ce11
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -82,6 +82,9 @@ public class AwsConnectorExecutor extends AbstractConnectorExecutor { | |
| @Setter | ||
| private boolean connectorPrivateIpEnabled; | ||
|
|
||
| @Setter | ||
| private boolean connectorSslVerificationEnabled; | ||
|
|
||
| public AwsConnectorExecutor(Connector connector) { | ||
| super.initialize(connector); | ||
| this.connector = (AwsConnector) connector; | ||
|
|
@@ -193,7 +196,7 @@ protected SdkAsyncHttpClient getHttpClient() { | |
| this.httpClientRef | ||
| .compareAndSet( | ||
| null, | ||
| MLHttpClientFactory.getAsyncHttpClient(connectionTimeout, readTimeout, maxConnection, connectorPrivateIpEnabled) | ||
| MLHttpClientFactory.getAsyncHttpClient(connectionTimeout, readTimeout, maxConnection, connectorPrivateIpEnabled, connectorSslVerificationEnabled) | ||
| ); | ||
|
Comment on lines
197
to
202
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fix spotless formatting violation. The pipeline indicates a formatting violation. Run the following command to fix: ./gradlew :opensearch-ml-algorithms:spotlessApply🧰 Tools🪛 GitHub Actions: Build and Test ml-commons[error] 196-196: spotlessJavaCheck failed: formatting violations detected in AwsConnectorExecutor.java. Run './gradlew :opensearch-ml-algorithms:spotlessApply' to fix. 🤖 Prompt for AI Agents |
||
| } | ||
| return httpClientRef.get(); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -53,6 +53,7 @@ public class RemoteModel implements Predictable { | |
| public static final String USER_RATE_LIMITER_MAP = "user_rate_limiter_map"; | ||
| public static final String GUARDRAILS = "guardrails"; | ||
| public static final String CONNECTOR_PRIVATE_IP_ENABLED = "connectorPrivateIpEnabled"; | ||
| public static final String CONNECTOR_SSL_VERIFICATION_ENABLED = "connectorSslVerificationEnabled"; | ||
|
||
| public static final String SDK_CLIENT = "sdk_client"; | ||
| public static final String SETTINGS = "settings"; | ||
|
|
||
|
|
@@ -127,6 +128,7 @@ public CompletionStage<Boolean> initModelAsync(MLModel model, Map<String, Object | |
| this.connectorExecutor.setUserRateLimiterMap((Map<String, TokenBucket>) params.get(USER_RATE_LIMITER_MAP)); | ||
| this.connectorExecutor.setMlGuard((MLGuard) params.get(GUARDRAILS)); | ||
| this.connectorExecutor.setConnectorPrivateIpEnabled((boolean) params.getOrDefault(CONNECTOR_PRIVATE_IP_ENABLED, false)); | ||
| this.connectorExecutor.setConnectorSslVerificationEnabled((boolean) params.getOrDefault(CONNECTOR_SSL_VERIFICATION_ENABLED, true)); | ||
| return CompletableFuture.completedStage(true); | ||
| }).exceptionally(e -> { | ||
| log.error("Failed to init remote model.", e); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using TRUST_ALL_CERTIFICATES creates significant security exposure.
Setting
TRUST_ALL_CERTIFICATEStotruecompletely disables SSL/TLS certificate validation, making the connection vulnerable to man-in-the-middle attacks. While this feature was explicitly requested for private network scenarios, operators should be clearly warned of the security implications.Consider adding:
return doPrivileged(() -> { - log - .debug( - "Creating MLHttpClient with connectionTimeout: {}, readTimeout: {}, maxConnections: {}", + if (!connectorSslVerificationEnabled) { + log.warn( + "SSL certificate verification is DISABLED. This connection is vulnerable to man-in-the-middle attacks. " + + "Only use this setting in trusted environments." + ); + } + log + .debug( + "Creating MLHttpClient with connectionTimeout: {}, readTimeout: {}, maxConnections: {}, sslVerificationEnabled: {}", connectionTimeout, readTimeout, - maxConnections + maxConnections, + connectorSslVerificationEnabled );🤖 Prompt for AI Agents