From e8f21fcd6fd6b39bdd55168204e4760f3689d9ef Mon Sep 17 00:00:00 2001 From: ryusuke Date: Mon, 5 Oct 2026 21:13:53 +0900 Subject: [PATCH 1/2] Always send the configured client certificate to HTTP stores Go's default client certificate selection sends no certificate when the server's CertificateRequest doesn't list the issuing CA. Set GetClientCertificate so the configured certificate is always presented, as curl and OpenSSL-based clients do. Fixes #443 Co-Authored-By: Claude Opus 5.5 --- store.go | 8 ++++ store_tls_test.go | 98 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 106 insertions(+) create mode 100644 store_tls_test.go diff --git a/store.go b/store.go index dc52920..906e080 100644 --- a/store.go +++ b/store.go @@ -206,6 +206,14 @@ func (o StoreOptions) tlsClientConfig() (*tls.Config, error) { return nil, fmt.Errorf("failed to load client certificate from %s: %w", o.ClientCert, err) } tlsConfig.Certificates = []tls.Certificate{certificate} + // Always present the configured certificate, regardless of the CAs the + // server lists in its CertificateRequest. Go's default selection sends + // no certificate when the list doesn't include the issuer, which breaks + // TLS terminators that advertise a fixed set of public CAs and forward + // the client certificate to the backend for verification. + tlsConfig.GetClientCertificate = func(*tls.CertificateRequestInfo) (*tls.Certificate, error) { + return &certificate, nil + } } // Load custom CA set if provided diff --git a/store_tls_test.go b/store_tls_test.go new file mode 100644 index 0000000..6610b32 --- /dev/null +++ b/store_tls_test.go @@ -0,0 +1,98 @@ +package desync + +import ( + "crypto/ecdsa" + "crypto/elliptic" + "crypto/rand" + "crypto/tls" + "crypto/x509" + "crypto/x509/pkix" + "encoding/pem" + "math/big" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "testing" + "time" + + "github.com/stretchr/testify/require" +) + +// newTestCA returns a self-signed CA certificate and its key. +func newTestCA(t *testing.T, cn string) (*x509.Certificate, *ecdsa.PrivateKey) { + t.Helper() + key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + require.NoError(t, err) + tmpl := &x509.Certificate{ + SerialNumber: big.NewInt(1), + Subject: pkix.Name{CommonName: cn}, + NotBefore: time.Now().Add(-time.Hour), + NotAfter: time.Now().Add(time.Hour), + IsCA: true, + BasicConstraintsValid: true, + KeyUsage: x509.KeyUsageCertSign, + } + der, err := x509.CreateCertificate(rand.Reader, tmpl, tmpl, &key.PublicKey, key) + require.NoError(t, err) + cert, err := x509.ParseCertificate(der) + require.NoError(t, err) + return cert, key +} + +// TestTLSClientCertSentRegardlessOfAcceptableCAs checks that the configured +// client certificate is sent even when the server's CertificateRequest lists +// CAs that did not issue it. Some TLS terminators (for example managed +// ingresses that forward the client certificate to the backend for +// verification) advertise a fixed list of public CAs, and Go's default +// selection would then send no certificate at all. +func TestTLSClientCertSentRegardlessOfAcceptableCAs(t *testing.T) { + clientCA, clientCAKey := newTestCA(t, "private client CA") + unrelatedCA, _ := newTestCA(t, "unrelated public CA") + + // Client certificate issued by the private CA. + clientKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + require.NoError(t, err) + clientTmpl := &x509.Certificate{ + SerialNumber: big.NewInt(2), + Subject: pkix.Name{CommonName: "client"}, + NotBefore: time.Now().Add(-time.Hour), + NotAfter: time.Now().Add(time.Hour), + KeyUsage: x509.KeyUsageDigitalSignature, + ExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageClientAuth}, + } + clientDER, err := x509.CreateCertificate(rand.Reader, clientTmpl, clientCA, &clientKey.PublicKey, clientCAKey) + require.NoError(t, err) + + dir := t.TempDir() + certFile := filepath.Join(dir, "client.crt") + keyFile := filepath.Join(dir, "client.key") + keyDER, err := x509.MarshalECPrivateKey(clientKey) + require.NoError(t, err) + require.NoError(t, os.WriteFile(certFile, pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: clientDER}), 0o600)) + require.NoError(t, os.WriteFile(keyFile, pem.EncodeToMemory(&pem.Block{Type: "EC PRIVATE KEY", Bytes: keyDER}), 0o600)) + + // The server advertises only the unrelated CA in its CertificateRequest + // and requires a certificate without verifying it, like an ingress that + // leaves verification to the backend. + advertised := x509.NewCertPool() + advertised.AddCert(unrelatedCA) + var gotCN string + srv := httptest.NewUnstartedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if len(r.TLS.PeerCertificates) > 0 { + gotCN = r.TLS.PeerCertificates[0].Subject.CommonName + } + })) + srv.TLS = &tls.Config{ClientAuth: tls.RequireAnyClientCert, ClientCAs: advertised} + srv.StartTLS() + defer srv.Close() + + opt := StoreOptions{ClientCert: certFile, ClientKey: keyFile, TrustInsecure: true} + tlsConfig, err := opt.tlsClientConfig() + require.NoError(t, err) + client := &http.Client{Transport: &http.Transport{TLSClientConfig: tlsConfig}} + resp, err := client.Get(srv.URL) + require.NoError(t, err, "client certificate was likely not sent") + resp.Body.Close() + require.Equal(t, "client", gotCN) +} From 8c7f8e0505c2837bcdcf86c0404504eb412bbb0a Mon Sep 17 00:00:00 2001 From: ryusuke Date: Mon, 5 Oct 2026 21:13:53 +0900 Subject: [PATCH 2/2] Check the response body close error in the TLS test golangci-lint's errcheck only exempts deferred Close calls. Co-Authored-By: Claude Opus 5.5 --- store_tls_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/store_tls_test.go b/store_tls_test.go index 6610b32..890a841 100644 --- a/store_tls_test.go +++ b/store_tls_test.go @@ -93,6 +93,6 @@ func TestTLSClientCertSentRegardlessOfAcceptableCAs(t *testing.T) { client := &http.Client{Transport: &http.Transport{TLSClientConfig: tlsConfig}} resp, err := client.Get(srv.URL) require.NoError(t, err, "client certificate was likely not sent") - resp.Body.Close() + require.NoError(t, resp.Body.Close()) require.Equal(t, "client", gotCN) }