Repository: caddyserver/caddy (language: go)

Issue:
reverse proxy: use the new CA providers
The PR #5784 introduces modular CA providers. The HTTP(S) transport of the reverse proxy module has a copy of the older approach which was removed in the PR #5784. Moreover, the PR included copying the `TLSConfig` struct with minor modifications to.

In the PR, I opted not to touch the reverse-proxy transport to minimize the impact and reduce the complexity of the PR by leaving it focused to the `caddytls` package. Now that the PR is merged, the reverse-proxy module can re-use the logic and introduced modules to have flexible source of certificate-authority pool. Implementing this means the reverse-proxy transport can trust certificates from, e.g., distributed certificate-authority providers shared over distributed storage or other Caddy instances.

One powerful use case of this is when a Caddy instance acts as an ACME server for the internal services, and other caddy instances serving as load-balancers will trust the same certificate authorities used by the caddy ACME server.

The change for this issue requires looking into, at least, the following parts of the `reverseproxy` package:

https://github.com/caddyserver/caddy/blob/b9c40e711115d35c2809313aeec42ab10fcd6914/modules/caddyhttp/reverseproxy/httptransport.go#L55-L57

https://github.com/caddyserver/caddy/blob/b9c40e711115d35c2809313aeec42ab10fcd6914/modules/caddyhttp/reverseproxy/httptransport.go#L502-L601


Discussion / hints:
I am trying out a possible implementation. Impressive work on the linked PR btw! 👍  

