-
Notifications
You must be signed in to change notification settings - Fork 5
DSP-24887: Enable SAN-aware peer identity lookup for endpoint verification on hostname #40
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: dse-netty-4.1.128.2
Are you sure you want to change the base?
Changes from all commits
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 |
|---|---|---|
|
|
@@ -73,7 +73,8 @@ public final class ReferenceCountedOpenSslClientContext extends ReferenceCounted | |
| try { | ||
| sessionContext = newSessionContext(this, ctx, engineMap, trustCertCollection, trustManagerFactory, | ||
| keyCertChain, key, keyPassword, keyManagerFactory, keyStore, | ||
| sessionCacheSize, sessionTimeout, resumptionController); | ||
| sessionCacheSize, sessionTimeout, endpointIdentificationAlgorithm, | ||
| resumptionController, options); | ||
| success = true; | ||
| } finally { | ||
| if (!success) { | ||
|
|
@@ -94,7 +95,9 @@ static OpenSslSessionContext newSessionContext(ReferenceCountedOpenSslContext th | |
| X509Certificate[] keyCertChain, PrivateKey key, | ||
| String keyPassword, KeyManagerFactory keyManagerFactory, | ||
| String keyStore, long sessionCacheSize, long sessionTimeout, | ||
| ResumptionController resumptionController) | ||
| String endpointIdentificationAlgorithm, | ||
| ResumptionController resumptionController, | ||
| Map.Entry<SslContextOption<?>, Object>... options) | ||
| throws SSLException { | ||
| if (key == null && keyCertChain != null || key != null && keyCertChain == null) { | ||
| throw new IllegalArgumentException( | ||
|
|
@@ -155,8 +158,11 @@ static OpenSslSessionContext newSessionContext(ReferenceCountedOpenSslContext th | |
| TrustManagerFactory.getDefaultAlgorithm()); | ||
| trustManagerFactory.init((KeyStore) null); | ||
| } | ||
| final X509TrustManager manager = chooseTrustManager( | ||
| trustManagerFactory.getTrustManagers(), resumptionController); | ||
| final boolean sanPeerIdentityLookup = isSanPeerIdentityLookupEnabled( | ||
| endpointIdentificationAlgorithm, options); | ||
| final X509TrustManager manager = wrapTrustManagerIfNeeded( | ||
| chooseTrustManager(trustManagerFactory.getTrustManagers(), resumptionController), | ||
| sanPeerIdentityLookup); | ||
|
|
||
| // IMPORTANT: The callbacks set for verification must be static to prevent memory leak as | ||
| // otherwise the context can never be collected. This is because the JNI code holds | ||
|
|
@@ -204,6 +210,37 @@ private static void setVerifyCallback(long ctx, OpenSslEngineMap engineMap, X509 | |
| } | ||
| } | ||
|
|
||
| private static boolean isSanPeerIdentityLookupEnabled(String endpointIdentificationAlgorithm, | ||
| Map.Entry<SslContextOption<?>, Object>[] options) { | ||
| if (endpointIdentificationAlgorithm == null) { | ||
| return false; | ||
| } | ||
| if (options == null) { | ||
| return false; | ||
| } | ||
| for (Map.Entry<SslContextOption<?>, Object> option : options) { | ||
| if (option == null) { | ||
| continue; | ||
| } | ||
| if (SanPeerIdentityConfig.SAN_PEER_IDENTITY_LOOKUP.equals(option.getKey())) { | ||
| return Boolean.TRUE.equals(option.getValue()); | ||
| } | ||
| } | ||
| return false; | ||
| } | ||
|
|
||
| @SuppressJava6Requirement(reason = "Usage guarded by java version check") | ||
| private static X509TrustManager wrapTrustManagerIfNeeded(X509TrustManager manager, | ||
| boolean sanPeerIdentityLookup) { | ||
| if (!sanPeerIdentityLookup) { | ||
| return manager; | ||
| } | ||
| if (useExtendedTrustManager(manager)) { | ||
|
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. Please explain what's the intent behind the
Author
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.
|
||
| return new SanPeerIdentityTrustManager((X509ExtendedTrustManager) manager); | ||
| } | ||
| return manager; | ||
| } | ||
|
|
||
| static final class OpenSslClientSessionContext extends OpenSslSessionContext { | ||
| OpenSslClientSessionContext(ReferenceCountedOpenSslContext context, OpenSslKeyMaterialProvider provider) { | ||
| super(context, provider, SSL.SSL_SESS_CACHE_CLIENT, new OpenSslClientSessionCache(context.engineMap)); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,25 @@ | ||
| /* | ||
| * Copyright 2026 The Netty Project | ||
| * | ||
| * The Netty Project licenses this file to you under the Apache License, | ||
| * version 2.0 (the "License"); you may not use this file except in compliance | ||
| * with the License. You may obtain a copy of the License at: | ||
| * | ||
| * https://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, software | ||
| * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT | ||
| * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the | ||
| * License for the specific language governing permissions and limitations | ||
| * under the License. | ||
| */ | ||
| package io.netty.handler.ssl; | ||
|
|
||
| final class SanPeerIdentityConfig { | ||
| static final SslContextOption<Boolean> SAN_PEER_IDENTITY_LOOKUP = | ||
| new SslContextOption<Boolean>("SAN_PEER_IDENTITY_LOOKUP"); | ||
|
Comment on lines
+19
to
+20
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. Where are we setting these options?
Author
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. This option is not enabled inside Netty itself. It is intended to be set by the downstream caller that builds the client SslContextBuilder. In our case, that caller is BDP, which enables the option when constructing the client SSL context. look here. |
||
|
|
||
| private SanPeerIdentityConfig() { | ||
| } | ||
| } | ||
|
|
||
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.
There is no java version check with
wrapTrustManagerIfNeeded, what is the point of this annotation?Uh oh!
There was an error while loading. Please reload this page.
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.
wrapTrustManagerIfNeeded()referencesX509ExtendedTrustManager, which in my understanding triggers maybe Netty’s Java compatibility/static-analysis checks because it is newer than the older Java 6 SSL APIs. The suppression is used to acknowledge that intentional usage.In this case the safety comes from the runtime capability/type check we've done via
useExtendedTrustManager(manager), which ensures the SAN-aware wrapper(SanPeerIdentityTrustManager) is only applied when the provided trust manager supports the extended engine-aware APIs required by newly implementedSanPeerIdentityTrustManager.There are similar usages of
@SuppressJava6Requirementaround otherX509ExtendedTrustManagerintegrations in the same class, such assetVerifyCallback()andExtendedTrustManagerVerifyCallback().