Caddy not setting PROXYv2 TLV fields in reverse_proxy mode

[eest]

Hello,

I am attempting to use Caddy for TLS termination in front of a Varnish cache server.
The configuration for hooking up Caddy to Varnish looks like this:

reverse_proxy 127.0.0.1:8443 {
        transport http {
                proxy_protocol v2
        }
}

The traffic flow works well, but then I wanted to be able to set headers in Varnish based on TLV attributes in the PROXYv2 packets, looking something like this:

sub vcl_recv {
    if (proxy.is_ssl()) {
      set req.http.X-Forwarded-Proto = "https";
    } else {
      set req.http.X-Forwarded-Proto = "http";
    }
}

This is using the Varnish vmod_proxy to read TLV values: https://varnish-cache.org/docs/trunk/reference/vmod_proxy.html.

I then noticed that Caddy does not appear to set such values, and based on mastercactapus/proxyprotocol#3 it looks like the currently used proxy protocol library does not support setting them either. I have also seen that there appears to be WIP to start using another proxy protocol lib in #5915.

From what I can tell even if the latter PR is merged this would still not set TLV fields, would it make sense to add such code?

[mohammed90]

The PR to replace the lib was influenced by the need to accept connections with TLVs, not adding TLVs for upstreams. It appears to be trivial to add TLVs to the header. The only tricky part is translating the binary "value" (byte) to a decent UI/UX in the Caddyfile and JSON. If you can help me with that, I can try squeezing this requirement into the PR. Do you have any imagination of the UX to set TLVs?

[eest]

Hello @mohammed90, thanks for taking a look at my issue :).

For my needs I don't think we need to be able to set TLV attributes in the configuration file, instead they should be populated based on the request that came in to Caddy. For example, looking at https://www.haproxy.org/download/2.3/doc/proxy-protocol.txt it states:

The PP2_CLIENT_SSL flag indicates that the client connected over SSL/TLS. When
this field is present, the US-ASCII string representation of the TLS version is
appended at the end of the field in the TLV format using the type
PP2_SUBTYPE_SSL_VERSION.

I am not very familiar with the Caddy codebase so apologies if I am missing something, but there appears to be a ProxyProtocolInfo struct at

caddy/modules/caddyhttp/reverseproxy/hosts.go

Lines 242 to 246 in 4173e2c

	// ProxyProtocolInfo contains information needed to write proxy protocol to a
	// connection to an upstream host.
	type ProxyProtocolInfo struct {
	AddrPort netip.AddrPort
	}

And this is populated at

caddy/modules/caddyhttp/reverseproxy/reverseproxy.go

Line 647 in 4173e2c

proxyProtocolInfo := ProxyProtocolInfo{AddrPort: addrPort}

Maybe the struct could be extended with a TLVs []proxyproto.TLV field and then we could create such fields based on the contents of the req *http.Request passed to prepareRequest()

I have spent some time looking at the new proxyproto lib and while I cant find a "*http.Request to []proxyproto.TLV" function, here is an example I came up with regarding how some of the fields could be picked up from the *http.Request (not very tested yet):

package main

import (
        "crypto/tls"
        "fmt"
        "io"
        "log"
        "net/http"

        proxyproto "github.com/pires/go-proxyproto"
        "github.com/pires/go-proxyproto/tlvparse"
)

