feat: Replace X509Certificate2 with BouncyCastle for certificate operations#64
Open
spbsoluble wants to merge 80 commits intorelease-1.3from
Open
feat: Replace X509Certificate2 with BouncyCastle for certificate operations#64spbsoluble wants to merge 80 commits intorelease-1.3from
spbsoluble wants to merge 80 commits intorelease-1.3from
Conversation
Add BouncyCastle-based certificate utility layer to replace deprecated X509Certificate2 APIs (SYSLIB0028). **Added:** - CertificateUtilities: Comprehensive utility class with 30+ methods - Certificate parsing (PEM, DER, PKCS12) with auto-detection - Properties extraction (thumbprint, subject/issuer, dates, algorithms) - Private key operations (extract, export as PEM/PKCS8) - Certificate chain loading and manipulation - Format detection and conversion utilities - K8SCertificateContext: Wrapper model replacing X509Certificate2 - Factory methods for all certificate formats - Computed properties for certificate attributes - Export methods for certificates and private keys - Test project with 40+ comprehensive unit tests - Certificate parsing, format detection, conversion tests - Thumbprint calculation validated against X509Certificate2 - RSA and ECDSA test certificate generation - Null argument validation **Technical Details:** - Uses existing BouncyCastle.Cryptography v2.6.2 dependency - Thumbprint uses SHA-1 hash (uppercase hex) matching X509Certificate2 - PKCS#8 format for private keys (cross-platform compatible) - Reuses patterns from KubeClient.cs and K8SPKCS12/Store.cs **Impact:** - No existing code modified (additive only) - Zero breaking changes - Builds successfully on net8.0 and net10.0 Refs: SYSLIB0028 deprecation warnings in JobBase.cs:1138,1154
Complete refactoring of KubeClient.cs PKCS12 operations to use BouncyCastle Pkcs12Store instead of deprecated X509Certificate2Collection and X509Certificate2 APIs. **Added Helper Methods:** - FindAliasByCN(): Find PKCS12 alias by Common Name - FindAliasByThumbprint(): Find PKCS12 alias by thumbprint - FindAliasByName(): Find PKCS12 alias by partial DN match All methods work directly with Pkcs12Store without X509Certificate2 **Refactored Methods:** 1. RemoveFromPKCS12SecretStore: - Replaced X509Certificate2Collection with Pkcs12Store - Import() → store.Load() - Remove() → store.DeleteEntry() - Export() → store.Save() 2. UpdatePKCS12SecretStore: - Replaced X509Certificate2Collection with Pkcs12Store - Multiple Import() calls → store.Load() - Certificate finding → FindAliasByCN/FindAliasByName - SetKeyEntry() for adding certs with private keys - Export() → store.Save() 3. CreatePKCS12Collection: - Removed X509Certificate2 instantiation - Direct Pkcs12Store loading and manipulation - Use CertificateUtilities.GetThumbprint() for alias 4. GetCertificateSigningRequestStatus: - X509Certificate2 → CertificateUtilities.ParseCertificateFromPem() 5. DiscoverCertificates: - X509Certificate2 → CertificateUtilities.ParseCertificateFromPem() - GetNameInfo() → CertificateUtilities.GetSubjectCN() **Deprecated (backward compat):** - FindCertificateByCN: Marked [Obsolete] - FindCertificateByThumbprint: Marked [Obsolete] - FindCertificateByAlias: Marked [Obsolete] **Technical Details:** - Uses CertificateUtilities for all certificate property extraction - Maintains functional equivalence with existing code - Zero X509Certificate2Collection usage in refactored methods - Pkcs12Store used for all PKCS12 operations **Impact:** - Build succeeds with 0 errors - SYSLIB0028/SYSLIB0057 warnings eliminated in KubeClient.cs - No breaking changes to public APIs - All KubeClient PKCS12 operations now use BouncyCastle
Complete refactoring of JobBase.cs to eliminate deprecated X509Certificate2 APIs and use BouncyCastle for all certificate operations. SYSLIB0028 warnings eliminated. **Added:** - GetKeyBytes(Pkcs12Store, string, string): New method using BouncyCastle - Extracts private keys directly from Pkcs12Store - Uses CertificateUtilities.ExportPrivateKeyPkcs8() - No deprecated X509Certificate2.PrivateKey usage **Refactored:** 1. InitJobCertificate() method: - Replaced X509Certificate2 instantiation with Pkcs12Store loading - Uses LoadPkcs12Store() for all certificate data - Certificate parsing: CertificateUtilities for thumbprint/properties - Key type detection: CertificateUtilities.GetPrivateKeyType() - Private key extraction: KubeClient.ExtractPrivateKeyAsPem() - Chain handling: Uses BouncyCastle X509CertificateEntry arrays 2. K8SJobCertificate class: - Added K8SCertificateContext property (optional, modern approach) - Added GetCertificateContext() factory method - Maintains backward compatibility with existing properties - Enhanced documentation for BouncyCastle properties **Deprecated (backward compat):** - GetKeyBytes(X509Certificate2, string): Marked [Obsolete] - Pragma directives suppress SYSLIB0028 warnings in old method - Kept for backward compatibility only - Warning logged when called **Technical Details:** - Zero SYSLIB0028 warnings in JobBase.cs - Zero X509Certificate2 usage in refactored code paths - Uses CertificateUtilities for all certificate operations - Pkcs12Store used for certificate/key loading **Impact:** - SYSLIB0028 warnings eliminated in JobBase.cs ✅ - No breaking changes to public APIs - All InitJobCertificate operations now use BouncyCastle
Refactor Inventory.cs certificate parsing to use CertificateUtilities instead of deprecated X509Certificate2 constructors. SYSLIB0057 warnings eliminated. **Refactored:** - Lines 512-520: Certificate parsing with thumbprint extraction - X509Certificate2 → CertificateUtilities.ParseCertificateFromPem/Der() - cert.Thumbprint → CertificateUtilities.GetThumbprint() - Lines 594-598: Certificate validation parsing - X509Certificate2 → CertificateUtilities.ParseCertificateFromPem/Der() - Maintains same validation logic **Technical Details:** - Simple string replacement pattern for both locations - Automatically detects PEM vs DER format - Uses CertificateUtilities.ParseCertificateFromPem() for PEM - Uses CertificateUtilities.ParseCertificateFromDer() for DER - Thumbprint extraction via CertificateUtilities.GetThumbprint() **Impact:** - SYSLIB0057 warnings eliminated in Inventory.cs ✅ - Zero SYSLIB deprecation warnings remaining ✅ - No breaking changes to inventory functionality - Maintains identical behavior with BouncyCastle
…tive data redaction Implements security-first comprehensive logging across the entire Kubernetes orchestrator extension, adding trace-level observability while ensuring zero sensitive data exposure. This addresses the need for verbose troubleshooting capabilities without compromising security. ## New Components ### LoggingUtilities.cs (~450 lines) Centralized sensitive data redaction utility providing secure logging helpers: - RedactPassword() - Shows length only, never password value - GetPasswordCorrelationId() - SHA-256 hash prefix for password correlation across logs - RedactPrivateKey*() - Redacts PEM, DER, bytes, and BouncyCastle AsymmetricKeyParameter objects - GetCertificateSummary() - Shows thumbprint, CN, validity dates only (no certificate data) - GetSecretSummary() - Shows V1Secret metadata only (name, namespace, type, key count) - RedactKubeconfig() - Safely logs kubeconfig structure without tokens/certificates - RedactToken() - Shows token length with prefix/suffix only - GetFieldPresence() - Indicates field presence without revealing values ### LoggingSafetyTests.cs (12 tests) Automated security verification suite ensuring no sensitive data leaks: - Regex-based source code scanning for insecure logging patterns - Verification of all LoggingUtilities redaction methods - Detection of TODO:INSECURE comments - Password, private key, and token logging detection ## Security Improvements ### K8SJKS/Store.cs - REMOVED 15+ insecure commented logging statements marked with "TODO: INSECURE" - Replaced all direct password logging with LoggingUtilities.RedactPassword() - Added password correlation IDs for troubleshooting without exposing passwords - Lines cleaned: 48, 58-59, 81, 98, 150-151, 157-158, 181-182, 194-195, 222-223, 244-245, 311-312, 402-403, 410-411 ### JobBase.cs - Standardized password redaction in getK8SStorePassword() method (line 1040) - Added password correlation IDs for troubleshooting - Enhanced InitJobCertificate() with certificate summary logging - Added PKCS12 data size logging (not content) - Added private key presence indicators (not key material) ### Inventory.cs - Replaced direct password hash logging with correlation IDs (line 327) ## Enhanced Logging Coverage ### CertificateUtilities.cs (~60+ logging statements) Added comprehensive trace-level logging to previously unlogged utility: - ParseCertificate() - Format detection and parsing flow - ParseCertificateFromPem() - PEM parsing with certificate summaries - ParseCertificateFromDer() - DER parsing diagnostics - ParseCertificateFromPkcs12() - PKCS12 parsing with redacted passwords - GetThumbprint() - Computed thumbprint logging - ExtractPrivateKey() - Key extraction with redacted key info - ExportPrivateKeyPkcs8() - Export with redacted bytes - LoadPkcs12Store() - Store loading with password correlation IDs - LoadCertificateChain() - Chain parsing diagnostics - DetectFormat() - Format detection logging ### K8SCertificateContext.cs (~40 logging statements) Added logging to factory methods and exports: - FromPkcs12() - Alias selection, certificate loading, chain extraction - FromPem() - PEM parsing and chain detection - FromDer() - DER parsing diagnostics - FromCertificate() - Context creation logging - ExportCertificatePem() - Export with redacted output - ExportPrivateKeyPkcs8() - Key export with redacted bytes ### KubeClient.cs Enhanced ParseKubeConfig() and RemoveFromPKCS12SecretStore(): - Safe kubeconfig logging without tokens/certificates - Token presence indicators (not token values) - Cluster/user/context metadata logging - Buddy secret password handling with redaction - Password correlation IDs for troubleshooting ### Management.cs Enhanced HandleOpaqueSecret(): - Certificate summary logging (thumbprint, CN, validity) - Private key presence indicators - Chain certificate count logging - Operation parameter logging (overwrite, append modes) ## Log Level Best Practices TRACE: Method parameters (redacted), execution flow, correlation IDs, data sizes DEBUG: Certificate metadata, operation outcomes, configuration values, K8S resource metadata INFORMATION: Job milestones, successful operations WARNING: Non-critical errors with recovery, deprecated usage ERROR: Failures with full context and stack traces ## Testing - All 12 LoggingSafetyTests passed (100% pass rate) - Build successful: 0 errors, 35 pre-existing warnings - Verified no sensitive data patterns in logs - Automated regex scanning detects password/key/token logging ## Security Guarantees ✅ Zero passwords logged (only length and correlation IDs) ✅ Zero private keys logged (only type and presence indicators) ✅ Zero tokens logged (only length and truncated prefix/suffix) ✅ Zero certificate PEM/DER logged (only thumbprint, CN, validity) ✅ Zero kubeconfig secrets logged (only structure metadata) ✅ All V1Secret data values redacted (only keys and counts) ## Breaking Changes None - Only additive logging enhancements Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Dependency ReviewThe following issues were found:
License Issues.github/workflows/dotnet-security-scan.yml
.github/workflows/secret-scanning.yml
.github/workflows/unit-tests.yml
OpenSSF ScorecardScorecard details
Scanned Files
|
…to an existing secret that has a private key.
…tion Feat/experimental der pem detection
…amespace` parameter
…nto feat/x509certificate2_removal # Conflicts: # kubernetes-orchestrator-extension.Tests/Integration/K8SNSStoreIntegrationTests.cs
…chedules an job with no certificate. NOTE `K8SCluster` and `K8SNS` do not support this operation as they manage multiple secrets.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.