Fix patch:
diff --git a/modules/caddyhttp/reverseproxy/caddyfile.go b/modules/caddyhttp/reverseproxy/caddyfile.go
index 42fd6f99d65..05f551f84ab 100644
--- a/modules/caddyhttp/reverseproxy/caddyfile.go
+++ b/modules/caddyhttp/reverseproxy/caddyfile.go
@@ -30,6 +30,7 @@ import (
 	"github.com/caddyserver/caddy/v2/modules/caddyhttp"
 	"github.com/caddyserver/caddy/v2/modules/caddyhttp/headers"
 	"github.com/caddyserver/caddy/v2/modules/caddyhttp/rewrite"
+	"github.com/caddyserver/caddy/v2/modules/caddytls"
 )
 
 func init() {
@@ -1103,6 +1104,9 @@ func (h *HTTPTransport) UnmarshalCaddyfile(d *caddyfile.Dispenser) error {
 			if h.TLS == nil {
 				h.TLS = new(TLSConfig)
 			}
+			if len(h.TLS.CARaw) != 0 {
+				return d.Err("cannot specify both 'tls_trust_pool' and 'tls_trusted_ca_certs")
+			}
 			h.TLS.RootCAPEMFiles = args
 
 		case "tls_server_name":
@@ -1218,6 +1222,31 @@ func (h *HTTPTransport) UnmarshalCaddyfile(d *caddyfile.Dispenser) error {
 			}
 			h.MaxConnsPerHost = num
 
+		case "tls_trust_pool":
+			if !d.NextArg() {
+				return d.ArgErr()
+			}
+			modStem := d.Val()
+			modID := "tls.ca_pool.source." + modStem
+			unm, err := caddyfile.UnmarshalModule(d, modID)
+			if err != nil {
+				return err
+			}
+			ca, ok := unm.(caddytls.CA)
+			if !ok {
+				return d.Errf("module %s is not a caddytls.CA", modID)
+			}
+			if h.TLS == nil {
+				h.TLS = new(TLSConfig)
+			}
+			if len(h.TLS.RootCAPEMFiles) != 0 {
+				return d.Err("cannot specify both 'tls_trust_pool' and 'tls_trusted_ca_certs'")
+			}
+			if h.TLS.CARaw != nil {
+				return d.Err("cannot specify \"tls_trust_pool\" twice in caddyfile")
+			}
+			h.TLS.CARaw = caddyconfig.JSONModuleObject(ca, "provider", modStem, nil)
+
 		default:
 			return d.Errf("unrecognized subdirective %s", d.Val())
 		}
diff --git a/modules/caddyhttp/reverseproxy/httptransport.go b/modules/caddyhttp/reverseproxy/httptransport.go
index 0a803a83a0e..6249e04d687 100644
--- a/modules/caddyhttp/reverseproxy/httptransport.go
+++ b/modules/caddyhttp/reverseproxy/httptransport.go
@@ -19,6 +19,7 @@ import (
 	"crypto/tls"
 	"crypto/x509"
 	"encoding/base64"
+	"encoding/json"
 	"fmt"
 	weakrand "math/rand"
 	"net"
@@ -442,9 +443,14 @@ func (h HTTPTransport) Cleanup() error {
 // TLSConfig holds configuration related to the TLS configuration for the
 // transport/client.
 type TLSConfig struct {
+	// Certificate authority module which provides the certificate pool of trusted certificates
+	CARaw json.RawMessage `json:"ca,omitempty" caddy:"namespace=tls.ca_pool.source inline_key=provider"`
+
+	// DEPRECATED: Use the `ca` field with the `tls.ca_pool.source.inline` module instead.
 	// Optional list of base64-encoded DER-encoded CA certificates to trust.
 	RootCAPool []string `json:"root_ca_pool,omitempty"`
 
+	// DEPRECATED: Use the `ca` field with the `tls.ca_pool.source.file` module instead.
 	// List of PEM-encoded CA certificate files to add to the same trust
 	// store as RootCAPool (or root_ca_pool in the JSON).
 	RootCAPEMFiles []string `json:"root_ca_pem_files,omitempty"`
@@ -546,6 +552,7 @@ func (t TLSConfig) MakeTLSClientConfig(ctx caddy.Context) (*tls.Config, error) {
 
 	// trusted root CAs
 	if len(t.RootCAPool) > 0 || len(t.RootCAPEMFiles) > 0 {
+		ctx.Logger().Warn("root_ca_pool and root_ca_pem_files are deprecated. Use one of the tls.ca_pool.source modules instead")
 		rootPool := x509.NewCertPool()
 		for _, encodedCACert := range t.RootCAPool {
 			caCert, err := decodeBase64DERCert(encodedCACert)
@@ -564,6 +571,21 @@ func (t TLSConfig) MakeTLSClientConfig(ctx caddy.Context) (*tls.Config, error) {
 		cfg.RootCAs = rootPool
 	}
 
+	if t.CARaw != nil {
+		if len(t.RootCAPool) > 0 || len(t.RootCAPEMFiles) > 0 {
+			return nil, fmt.Errorf("conflicting config for Root CA pool")
+		}
+		caRaw, err := ctx.LoadModule(t, "CARaw")
+		if err != nil {
+			return nil, fmt.Errorf("failed to load ca module: %v", err)
+		}
+		ca, ok := caRaw.(caddytls.CA)
+		if !ok {
+			return nil, fmt.Errorf("CA module '%s' is not a certificate pool provider", ca)
+		}
+		cfg.RootCAs = ca.CertPool()
+	}
+
 	// Renegotiation
 	switch t.Renegotiation {
 	case "never", "":


Test patch:
diff --git a/caddytest/integration/caddyfile_adapt/reverse_proxy_http_transport_tls_file_cert.txt b/caddytest/integration/caddyfile_adapt/reverse_proxy_http_transport_tls_file_cert.txt
new file mode 100644
index 00000000000..d43aa1175c3
--- /dev/null
+++ b/caddytest/integration/caddyfile_adapt/reverse_proxy_http_transport_tls_file_cert.txt
@@ -0,0 +1,47 @@
+:8884
+reverse_proxy 127.0.0.1:65535 {
+	transport http {
+		tls_trust_pool file {
+			pem_file ../caddy.ca.cer
+		}
+	}
+}
+----------
+{
+	"apps": {
+		"http": {
+			"servers": {
+				"srv0": {
+					"listen": [
+						":8884"
+					],
+					"routes": [
+						{
+							"handle": [
+								{
+									"handler": "reverse_proxy",
+									"transport": {
+										"protocol": "http",
+										"tls": {
+											"ca": {
+												"pem_files": [
+													"../caddy.ca.cer"
+												],
+												"provider": "file"
+											}
+										}
+									},
+									"upstreams": [
+										{
+											"dial": "127.0.0.1:65535"
+										}
+									]
+								}
+							]
+						}
+					]
+				}
+			}
+		}
+	}
+}
diff --git a/caddytest/integration/caddyfile_adapt/reverse_proxy_http_transport_tls_inline_cert.txt b/caddytest/integration/caddyfile_adapt/reverse_proxy_http_transport_tls_inline_cert.txt
new file mode 100644
index 00000000000..ef9e8243af6
--- /dev/null
+++ b/caddytest/integration/caddyfile_adapt/reverse_proxy_http_transport_tls_inline_cert.txt
@@ -0,0 +1,47 @@
+:8884
+reverse_proxy 127.0.0.1:65535 {
+	transport http {
+		tls_trust_pool inline {
+			trust_der MIIDSzCCAjOgAwIBAgIUfIRObjWNUA4jxQ/0x8BOCvE2Vw4wDQYJKoZIhvcNAQELBQAwFjEUMBIGA1UEAwwLRWFzeS1SU0EgQ0EwHhcNMTkwODI4MTYyNTU5WhcNMjkwODI1MTYyNTU5WjAWMRQwEgYDVQQDDAtFYXN5LVJTQSBDQTCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEBAK5m5elxhQfMp/3aVJ4JnpN9PUSz6LlP6LePAPFU7gqohVVFVtDkChJAG3FNkNQNlieVTja/bgH9IcC6oKbROwdY1h0MvNV8AHHigvl03WuJD8g2ReVFXXwsnrPmKXCFzQyMI6TYk3m2gYrXsZOU1GLnfMRC3KAMRgE2F45twOs9hqG169YJ6mM2eQjzjCHWI6S2/iUYvYxRkCOlYUbLsMD/AhgAf1plzg6LPqNxtdlwxZnA0ytgkmhK67HtzJu0+ovUCsMv0RwcMhsEo9T8nyFAGt9XLZ63X5WpBCTUApaAUhnG0XnerjmUWb6eUWw4zev54sEfY5F3x002iQaW6cECAwEAAaOBkDCBjTAdBgNVHQ4EFgQU4CBUbZsS2GaNIkGRz/cBsD5ivjswUQYDVR0jBEowSIAU4CBUbZsS2GaNIkGRz/cBsD5ivjuhGqQYMBYxFDASBgNVBAMMC0Vhc3ktUlNBIENBghR8hE5uNY1QDiPFD/THwE4K8TZXDjAMBgNVHRMEBTADAQH/MAsGA1UdDwQEAwIBBjANBgkqhkiG9w0BAQsFAAOCAQEAKB3V4HIzoiO/Ch6WMj9bLJ2FGbpkMrcb/Eq01hT5zcfKD66lVS1MlK+cRL446Z2b2KDP1oFyVs+qmrmtdwrWgD+nfe2sBmmIHo9m9KygMkEOfG3MghGTEcS+0cTKEcoHYWYyOqQh6jnedXY8Cdm4GM1hAc9MiL3/sqV8YCVSLNnkoNysmr06/rZ0MCUZPGUtRmfd0heWhrfzAKw2HLgX+RAmpOE2MZqWcjvqKGyaRiaZks4nJkP6521aC2Lgp0HhCz1j8/uQ5ldoDszCnu/iro0NAsNtudTMD+YoLQxLqdleIh6CW+illc2VdXwj7mn6J04yns9jfE2jRjW/yTLFuQ==
+		}
+	}
+}
+----------
+{
+	"apps": {
+		"http": {
+			"servers": {
+				"srv0": {
+					"listen": [
+						":8884"
+					],
+					"routes": [
+						{
+							"handle": [
+								{
+									"handler": "reverse_proxy",
+									"transport": {
+										"protocol": "http",
+										"tls": {
+											"ca": {
+												"provider": "inline",
+												"trusted_ca_certs": [
+													"MIIDSzCCAjOgAwIBAgIUfIRObjWNUA4jxQ/0x8BOCvE2Vw4wDQYJKoZIhvcNAQELBQAwFjEUMBIGA1UEAwwLRWFzeS1SU0EgQ0EwHhcNMTkwODI4MTYyNTU5WhcNMjkwODI1MTYyNTU5WjAWMRQwEgYDVQQDDAtFYXN5LVJTQSBDQTCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEBAK5m5elxhQfMp/3aVJ4JnpN9PUSz6LlP6LePAPFU7gqohVVFVtDkChJAG3FNkNQNlieVTja/bgH9IcC6oKbROwdY1h0MvNV8AHHigvl03WuJD8g2ReVFXXwsnrPmKXCFzQyMI6TYk3m2gYrXsZOU1GLnfMRC3KAMRgE2F45twOs9hqG169YJ6mM2eQjzjCHWI6S2/iUYvYxRkCOlYUbLsMD/AhgAf1plzg6LPqNxtdlwxZnA0ytgkmhK67HtzJu0+ovUCsMv0RwcMhsEo9T8nyFAGt9XLZ63X5WpBCTUApaAUhnG0XnerjmUWb6eUWw4zev54sEfY5F3x002iQaW6cECAwEAAaOBkDCBjTAdBgNVHQ4EFgQU4CBUbZsS2GaNIkGRz/cBsD5ivjswUQYDVR0jBEowSIAU4CBUbZsS2GaNIkGRz/cBsD5ivjuhGqQYMBYxFDASBgNVBAMMC0Vhc3ktUlNBIENBghR8hE5uNY1QDiPFD/THwE4K8TZXDjAMBgNVHRMEBTADAQH/MAsGA1UdDwQEAwIBBjANBgkqhkiG9w0BAQsFAAOCAQEAKB3V4HIzoiO/Ch6WMj9bLJ2FGbpkMrcb/Eq01hT5zcfKD66lVS1MlK+cRL446Z2b2KDP1oFyVs+qmrmtdwrWgD+nfe2sBmmIHo9m9KygMkEOfG3MghGTEcS+0cTKEcoHYWYyOqQh6jnedXY8Cdm4GM1hAc9MiL3/sqV8YCVSLNnkoNysmr06/rZ0MCUZPGUtRmfd0heWhrfzAKw2HLgX+RAmpOE2MZqWcjvqKGyaRiaZks4nJkP6521aC2Lgp0HhCz1j8/uQ5ldoDszCnu/iro0NAsNtudTMD+YoLQxLqdleIh6CW+illc2VdXwj7mn6J04yns9jfE2jRjW/yTLFuQ=="
+												]
+											}
+										}
+									},
+									"upstreams": [
+										{
+											"dial": "127.0.0.1:65535"
+										}
+									]
+								}
+							]
+						}
+					]
+				}
+			}
+		}
+	}
+}
diff --git a/modules/caddyhttp/reverseproxy/httptransport_test.go b/modules/caddyhttp/reverseproxy/httptransport_test.go
new file mode 100644
index 00000000000..46931c8b17c
--- /dev/null
+++ b/modules/caddyhttp/reverseproxy/httptransport_test.go
@@ -0,0 +1,96 @@
+package reverseproxy
+
+import (
+	"encoding/json"
+	"fmt"
+	"reflect"
+	"testing"
+
+	"github.com/caddyserver/caddy/v2/caddyconfig/caddyfile"
+)
+
+func TestHTTPTransportUnmarshalCaddyFileWithCaPools(t *testing.T) {
+	const test_der_1 = `MIIDSzCCAjOgAwIBAgIUfIRObjWNUA4jxQ/0x8BOCvE2Vw4wDQYJKoZIhvcNAQELBQAwFjEUMBIGA1UEAwwLRWFzeS1SU0EgQ0EwHhcNMTkwODI4MTYyNTU5WhcNMjkwODI1MTYyNTU5WjAWMRQwEgYDVQQDDAtFYXN5LVJTQSBDQTCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEBAK5m5elxhQfMp/3aVJ4JnpN9PUSz6LlP6LePAPFU7gqohVVFVtDkChJAG3FNkNQNlieVTja/bgH9IcC6oKbROwdY1h0MvNV8AHHigvl03WuJD8g2ReVFXXwsnrPmKXCFzQyMI6TYk3m2gYrXsZOU1GLnfMRC3KAMRgE2F45twOs9hqG169YJ6mM2eQjzjCHWI6S2/iUYvYxRkCOlYUbLsMD/AhgAf1plzg6LPqNxtdlwxZnA0ytgkmhK67HtzJu0+ovUCsMv0RwcMhsEo9T8nyFAGt9XLZ63X5WpBCTUApaAUhnG0XnerjmUWb6eUWw4zev54sEfY5F3x002iQaW6cECAwEAAaOBkDCBjTAdBgNVHQ4EFgQU4CBUbZsS2GaNIkGRz/cBsD5ivjswUQYDVR0jBEowSIAU4CBUbZsS2GaNIkGRz/cBsD5ivjuhGqQYMBYxFDASBgNVBAMMC0Vhc3ktUlNBIENBghR8hE5uNY1QDiPFD/THwE4K8TZXDjAMBgNVHRMEBTADAQH/MAsGA1UdDwQEAwIBBjANBgkqhkiG9w0BAQsFAAOCAQEAKB3V4HIzoiO/Ch6WMj9bLJ2FGbpkMrcb/Eq01hT5zcfKD66lVS1MlK+cRL446Z2b2KDP1oFyVs+qmrmtdwrWgD+nfe2sBmmIHo9m9KygMkEOfG3MghGTEcS+0cTKEcoHYWYyOqQh6jnedXY8Cdm4GM1hAc9MiL3/sqV8YCVSLNnkoNysmr06/rZ0MCUZPGUtRmfd0heWhrfzAKw2HLgX+RAmpOE2MZqWcjvqKGyaRiaZks4nJkP6521aC2Lgp0HhCz1j8/uQ5ldoDszCnu/iro0NAsNtudTMD+YoLQxLqdleIh6CW+illc2VdXwj7mn6J04yns9jfE2jRjW/yTLFuQ==`
+	type args struct {
+		d *caddyfile.Dispenser
+	}
+	tests := []struct {
+		name              string
+		args              args
+		expectedTLSConfig TLSConfig
+		wantErr           bool
+	}{
+		{
+			name: "tls_trust_pool without a module argument returns an error",
+			args: args{
+				d: caddyfile.NewTestDispenser(
+					`http {
+					tls_trust_pool
+				}`),
+			},
+			wantErr: true,
+		},
+		{
+			name: "providing both 'tls_trust_pool' and 'tls_trusted_ca_certs' returns an error",
+			args: args{
+				d: caddyfile.NewTestDispenser(fmt.Sprintf(
+					`http {
+					tls_trust_pool inline %s
+					tls_trusted_ca_certs %s
+				}`, test_der_1, test_der_1)),
+			},
+			wantErr: true,
+		},
+		{
+			name: "setting 'tls_trust_pool' and 'tls_trusted_ca_certs' produces an error",
+			args: args{
+				d: caddyfile.NewTestDispenser(fmt.Sprintf(
+					`http {
+					tls_trust_pool inline {
+						trust_der	%s
+					}
+					tls_trusted_ca_certs %s
+				}`, test_der_1, test_der_1)),
+			},
+			wantErr: true,
+		},
+		{
+			name: "using 'inline' tls_trust_pool loads the module successfully",
+			args: args{
+				d: caddyfile.NewTestDispenser(fmt.Sprintf(
+					`http {
+						tls_trust_pool inline {
+							trust_der	%s
+						}
+					}
+				`, test_der_1)),
+			},
+			expectedTLSConfig: TLSConfig{CARaw: json.RawMessage(fmt.Sprintf(`{"provider":"inline","trusted_ca_certs":["%s"]}`, test_der_1))},
+		},
+		{
+			name: "setting 'tls_trusted_ca_certs' and 'tls_trust_pool' produces an error",
+			args: args{
+				d: caddyfile.NewTestDispenser(fmt.Sprintf(
+					`http {
+						tls_trusted_ca_certs %s
+						tls_trust_pool inline {
+							trust_der	%s
+						}
+				}`, test_der_1, test_der_1)),
+			},
+			wantErr: true,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			ht := &HTTPTransport{}
+			if err := ht.UnmarshalCaddyfile(tt.args.d); (err != nil) != tt.wantErr {
+				t.Errorf("HTTPTransport.UnmarshalCaddyfile() error = %v, wantErr %v", err, tt.wantErr)
+				return
+			}
+			if !tt.wantErr && !reflect.DeepEqual(&tt.expectedTLSConfig, ht.TLS) {
+				t.Errorf("HTTPTransport.UnmarshalCaddyfile() = %v, want %v", ht, tt.expectedTLSConfig)
+			}
+		})
+	}
+}