Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Dec 26, 2025

SkipTlsVerify = true disables all certificate validation (chain, expiration, signature) when users only need to disable revocation checking for certificates where CRL/OCSP endpoints are inaccessible from within clusters.

Changes

  • KubernetesClientConfiguration: Added ServerCertificateCustomValidationCallback property accepting Func<HttpRequestMessage, X509Certificate2, X509Chain, SslPolicyErrors, bool>

    • Takes precedence over SkipTlsVerify when set
    • HttpRequestMessage may be null for WebSocket connections
  • Kubernetes.ConfigInit.cs: Custom callback checked first, then SkipTlsVerify, then default validation

  • WebSocketBuilder.cs: Added SetServerCertificateCustomValidationCallback for WebSocket connection support

  • Kubernetes.WebSocket.cs: Applies custom callback to WebSocket connections

Example Usage

Disable revocation checking only while maintaining chain, expiration, and signature validation:

var config = KubernetesClientConfiguration.InClusterConfig();
config.ServerCertificateCustomValidationCallback = (request, cert, chain, errors) =>
{
    if (errors == SslPolicyErrors.None) return true;
    
    chain.ChainPolicy.RevocationMode = X509RevocationMode.NoCheck;
    return chain.Build(cert);
};
var client = new Kubernetes(config);

