Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[server/identifier] Support SAN inspection in system.authz policies #5538

Closed
charlieegan3 opened this issue Jan 4, 2023 · 0 comments · Fixed by #5742
Closed

[server/identifier] Support SAN inspection in system.authz policies #5538

charlieegan3 opened this issue Jan 4, 2023 · 0 comments · Fixed by #5742

Comments

@charlieegan3
Copy link
Contributor

What is the underlying problem you're trying to solve?

OPA has a functionality which allows some details about the client certificate to be used in authorization policy. (https://github.com/open-policy-agent/opa/blame/ac64e78c6979ce9cba161d94e69b62d477f74f3b/server/identifier/tls.go#L26)

The information currently available is in the Distinguished Names Syntax, e.g.

CN=Charlie Egan,O=Styra,C=GB

I would like to be able to write OPA server authorization policy that uses the full certificate in the authorization decision. Currently, the only information available is the subject of the certificate, not the other data, such as SANs for example. It's now common to use SANs to encode identity information (SPIFFE/Istio, etc).

The code below shows a certificate with a SAN URI which is not shown in the Subject.ToRDNSequence().String() data we use today. (go playground link: https://go.dev/play/p/cY574CIc9y2)



```go
package main

import (
	"crypto/x509"
	"encoding/pem"
	"fmt"
)

func main() {
	certPEM := `-----BEGIN CERTIFICATE-----
MIIC9jCCAd6gAwIBAgIJAKz5n9DYsEw6MA0GCSqGSIb3DQEBBQUAMBAxDjAMBgNV
BAMMBW15LWNhMB4XDTIzMDEwNDE3NTMwOVoXDTI1MDkzMDE3NTMwOVowFDESMBAG
A1UEAwwJbXktc2VydmVyMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEA
6O9HrkSF5AnyNCMIGcLnf616MFvR7Kr367aWzyDOXsD478YrmFPH0vh/0Ij3quMi
V/LbMlFfQrVplnTN8XSG3yVPZUkQHfCKvRt5MFrYUWnu8YTd7dbDj5MssBhEuHpt
jK3BFiY6rFk37Xp/4w9RLAUUdKlUtjphVi+og2BirEG5+Eilf3EGdJLr14q4QIid
z6EzOBjSIRUmlFfYcXC230Xe25cfl2GgZ0aUH496NzTZMx1TeDyG/J8X5Jg0jwAF
WghyKoQvjCgOMQLDlY3zaBlU8US/9xyr2QJfaJm7fSF2+byB2XB7w5KK8tLSf/kQ
2f4q3hW+8rsTT2HndMxDbQIDAQABo08wTTAJBgNVHRMEAjAAMAsGA1UdDwQEAwIF
4DAzBgNVHREELDAqhihzcGlmZmU6Ly9leGFtcGxlLmNvbS93b3JrbG9hZC1pZGVu
dGlmaWVyMA0GCSqGSIb3DQEBBQUAA4IBAQBqY2rLViClK54i10Hc8l07ONDDBRRw
OXf691ZcxbjXytBDTFIG6bR5IW5Ewhpl/kwypKVX/wGn+2XoOWR1XyI4bxcvBzHG
KuXUELiXp+v7CT2vLYQCRfBrJecOociZ8u9W/HTUjMrN1j0dZWFrzYdmLaUmbtzO
KExscgpixq0W1EAW6/4Oi18aD8OBcZ2B8D3ISaP86nYTAGMcPX/n831TO8FxAcYj
9SU+WvEVcf07zwL1GE3NwR6VzihoGeNpFKzKyTXhl3S6HHArZVnXeHvQNbqqrFls
n2YWvV4aPk/VgXQs6Kvp3jptWd7Kikvs19pKlLmTXHIfIYoDlXUlUY6N
-----END CERTIFICATE-----

`

	block, _ := pem.Decode([]byte(certPEM))
	cert, _ := x509.ParseCertificate(block.Bytes)

	fmt.Println("current OPA identity string: ", cert.Subject.ToRDNSequence().String())

	fmt.Println("example of missing data (SANs URI): ", cert.URIs)

}

Describe the ideal solution

We deprecate --authentication=tls with a warning and drop support in some far future version. We add a new mode, --authentication=x509 or --authentication=client-certificate. With this new option, input.identity would be bound to the JSON representation of the client certificate, e.g.:

d, _ := json.MarshalIndent(cert, "", "  ")

This is what the Rego built-in crypto.x509.parse_certificates uses (ref).

Describe a "Good Enough" solution

A new client_certificate key is added to the authz input which contains the certificate data. We leave identity unchanged for backwards compatibility.

Additional Context

This is only useful for users who terminate TLS as OPA instances. Otherwise, forwarded headers containing the client certificate would be more suitable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

2 participants