Caddy: Issue evaluating placeholder {http.request.tls.client.subject} within a CEL expression

Created on 15 Jul 2020  路  9Comments  路  Source: caddyserver/caddy

When trying to use http.matchers.expression module with Caddy placeholders to evaluate the client subject DN the comparison its failing. An example of the Caddyfile used:

https://hostname:443 {
    bind 10.10.10.10

    log {
        output file access.log {
            roll_size 1gb
            roll_keep 5
            roll_keep_for 720h
        }
    }

    tls tls/hostname-crt.pem tls/hostname-key.pem {
        client_auth {
            mode verify_if_given
            trusted_ca_cert_file tls/ca.pem
        }
    }

    @client_cert {
        expression {http.request.tls.client.subject} == 'CN=bla,OU=blabla,O=bla,L=bla,ST=bleh,C=buh'
    }

    reverse_proxy @client_cert localhost:9191
}

After doing some debugging, it seems that addHTTPVarsToReplacer function return an object instead of a string when replacing the client.subject placeholder.

https://github.com/caddyserver/caddy/blob/77f233a4840afb8b081d12b083b19ed314fd975a/modules/caddyhttp/replacer.go#L324

If Instead of returning the object return cert.Subject, true, we convert it to string return cert.Subject.String(), true the expression expression {http.request.tls.client.subject} == 'CN=bla,OU=blabla,O=bla,L=bla,ST=bleh,C=buh' will return true.

But its possible that I'm missing some detail regarding the CEL expression, may be there is another way to compare the subject DN.

Thank you,
Vitor

feature request

All 9 comments

Yeah, looks like it currently returns the Name struct which doesn't seem particularly useful.

https://golang.org/pkg/crypto/x509/pkix/#Name

@mholt I think it would technically be a breaking change to change this, but I don't expect the impact to be high. Should we consider adding a new placeholder instead that gives the .String() result?

Yeah, looks like it currently returns the Name struct which doesn't seem particularly useful.

Why isn't it useful? That should give you access to all the info about the subject, rather than assuming just one of the fields.

@mholt could you please advice us, how to access to the individual fields of the pkix Name struct within a CEL expression?

Haven't tried this myself, but does using . to dereference the fields not work?

Unfortunately didn't work, I've tried that before open the issue.

expression {http.request.tls.client.subject}.CommonName.contains('stuff')

I've added some logs, to cell_matcher.go

func (m MatchExpression) Match(r *http.Request) bool {
    caddy.Log().Info("### MATCH", zap.String("m.expandedExpr", m.expandedExpr))
    out, _, _ := m.prg.Eval(map[string]interface{}{
        "request": celHTTPRequest{r},
    })

    if outBool, ok := out.Value().(bool); ok {
        return outBool
    }
    return false
}

And got: ### MATCH {"m.expandedExpr": "caddyPlaceholder(request, \"http.request.tls.client.subject\").CommonName.contains('stuff')"}

But the match result was still false.

Edit:

Just added more logs, to get the error from the m.prg.Eval()

############### MATCH {"error": "unsupported type conversion: 'pkix.Name'"}

Ohh, I see. Thanks for looking into that!

I still think it's useful to return the entire struct, but it seems like we need a translation layer to make the fields available in CEL...

Meanwhile do you see any other way to achieve the same result but without using the CEL expression?

@v-rosa if you're willing to write a PR, you can implement CEL support for pkix.Name in celmatcher.go.

Take a look at the stuff with celHTTPRequest and how it maps http.Request to a type that CEL understands. Shouldn't be too difficult.

I've just created a PR. https://github.com/caddyserver/caddy/pull/3608

I've tried to play arround with celHTTPRequest approach for the pkix.Name struct but without success. So I opted to map the pkix.Name as a DefaultType (string) before invoking m.ta.NativeToValue().

If the proposed approach is not suitable I can try to share my pains with the other solution.

Was this page helpful?
0 / 5 - 0 ratings