func main() {
        http.HandleFunc("/", func(w http.ResponseWriter, req *http.Request) {
                if req.TLS != nil {
                        sslTLVData := tlvparse.PP2SSL{
                                // The PP2_CLIENT_SSL flag indicates that the client connected over SSL/TLS. When
                                // this field is present, the US-ASCII string representation of the TLS version is
                                // appended at the end of the field in the TLV format using the type
                                // PP2_SUBTYPE_SSL_VERSION.
                                Client: tlvparse.PP2_BITFIELD_CLIENT_SSL,
                                TLV: []proxyproto.TLV{
                                        {
                                                Type:  proxyproto.PP2_SUBTYPE_SSL_VERSION,
                                                Value: []byte(tls.VersionName(req.TLS.Version)),
                                        },
                                        // The second level TLV PP2_SUBTYPE_SSL_CIPHER provides the US-ASCII string name
                                        // of the used cipher, for example
                                        // "ECDHE-RSA-AES128-GCM-SHA256".
                                        // NOTE: tls.CipherSuiteName() uses
                                        // underscores, not hyphens, in the
                                        // name. This matches the IANA naming:
                                        // https://www.iana.org/assignments/tls-parameters/tls-parameters.xml#tls-parameters-4
                                        {
                                                Type:  proxyproto.PP2_SUBTYPE_SSL_CIPHER,
                                                Value: []byte(tls.CipherSuiteName(req.TLS.CipherSuite)),
                                        },
                                },
                        }

                        if req.TLS.VerifiedChains == nil {
                                fmt.Println("no verified client certs")
                                // The <verify> field will be zero if the client presented a certificate
                                // and it was successfully verified, and non-zero otherwise.
                                sslTLVData.Verify = 1
                        } else {
                                // PP2_CLIENT_CERT_CONN indicates that the client provided a certificate over the
                                // current connection.
                                if !req.TLS.DidResume {
                                        sslTLVData.Client = sslTLVData.Client | tlvparse.PP2_BITFIELD_CLIENT_CERT_CONN
                                }

                                // PP2_CLIENT_CERT_SESS indicates that the client provided a
                                // certificate at least once over the TLS session this connection belongs to.
                                sslTLVData.Client = sslTLVData.Client | tlvparse.PP2_BITFIELD_CLIENT_CERT_SESS

                                // In all cases, the string representation (in UTF8) of the Common Name field
                                // (OID: 2.5.4.3) of the client certificate's Distinguished Name, is appended
                                // using the TLV format and the type PP2_SUBTYPE_SSL_CN. E.g. "example.com".
                                sslTLVData.TLV = append(sslTLVData.TLV, proxyproto.TLV{Type: proxyproto.PP2_SUBTYPE_SSL_CN, Value: []byte(req.TLS.VerifiedChains[0][0].Subject.CommonName)})
                        }

                        sslTLV, err := sslTLVData.Marshal()
                        if err != nil {
                                log.Fatal(err)
                        }

                        tlvs := []proxyproto.TLV{}

                        tlvs = append(tlvs, proxyproto.TLV{Type: proxyproto.PP2_TYPE_ALPN, Value: []byte(req.TLS.NegotiatedProtocol)})
                        tlvs = append(tlvs, proxyproto.TLV{Type: proxyproto.PP2_TYPE_AUTHORITY, Value: []byte(req.TLS.ServerName)})

                        tlvs = append(tlvs, sslTLV)

                        fmt.Println(tlvs)
                }
                io.WriteString(w, "Hello, TLS!\n")
        })

        log.Printf("About to listen on 8443. Go to https://127.0.0.1:8443/")
        err := http.ListenAndServeTLS(":8443", "server.pem", "server.key", nil)
        log.Fatal(err)

[eest]

If we agree on the general direction of the solution I could probably look into opening a PR for adding this to Caddy as well. Of course it would be nice if the WIP to migrate to the new lib could be merged before that :).

[francislavoie]

I don't understand why this is needed. Caddy's proxy already passes through X-Forwarded-Proto correctly: https://caddyserver.com/docs/caddyfile/directives/reverse_proxy#defaults

[eest]

If I am to set things up so HTTPS traffic passes through Caddy and is proxied to Varnish I know Caddy will protect X-Forwarded-Proto from client manipulation, but given that ordinary HTTP traffic is going directly to Varnish there is nothing protecting headers, so a client could set it to "https" or something else for such traffic.

If I can check a TLV attribute to tell if the request came in via TLS then I can trust this is correct since a client would not be able to set the contents of the proxy protocol header. If I don't have access to that I will need to fall back to checking ports to decide if a given header can be considered safe or not.

It also appears to me there is potentially more information available in TLV attributes than then protected headers which can be usable in the backend system.