Commit 1e8b828
bosh-nats-sync: use code.cloudfoundry.org/tlsconfig instead of hand-rolled tls.Config
Address item 1 in PR review comment
#2746 (comment):
authprovider and userssync both built their director/UAA TLS client by hand
(manual tls.Config{MinVersion}, x509.NewCertPool(), AppendCertsFromPEM). CF
already ships code.cloudfoundry.org/tlsconfig as the standard helper for
exactly this, and the reviewer noted the hand-rolled config was actually
*less* hardened than the org-standard one (no MaxVersion, no cipher-suite
pinning).
Deliberately does NOT touch items 2/3 from the same comment (reusing
bosh-cli's director/uaa packages for the director API and UAA calls) per
explicit instruction to avoid a github.com/cloudfoundry/bosh-cli dependency;
that remains a footprint-vs-reuse tradeoff for a future PR, not something to
pull in incidentally here.
pkg/authprovider/auth_provider.go:
- buildHTTPClient now builds its *tls.Config via
tlsconfig.Build(tlsconfig.WithExternalServiceDefaults()).Client(...),
applying tlsconfig.WithAuthorityFromFile(caCertPath) only when CAFilePath()
resolves to a file with non-empty content. WithExternalServiceDefaults()
pins MinVersion/MaxVersion to TLS 1.2-1.3 and a curated Mozilla-Intermediate
cipher-suite list, in place of the previous MinVersion-only config.
- Preserves existing behavior exactly: a configured-but-unreadable CA file is
still a hard error, an empty file still falls back to the system trust
store, and a malformed (but present/non-empty) cert is still a hard error.
pkg/userssync/users_sync.go:
- buildHTTPClient now uses the same tlsconfig.Build(...).Client(...) pattern
for the director API client. Replaced directorCACertPool (manual
x509.CertPool + AppendCertsFromPEM) with a small usableCACertContent(path)
predicate that gates whether tlsconfig.WithAuthorityFromFile is applied.
- Preserves existing behavior: director_ca_cert missing/unreadable/empty
falls back to the system trust store (no error, matching Ruby's
usable_director_ca_cert?), while a configured-but-unparseable cert is a
hard error rather than a silent fallback.
pkg/userssync/export_test.go:
- Replaced the removed DirectorCACertPool test hook with
BuildDirectorTLSConfig, which builds the real *http.Client via
buildHTTPClient and returns its *tls.Config, so tests assert against the
actual TLS configuration (RootCAs, MinVersion) rather than an internal
CertPool-returning helper that no longer exists.
pkg/userssync/users_sync_test.go:
- Rewrote the "directorCACertPool" spec block against BuildDirectorTLSConfig:
same four cases (not configured / missing file / whitespace-only file /
unparseable content) now assert on tlsCfg.RootCAs and the error message,
plus a new case asserting MinVersion is pinned to TLS 1.2 via
WithExternalServiceDefaults.
Dependency footprint: go.mod/go.sum/vendor pick up
code.cloudfoundry.org/tlsconfig v0.61.0 and a matching golang.org/x/sys
patch bump (v0.44.0 -> v0.46.0, already an existing indirect dependency).
tlsconfig's own test-only dependencies (certstrap, go.step.sm/crypto,
golang.org/x/crypto, etc.) appear in go.sum for module-graph completeness
but are not imported by any non-test file, so `go mod vendor` does not pull
them into vendor/ or the build.
Verified `go build`, `go vet`, and `go test -race ./...` all pass with both
the default module resolution and explicit `-mod=vendor` (matching how
packages/nats/packaging invokes go build); gofmt and golangci-lint are clean.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>1 parent 0899a20 commit 1e8b828
83 files changed
Lines changed: 2635 additions & 831 deletions
File tree
- src/bosh-nats-sync
- pkg
- authprovider
- userssync
- vendor
- code.cloudfoundry.org/tlsconfig
- golang.org/x/sys/unix
Some content is hidden
Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
3 | 3 | | |
4 | 4 | | |
5 | 5 | | |
| 6 | + | |
6 | 7 | | |
7 | 8 | | |
8 | 9 | | |
| |||
19 | 20 | | |
20 | 21 | | |
21 | 22 | | |
22 | | - | |
| 23 | + | |
23 | 24 | | |
24 | 25 | | |
25 | 26 | | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
1 | 5 | | |
2 | 6 | | |
3 | 7 | | |
| |||
32 | 36 | | |
33 | 37 | | |
34 | 38 | | |
| 39 | + | |
| 40 | + | |
35 | 41 | | |
36 | 42 | | |
37 | 43 | | |
38 | 44 | | |
| 45 | + | |
| 46 | + | |
39 | 47 | | |
40 | 48 | | |
41 | 49 | | |
| |||
46 | 54 | | |
47 | 55 | | |
48 | 56 | | |
| 57 | + | |
| 58 | + | |
49 | 59 | | |
50 | 60 | | |
| 61 | + | |
| 62 | + | |
51 | 63 | | |
52 | 64 | | |
53 | 65 | | |
| |||
56 | 68 | | |
57 | 69 | | |
58 | 70 | | |
59 | | - | |
60 | | - | |
| 71 | + | |
| 72 | + | |
61 | 73 | | |
62 | 74 | | |
63 | 75 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
2 | 2 | | |
3 | 3 | | |
4 | 4 | | |
5 | | - | |
6 | | - | |
7 | 5 | | |
8 | 6 | | |
9 | 7 | | |
| |||
16 | 14 | | |
17 | 15 | | |
18 | 16 | | |
| 17 | + | |
19 | 18 | | |
20 | 19 | | |
21 | 20 | | |
| |||
97 | 96 | | |
98 | 97 | | |
99 | 98 | | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
100 | 106 | | |
101 | | - | |
102 | | - | |
103 | 107 | | |
| 108 | + | |
| 109 | + | |
104 | 110 | | |
105 | 111 | | |
106 | 112 | | |
107 | 113 | | |
108 | 114 | | |
109 | 115 | | |
110 | | - | |
111 | | - | |
112 | | - | |
113 | | - | |
114 | | - | |
| 116 | + | |
115 | 117 | | |
116 | 118 | | |
117 | 119 | | |
| 120 | + | |
| 121 | + | |
| 122 | + | |
| 123 | + | |
| 124 | + | |
118 | 125 | | |
119 | 126 | | |
120 | 127 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1 | 1 | | |
2 | 2 | | |
3 | | - | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
4 | 7 | | |
5 | 8 | | |
6 | 9 | | |
7 | 10 | | |
8 | 11 | | |
9 | | - | |
10 | | - | |
11 | | - | |
12 | | - | |
13 | | - | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
14 | 22 | | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
4 | 4 | | |
5 | 5 | | |
6 | 6 | | |
7 | | - | |
8 | | - | |
9 | 7 | | |
10 | 8 | | |
11 | 9 | | |
| |||
24 | 22 | | |
25 | 23 | | |
26 | 24 | | |
| 25 | + | |
| 26 | + | |
27 | 27 | | |
28 | 28 | | |
29 | 29 | | |
| |||
394 | 394 | | |
395 | 395 | | |
396 | 396 | | |
397 | | - | |
398 | | - | |
399 | | - | |
400 | | - | |
401 | | - | |
| 397 | + | |
| 398 | + | |
| 399 | + | |
| 400 | + | |
| 401 | + | |
| 402 | + | |
| 403 | + | |
| 404 | + | |
402 | 405 | | |
403 | | - | |
404 | | - | |
405 | | - | |
406 | | - | |
| 406 | + | |
| 407 | + | |
| 408 | + | |
| 409 | + | |
| 410 | + | |
407 | 411 | | |
408 | | - | |
409 | | - | |
| 412 | + | |
| 413 | + | |
| 414 | + | |
| 415 | + | |
410 | 416 | | |
| 417 | + | |
411 | 418 | | |
412 | 419 | | |
413 | 420 | | |
414 | 421 | | |
415 | 422 | | |
416 | 423 | | |
417 | | - | |
418 | | - | |
419 | | - | |
420 | | - | |
421 | | - | |
422 | | - | |
423 | | - | |
424 | | - | |
425 | | - | |
426 | | - | |
427 | | - | |
428 | | - | |
429 | | - | |
430 | | - | |
431 | | - | |
432 | | - | |
433 | | - | |
| 424 | + | |
| 425 | + | |
| 426 | + | |
| 427 | + | |
| 428 | + | |
| 429 | + | |
| 430 | + | |
434 | 431 | | |
435 | | - | |
| 432 | + | |
| 433 | + | |
436 | 434 | | |
437 | 435 | | |
438 | 436 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
3 | 3 | | |
4 | 4 | | |
5 | 5 | | |
| 6 | + | |
6 | 7 | | |
7 | 8 | | |
8 | 9 | | |
| |||
866 | 867 | | |
867 | 868 | | |
868 | 869 | | |
869 | | - | |
870 | | - | |
871 | | - | |
| 870 | + | |
| 871 | + | |
| 872 | + | |
| 873 | + | |
872 | 874 | | |
873 | 875 | | |
874 | 876 | | |
| |||
878 | 880 | | |
879 | 881 | | |
880 | 882 | | |
881 | | - | |
| 883 | + | |
882 | 884 | | |
883 | | - | |
| 885 | + | |
884 | 886 | | |
885 | | - | |
| 887 | + | |
886 | 888 | | |
887 | 889 | | |
888 | | - | |
| 890 | + | |
889 | 891 | | |
890 | | - | |
| 892 | + | |
891 | 893 | | |
892 | | - | |
| 894 | + | |
893 | 895 | | |
894 | 896 | | |
895 | | - | |
| 897 | + | |
896 | 898 | | |
897 | 899 | | |
898 | 900 | | |
899 | | - | |
| 901 | + | |
900 | 902 | | |
901 | | - | |
| 903 | + | |
902 | 904 | | |
903 | 905 | | |
904 | 906 | | |
905 | 907 | | |
906 | 908 | | |
907 | 909 | | |
908 | | - | |
| 910 | + | |
909 | 911 | | |
910 | 912 | | |
911 | 913 | | |
| 914 | + | |
| 915 | + | |
| 916 | + | |
| 917 | + | |
| 918 | + | |
| 919 | + | |
| 920 | + | |
912 | 921 | | |
913 | 922 | | |
914 | 923 | | |
| |||
Lines changed: 18 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
0 commit comments