Also supports certificate pinning, custom logging, or any X509Chain-based validation logic.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • crl.comodoca.com
    • Triggering command: /home/REDACTED/work/csharp/.codeql-scratch/dbs/csharp/working/.dotnet/dotnet /home/REDACTED/work/csharp/.codeql-scratch/dbs/csharp/working/.dotnet/dotnet restore --no-dependencies /home/REDACTED/work/csharp/csharp/src/KubernetesClient.Classic/KubernetesClient.Classic.csproj --packages /home/REDACTED/work/csharp/.codeql-scratch/dbs/csharp/working/packages /p:DisableImplicitNuGetFallbackFolder=true --verbosity normal /p:TargetFrameworkRootPath=/home/REDACTED/work/csharp/.codeql-scratch/dbs/csharp/working/emptyFakeDotnetRoot /p:NetCoreTargetingPackRoot=/home/REDACTED/work/csharp/.codeql-scratch/dbs/csharp/working/emptyFakeDotnetRoot (dns block)
  • ocsp.comodoca.com
    • Triggering command: /home/REDACTED/work/csharp/.codeql-scratch/dbs/csharp/working/.dotnet/dotnet /home/REDACTED/work/csharp/.codeql-scratch/dbs/csharp/working/.dotnet/dotnet restore --no-dependencies /home/REDACTED/work/csharp/csharp/src/KubernetesClient.Classic/KubernetesClient.Classic.csproj --packages /home/REDACTED/work/csharp/.codeql-scratch/dbs/csharp/working/packages /p:DisableImplicitNuGetFallbackFolder=true --verbosity normal /p:TargetFrameworkRootPath=/home/REDACTED/work/csharp/.codeql-scratch/dbs/csharp/working/emptyFakeDotnetRoot /p:NetCoreTargetingPackRoot=/home/REDACTED/work/csharp/.codeql-scratch/dbs/csharp/working/emptyFakeDotnetRoot (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

Original prompt

This section details on the original issue you should resolve

<issue_title>Add option to disable certificate revocation checking in KubernetesClientConfiguration</issue_title>
<issue_description>## Feature Request

Is your feature request related to a problem? Please describe.

When running applications inside Kubernetes clusters, the Kubernetes API server's TLS certificates often cannot have their revocation status checked because Certificate Revocation List (CRL) and OCSP endpoints are not accessible from within the cluster network. This causes certificate validation to fail even though the certificates are valid.

Currently, the only workaround is to completely disable TLS verification using SkipTlsVerify = true, which is a security risk as it disables all certificate validation including chain verification, expiration checks, and signature validation.

var config = KubernetesClientConfiguration.InClusterConfig();
// Bad: Disables ALL certificate validation
config.SkipTlsVerify = true;</issue_description>

<agent_instructions>add an option to allow user to customize tls verification</agent_instructions>

## Comments on the Issue (you are @copilot in this section)

<comments>
</comments>

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

@k8s-ci-robot k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Dec 26, 2025
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: Copilot
Once this PR has been reviewed and has the lgtm label, please ask for approval from tg123. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Dec 26, 2025
@k8s-ci-robot k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Dec 26, 2025
Copilot AI changed the title [WIP] Add option to disable certificate revocation checking in KubernetesClientConfiguration Add ServerCertificateCustomValidationCallback for granular TLS validation control Dec 26, 2025
Copilot AI requested a review from tg123 December 26, 2025 08:46
@tg123 tg123 marked this pull request as ready for review December 28, 2025 09:03
Copilot AI review requested due to automatic review settings December 28, 2025 09:03
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Dec 28, 2025
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This pull request adds a ServerCertificateCustomValidationCallback property to KubernetesClientConfiguration, enabling fine-grained control over TLS certificate validation. This addresses scenarios where users need to disable only certificate revocation checking while maintaining other security validations, rather than completely disabling all TLS verification with SkipTlsVerify.

  • Added ServerCertificateCustomValidationCallback property that takes precedence over SkipTlsVerify when set
  • Applied the custom callback to both HTTP and WebSocket connections with appropriate platform-specific handling
  • Included comprehensive unit tests demonstrating various validation scenarios including revocation checking, certificate pinning, and precedence behavior

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 10 comments.

Show a summary per file
File Description
src/KubernetesClient/KubernetesClientConfiguration.cs Added the ServerCertificateCustomValidationCallback property with detailed documentation and usage examples
src/KubernetesClient/Kubernetes.ConfigInit.cs Implemented callback initialization logic with proper precedence handling and platform-specific adaptations for NET5_0_OR_GREATER
src/KubernetesClient/Kubernetes.WebSocket.cs Applied custom validation callback to WebSocket connections with proper precedence over CaCerts and SkipTlsVerify
src/KubernetesClient/WebSocketBuilder.cs Added SetServerCertificateCustomValidationCallback method to support custom validation in WebSocket connections
tests/KubernetesClient.Tests/CustomCertificateValidationTests.cs Added comprehensive tests verifying callback configuration, precedence, and various validation scenarios

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

///
/// // Disable revocation checking
/// chain.ChainPolicy.RevocationMode = X509RevocationMode.NoCheck;
/// return chain.Build((X509Certificate2)cert);
Copy link

Copilot AI Dec 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The cast to X509Certificate2 in the documentation example is redundant since the 'cert' parameter is already typed as X509Certificate2 in the callback signature. The example should use 'cert' directly instead of casting it.

Suggested change
/// return chain.Build((X509Certificate2)cert);
/// return chain.Build(cert);

Copilot uses AI. Check for mistakes.

// Disable revocation checking
chain.ChainPolicy.RevocationMode = X509RevocationMode.NoCheck;
return chain.Build((X509Certificate2)cert);
Copy link

Copilot AI Dec 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The cast to X509Certificate2 is redundant. Since 'cert' parameter in the callback is already X509Certificate2, you can pass it directly to chain.Build() without casting.

Suggested change
return chain.Build((X509Certificate2)cert);
return chain.Build(cert);

Copilot uses AI. Check for mistakes.
(sender, certificate, chain, sslPolicyErrors) =>
{
// RemoteCertificateValidationCallback doesn't provide HttpRequestMessage, so pass null
var cert = certificate as X509Certificate2 ?? new X509Certificate2(certificate);
Copy link

Copilot AI Dec 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Creating a new X509Certificate2 from an existing certificate can lead to resource leaks since the newly created certificate is not disposed. Consider using a cast with null-coalescing instead: 'var cert = certificate as X509Certificate2 ?? throw new ArgumentException("Certificate must be X509Certificate2");' or document that callers should ensure proper disposal if needed.

Suggested change
var cert = certificate as X509Certificate2 ?? new X509Certificate2(certificate);
var cert = certificate as X509Certificate2
?? throw new ArgumentException("Certificate must be X509Certificate2", nameof(certificate));

Copilot uses AI. Check for mistakes.
(sender, certificate, chain, sslPolicyErrors) =>
{
// Convert to the expected signature (with HttpRequestMessage as first parameter)
var cert = certificate as X509Certificate2 ?? new X509Certificate2(certificate);
Copy link

Copilot AI Dec 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Creating a new X509Certificate2 from an existing certificate can lead to resource leaks since the newly created certificate is not disposed. Consider using a cast with null-coalescing instead: 'var cert = certificate as X509Certificate2 ?? throw new ArgumentException("Certificate must be X509Certificate2");' or document that callers should ensure proper disposal if needed.

Copilot uses AI. Check for mistakes.
};

// Act
var client = new Kubernetes(config);
Copy link

Copilot AI Dec 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This assignment to client is useless, since its value is never read.

Copilot uses AI. Check for mistakes.
};

// Act
var client = new Kubernetes(config);
Copy link

Copilot AI Dec 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This assignment to client is useless, since its value is never read.

Copilot uses AI. Check for mistakes.
};

// Act
var client = new Kubernetes(config);
Copy link

Copilot AI Dec 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This assignment to client is useless, since its value is never read.

Copilot uses AI. Check for mistakes.
};

// Act
var client = new Kubernetes(config);
Copy link

Copilot AI Dec 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This assignment to client is useless, since its value is never read.

Copilot uses AI. Check for mistakes.
};

// Act
var client = new Kubernetes(config);
Copy link

Copilot AI Dec 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This assignment to client is useless, since its value is never read.

Copilot uses AI. Check for mistakes.
};

// Act
var client = new Kubernetes(config);
Copy link

Copilot AI Dec 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This assignment to client is useless, since its value is never read.

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add option to disable certificate revocation checking in KubernetesClientConfiguration

3 participants