Skip to content

Commit 0451166

Browse files
committed
xds: fix TSAN data races in xDS security components and tests
This change resolves several data races reported by ThreadSanitizer (TSAN) in XdsSecurityClientServerTest and associated xDS security components. Key changes: - Made certificate, key storage, and SSL context fields volatile in CertProviderSslContextProvider and DynamicSslContextProvider to ensure safe publication between Netty EventLoop threads and background FileWatcher threads. - Made experimental static flags (enableSpiffe and useChannelAuthorityIfNoSniApplicable) volatile. These flags are toggled by tests while being read by background networking threads; making them volatile ensures these transitions are thread-safe and TSAN-compliant.
1 parent f90b881 commit 0451166

4 files changed

Lines changed: 7 additions & 7 deletions

File tree

xds/src/main/java/io/grpc/xds/internal/security/DynamicSslContextProvider.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ public abstract class DynamicSslContextProvider extends SslContextProvider {
4242

4343
protected final List<Callback> pendingCallbacks = new ArrayList<>();
4444
@Nullable protected final CertificateValidationContext staticCertificateValidationContext;
45-
@Nullable protected AbstractMap.SimpleImmutableEntry<SslContext, X509TrustManager>
45+
@Nullable protected volatile AbstractMap.SimpleImmutableEntry<SslContext, X509TrustManager>
4646
sslContextAndTrustManager;
4747

4848
protected DynamicSslContextProvider(

xds/src/main/java/io/grpc/xds/internal/security/certprovider/CertProviderSslContextProvider.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -39,10 +39,10 @@ abstract class CertProviderSslContextProvider extends DynamicSslContextProvider
3939
@Nullable private final NoExceptionCloseable rootCertHandle;
4040
@Nullable private final CertificateProviderInstance certInstance;
4141
@Nullable protected final CertificateProviderInstance rootCertInstance;
42-
@Nullable protected PrivateKey savedKey;
43-
@Nullable protected List<X509Certificate> savedCertChain;
44-
@Nullable protected List<X509Certificate> savedTrustedRoots;
45-
@Nullable protected Map<String, List<X509Certificate>> savedSpiffeTrustMap;
42+
@Nullable protected volatile PrivateKey savedKey;
43+
@Nullable protected volatile List<X509Certificate> savedCertChain;
44+
@Nullable protected volatile List<X509Certificate> savedTrustedRoots;
45+
@Nullable protected volatile Map<String, List<X509Certificate>> savedSpiffeTrustMap;
4646
private final boolean isUsingSystemRootCerts;
4747

4848
protected CertProviderSslContextProvider(

xds/src/main/java/io/grpc/xds/internal/security/certprovider/FileWatcherCertificateProviderProvider.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ public final class FileWatcherCertificateProviderProvider implements Certificate
3838

3939
// TODO(lwge): Remove the old env var check once it's confirmed to be unused.
4040
@VisibleForTesting
41-
public static boolean enableSpiffe = GrpcUtil.getFlag("GRPC_EXPERIMENTAL_SPIFFE_TRUST_BUNDLE_MAP",
41+
public static volatile boolean enableSpiffe = GrpcUtil.getFlag("GRPC_EXPERIMENTAL_SPIFFE_TRUST_BUNDLE_MAP",
4242
false) || GrpcUtil.getFlag("GRPC_EXPERIMENTAL_XDS_MTLS_SPIFFE", false);
4343
private static final String CERT_FILE_KEY = "certificate_file";
4444
private static final String KEY_FILE_KEY = "private_key_file";

xds/src/main/java/io/grpc/xds/internal/security/trust/CertificateUtils.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@
3030
* Contains certificate utility method(s).
3131
*/
3232
public final class CertificateUtils {
33-
public static boolean useChannelAuthorityIfNoSniApplicable
33+
public static volatile boolean useChannelAuthorityIfNoSniApplicable
3434
= GrpcUtil.getFlag("GRPC_USE_CHANNEL_AUTHORITY_IF_NO_SNI_APPLICABLE", false);
3535

3636
/**

0 commit comments

Comments
 (0)