fix: redirect config-server dependency to config-server-release for SHA-1 fix - #748
Open
G-Gobi wants to merge 1 commit into
Open
fix: redirect config-server dependency to config-server-release for SHA-1 fix#748G-Gobi wants to merge 1 commit into
G-Gobi wants to merge 1 commit into
Conversation
…HA-1 fix github.com/cloudfoundry/config-server is archived (read-only) and can never receive new tags. Its actual source of truth is now cloudfoundry/config-server-release's src/config-server directory, which merged a fix replacing SHA-1 with SHA-256 (RFC 7093 Method 4) for X.509 SubjectKeyIdentifier computation in certificate_generator.go (cloudfoundry/config-server-release#21). Adds a replace directive pointing the existing github.com/cloudfoundry/config-server import path at github.com/cloudfoundry/config-server-release/src/config-server, pinned to the merge commit (132a0f6). A plain version bump isn't possible: no v0.2.0-equivalent tag exists for the nested Go module (the repo's bare v0.2.0/v0.1.x tags are BOSH release tags, not path-prefixed Go module tags), and the nested go.mod's declared module path doesn't match the config-server-release import path anyway, which is why replace (not require) is needed here regardless. CertificateReconciler unconditionally calls Generate() -> generateCertificate() for every Certificate CRD, which previously computed SubjectKeyId via SHA-1 unconditionally. Verified directly against the vendored dependency that certificates now get a 32-byte SHA-256-based SubjectKeyId instead of the old 20-byte SHA-1 one. No public API surface (Generate, CertResponse) changed, and the existing param-based drift detection in the reconciler is unaffected, so this does not force regeneration of already-existing certificates. Bumping the dependency also pulled in several transitive dependency updates via Go's minimal version selection (bosh-utils, go-logr, golang.org/x/{crypto,mod,net,sync,sys,term,text,tools}) since config-server-release's go.mod requires newer versions of those than this module currently did. Signed-off-by: Gobi Ganesan <gobi.ganesan1509@gmail.com>
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.
Summary
github.com/cloudfoundry/config-serveris archived (read-only on GitHub) and can never receive new tags.cloudfoundry/config-server-release'ssrc/config-serverdirectory, which merged cloudfoundry/config-server-release#21 — replacing SHA-1 with SHA-256 (RFC 7093 Method 4) for X.509SubjectKeyIdentifiercomputation incertificate_generator.go.replacedirective redirecting the existinggithub.com/cloudfoundry/config-serverimport path togithub.com/cloudfoundry/config-server-release/src/config-server, pinned to the merge commit (132a0f6) via its auto-derived pseudo-version.Why
replaceand not a version bumpNo
v0.2.0-equivalent tag exists for the nested Go module — the repo'sv0.2.0/v0.1.xtags are bare, repo-root BOSH release tags, not thesrc/config-server/vX.Y.Z-prefixed tags Go module tooling requires for a subdirectory module (confirmed directly: attempting to resolvesrc/config-server@v0.2.0fails withunknown revision src/config-server/v0.2.0). Separately, the nestedgo.mod's declaredmodulepath (github.com/cloudfoundry/config-server) doesn't match theconfig-server-releaseimport path, so a plainrequirebump wouldn't resolve regardless —replaceis the correct mechanism here either way.Impact
CertificateReconcilerunconditionally callsGenerate()→generateCertificate()for everyCertificateCRD, which previously computedSubjectKeyIdvia SHA-1 unconditionally. Verified directly against the vendored dependency (via an in-module test, since removed) that newly generated certificates now get a 32-byte SHA-256-basedSubjectKeyIdinstead of the old 20-byte SHA-1 one.Generate(interface{}) (interface{}, error),CertResponse{Certificate, PrivateKey, CA}are untouched) — this is a drop-in swap.GenerateInputs.IsChanged) only compares spec-derivedcertParams, not generated cert bytes, so this does not force regeneration of already-existing certificates on upgrade.bosh-utils,go-logr,golang.org/x/{crypto,mod,net,sync,sys,term,text,tools}), sinceconfig-server-release's owngo.modrequires newer versions of those than this module currently did. This inflates the vendor diff beyond just the SHA-1 fix, but is a mechanical, expected consequence of MVS — not a manual change.Test plan
go build ./...andgo build -mod=vendor ./...go vet ./...(pre-existing unrelated findings only, unchanged by this PR)go mod tidy && go mod vendorproduce no further diff (fully consistent)go test ./pkg/generator/...passescfgtypes.NewCertificateGenerator(nil).Generate(...)now has a 32-byteSubjectKeyId, confirming the SHA-256 fix is actually exercised through this dependency, not just present in source
Validated on `ubuntu@10.85.2.109` using a kind cluster. Tests: (1) stock secretgen-controller v0.21.1 genuinely produces SHA-1-based `SubjectKeyId`s, (2) after upgrading to our fixed image, pre-existing certificates are left byte-for-byte untouched, (3) newly created certificates get SHA-256-based `SubjectKeyId`s and correctly chain against pre-existing (old, SHA-1) CAs, and (4) if an existing certificate's `Secret` is deleted post-upgrade, it regenerates using the new SHA-256 code.Live Cluster Validation — secretgen-controller SHA-1 → SHA-256 fix (branch
fix/config-server-sha1-removal, commitde54e5e8)1. Create kind cluster
2. Install stock secretgen-controller v0.21.1
3. Seed brownfield state — CA + leaf cert on stock (SHA-1) controller
4. Baseline — confirm claim 1: stock controller genuinely produces SHA-1
Both
Subject Key Identifiers are 20 bytes — the SHA-1 length. Claim 1 confirmed: the stock controller genuinely produces SHA-1-basedSubjectKeyIds, not just "presumed from source code."5. Build our fixed branch and a custom controller image
6. Load into kind and upgrade the deployment
7. Confirm claim 2 (part 1) — existing certs untouched, despite being reconciled
Both objects were reconciled by the new (fixed) controller on startup — confirmed via logs — yet their
SecretresourceVersions are exactly unchanged (629, 635 — identical to step 4). Kubernetes only bumpsresourceVersionon an actual write, so this alone proves no regeneration occurred.Byte-for-byte identical PEM data before and after the upgrade, and the exact same SKI value (not just the same length). Claim 2 (part 1) confirmed: pre-existing certificates are completely unaffected by the upgrade.
8. Confirm claim 2 (part 2) — new certs get SHA-256, chain correctly against the old CA
test-leaf-after'sSubject Key Identifieris now 32 bytes (SHA-256), while itsAuthority Key Identifierstill correctly matchestest-ca's original 20-byte SHA-1SubjectKeyId— confirming AKI is read directly off the real CA certificate, not recomputed. The chain validates cleanly (OK) despite the algorithm/length mismatch between CA and leaf, per RFC 5280. Claim 2 (part 2) confirmed.9. Deletion case — does a deleted cert regenerate with SHA-256?
(Deleting the
Secretalone doesn't trigger an immediate reconcile —CertificateReconciler.AttachWatchesonly watchesCertificateobjects, notSecrets. A trivial annotation touch on theCertificateCR was used to force it, since it's watched viaEnqueueRequestForObject.)Confirmed: once the
Secretis deleted and reconciliation is re-triggered,CertificateReconcilertakes thecreateSecret(not-found) path, generates a completely new certificate (new key pair —DIFFERENTbyte-diff, not just a new SKI), and that new certificate has a 32-byte SHA-256SubjectKeyId. ItsAuthority Key Identifierstill correctly matches the untouchedtest-ca's original SKI, and the chain still validates.resourceVersionjumped to1073, confirming a real write happened this time — in clear contrast to steps 7–8 where untouched objects kept their originalresourceVersion.Conclusion
All four things asked were directly verified on a live kind cluster, not inferred from source alone:
SubjectKeyIds for both CA and leaf certs.test-caandtest-leaf-beforeare reconciled (per logs) but theirSecrets are byte-for-byte identical andresourceVersion-unchanged — the dependency bump does not force regeneration.test-leaf-after) against the same pre-existing CA gets a 32-byte SHA-256SubjectKeyId, correctly chains against the CA's old SHA-1 SKI viaAuthorityKeyId, andopenssl verifypasses.Secret(with a manual reconcile trigger, sinceSecretdeletion isn't itself watched) causes a full regeneration — new key pair, new serial, and a 32-byte SHA-256 SKI — still correctly chaining against the unchanged CA.No regression, and the fix behaves correctly in every tested scenario, including mixed old-CA/new-leaf interoperability.