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

Implement annotations proxy-ssl-client-secret and proxy-ssl-ca-configmap #12469

Draft
wants to merge 12 commits into
base: main
Choose a base branch
from
2 changes: 2 additions & 0 deletions docs/user-guide/nginx-configuration/annotations-risk.md
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,9 @@
| Proxy | proxy-redirect-to | Medium | location |
| Proxy | proxy-request-buffering | Low | location |
| Proxy | proxy-send-timeout | Low | location |
| ProxySSL | proxy-ssl-ca-configmap | Medium | ingress |
| ProxySSL | proxy-ssl-ciphers | Medium | ingress |
| ProxySSL | proxy-ssl-client-secret | Medium | ingress |
| ProxySSL | proxy-ssl-name | High | ingress |
| ProxySSL | proxy-ssl-protocols | Low | ingress |
| ProxySSL | proxy-ssl-secret | Medium | ingress |
Expand Down
103 changes: 88 additions & 15 deletions internal/ingress/annotations/proxyssl/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,13 +45,15 @@ var (
)

const (
proxySSLSecretAnnotation = "proxy-ssl-secret"
proxySSLCiphersAnnotation = "proxy-ssl-ciphers"
proxySSLProtocolsAnnotation = "proxy-ssl-protocols"
proxySSLNameAnnotation = "proxy-ssl-name"
proxySSLVerifyAnnotation = "proxy-ssl-verify"
proxySSLVerifyDepthAnnotation = "proxy-ssl-verify-depth"
proxySSLServerNameAnnotation = "proxy-ssl-server-name"
proxySSLSecretAnnotation = "proxy-ssl-secret" // DEPRECATED Use proxy-ssl-client-secret and proxy-ssl-ca-configmap instead
proxySSLClientSecretAnnotation = "proxy-ssl-client-secret" // #nosec
proxySSLCAConfigMapAnnotation = "proxy-ssl-ca-configmap"
proxySSLCiphersAnnotation = "proxy-ssl-ciphers"
proxySSLProtocolsAnnotation = "proxy-ssl-protocols"
proxySSLNameAnnotation = "proxy-ssl-name"
proxySSLVerifyAnnotation = "proxy-ssl-verify"
proxySSLVerifyDepthAnnotation = "proxy-ssl-verify-depth"
proxySSLServerNameAnnotation = "proxy-ssl-server-name"
)

var proxySSLAnnotation = parser.Annotation{
Expand All @@ -61,11 +63,30 @@ var proxySSLAnnotation = parser.Annotation{
Validator: parser.ValidateRegex(parser.BasicCharsRegex, true),
Scope: parser.AnnotationScopeIngress,
Risk: parser.AnnotationRiskMedium,
Documentation: `This annotation specifies a Secret with the certificate tls.crt, key tls.key in PEM format used for authentication to a proxied HTTPS server.
Documentation: `(DEPRECATED: Use proxy-ssl-client-secret and proxy-ssl-ca-configmap instead)
This annotation specifies a Secret with the certificate tls.crt, key tls.key in PEM format used for authentication to a proxied HTTPS server.
It should also contain trusted CA certificates ca.crt in PEM format used to verify the certificate of the proxied HTTPS server.
This annotation expects the Secret name in the form "namespace/secretName"
Just secrets on the same namespace of the ingress can be used.`,
},
proxySSLClientSecretAnnotation: {
Validator: parser.ValidateRegex(parser.BasicCharsRegex, true),
Scope: parser.AnnotationScopeIngress,
Risk: parser.AnnotationRiskMedium,
Documentation: `This annotation specifies a Secret with the certificate tls.crt, key tls.key in PEM format used for authentication to a proxied HTTPS server.
If the annotation proxy-ssl-secret is also present, the tls.crt and tls.key from this secret will take precedence.
This annotation expects the Secret name in the form "namespace/secretName"
Just secrets on the same namespace of the ingress can be used.`,
},
proxySSLCAConfigMapAnnotation: {
Validator: parser.ValidateRegex(parser.BasicCharsRegex, true),
Scope: parser.AnnotationScopeIngress,
Risk: parser.AnnotationRiskMedium,
Documentation: `This annotation specifies a ConfigMap with the trusted CA certificates ca.crt in PEM format used to verify the certificate of the proxied HTTPS server.
If the annotation proxy-ssl-secret is also present, ca tls.crt and ca.clr (revocation list) from this configMap will take precedence.
This annotation expects the ConfigMap name in the form "namespace/configMapName"
Just configMaps on the same namespace of the ingress can be used.`,
},
proxySSLCiphersAnnotation: {
Validator: parser.ValidateRegex(proxySSLCiphersRegex, true),
Scope: parser.AnnotationScopeIngress,
Expand Down Expand Up @@ -107,16 +128,18 @@ var proxySSLAnnotation = parser.Annotation{
},
}

// Config contains the AuthSSLCert used for mutual authentication
// Config contains the Proxy SSL certificates and CAs used for mutual authentication
// and the configured VerifyDepth
type Config struct {
resolver.AuthSSLCert
Ciphers string `json:"ciphers"`
Protocols string `json:"protocols"`
ProxySSLName string `json:"proxySSLName"`
Verify string `json:"verify"`
VerifyDepth int `json:"verifyDepth"`
ProxySSLServerName string `json:"proxySSLServerName"`
ProxySSLClientCert resolver.SSLClientCert `json:"proxySSLClientCert"`
ProxySSLCA resolver.SSLCA `json:"proxySSLCA"`
Ciphers string `json:"ciphers"`
Protocols string `json:"protocols"`
ProxySSLName string `json:"proxySSLName"`
Verify string `json:"verify"`
VerifyDepth int `json:"verifyDepth"`
ProxySSLServerName string `json:"proxySSLServerName"`
}

// Equal tests for equality between two Config types
Expand All @@ -130,6 +153,12 @@ func (pssl1 *Config) Equal(pssl2 *Config) bool {
if !(&pssl1.AuthSSLCert).Equal(&pssl2.AuthSSLCert) {
return false
}
if !(&pssl1.ProxySSLClientCert).Equal(&pssl2.ProxySSLClientCert) {
return false
}
if !(&pssl1.ProxySSLCA).Equal(&pssl2.ProxySSLCA) {
return false
}
if pssl1.Ciphers != pssl2.Ciphers {
return false
}
Expand Down Expand Up @@ -212,6 +241,50 @@ func (p proxySSL) Parse(ing *networking.Ingress) (interface{}, error) {
}
config.AuthSSLCert = *proxyCert

proxysslclientsecret, err := parser.GetStringAnnotation(proxySSLClientSecretAnnotation, ing, p.annotationConfig.Annotations)
if err != nil {
return &Config{}, err
}

ns, _, err = k8s.ParseNameNS(proxysslclientsecret)
if err != nil {
return &Config{}, ing_errors.NewLocationDenied(err.Error())
}

// We don't accept different namespaces for secrets.
if !secCfg.AllowCrossNamespaceResources && ns != ing.Namespace {
return &Config{}, ing_errors.NewLocationDenied("cross namespace secrets are not supported")
}

sslClientCert, err := p.r.GetSSLClientCert(proxysslclientsecret)
if err != nil {
e := fmt.Errorf("error obtaining ssl client certificate: %w", err)
return &Config{}, ing_errors.LocationDeniedError{Reason: e}
}
config.ProxySSLClientCert = *sslClientCert

proxysslcaconfigmap, err := parser.GetStringAnnotation(proxySSLCAConfigMapAnnotation, ing, p.annotationConfig.Annotations)
if err != nil {
return &Config{}, err
}

ns, _, err = k8s.ParseNameNS(proxysslcaconfigmap)
if err != nil {
return &Config{}, ing_errors.NewLocationDenied(err.Error())
}

// We don't accept different namespaces for configmaps.
if !secCfg.AllowCrossNamespaceResources && ns != ing.Namespace {
return &Config{}, ing_errors.NewLocationDenied("cross namespace configmaps are not supported")
}

sslCA, err := p.r.GetSSLCA(proxysslcaconfigmap)
if err != nil {
e := fmt.Errorf("error obtaining ssl certificate authority: %w", err)
return &Config{}, ing_errors.LocationDeniedError{Reason: e}
}
config.ProxySSLCA = *sslCA

config.Ciphers, err = parser.GetStringAnnotation(proxySSLCiphersAnnotation, ing, p.annotationConfig.Annotations)
if err != nil {
if ing_errors.IsValidationError(err) {
Expand Down
56 changes: 51 additions & 5 deletions internal/ingress/annotations/proxyssl/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,12 @@ import (
)

const (
defaultDemoSecret = "default/demo-secret"
proxySslCiphers = "HIGH:-SHA"
off = "off"
sslServerName = "w00t"
defaultProtocol = "TLSv1.2 TLSv1.3"
defaultDemoSecret = "default/demo-secret"
defaultDemoConfigMap = "default/demo-configmap"
proxySslCiphers = "HIGH:-SHA"
off = "off"
sslServerName = "w00t"
defaultProtocol = "TLSv1.2 TLSv1.3"
)

func buildIngress() *networking.Ingress {
Expand Down Expand Up @@ -96,11 +97,37 @@ func (m mockSecret) GetAuthCertificate(name string) (*resolver.AuthSSLCert, erro
}, nil
}

// GetSSLClientCert resolves a given secret name into an SSL certificate.
func (m mockSecret) GetSSLClientCert(name string) (*resolver.SSLClientCert, error) {
if name != defaultDemoSecret {
return nil, errors.Errorf("there is no secret with name %v", name)
}

return &resolver.SSLClientCert{
Secret: defaultDemoSecret,
}, nil
}

// GetSSLCA resolves a given configMap name into an SSL CA.
func (m mockSecret) GetSSLCA(name string) (*resolver.SSLCA, error) {
if name != defaultDemoConfigMap {
return nil, errors.Errorf("there is no configmap with name %v", name)
}

return &resolver.SSLCA{
ConfigMap: defaultDemoConfigMap,
CAFileName: "/ssl/ca.crt",
CASHA: "abc",
}, nil
}

func TestAnnotations(t *testing.T) {
ing := buildIngress()
data := map[string]string{}

data[parser.GetAnnotationWithPrefix(proxySSLSecretAnnotation)] = defaultDemoSecret
data[parser.GetAnnotationWithPrefix(proxySSLClientSecretAnnotation)] = defaultDemoSecret
data[parser.GetAnnotationWithPrefix(proxySSLCAConfigMapAnnotation)] = defaultDemoConfigMap
data[parser.GetAnnotationWithPrefix("proxy-ssl-ciphers")] = proxySslCiphers
data[parser.GetAnnotationWithPrefix("proxy-ssl-name")] = "$host"
data[parser.GetAnnotationWithPrefix("proxy-ssl-protocols")] = "TLSv1.3 TLSv1.2"
Expand All @@ -126,10 +153,24 @@ func TestAnnotations(t *testing.T) {
if err != nil {
t.Errorf("unexpected error getting secret %v", err)
}
clientSecret, err := fakeSecret.GetSSLClientCert(defaultDemoSecret)
if err != nil {
t.Errorf("unexpected error getting secret %v", err)
}
configMap, err := fakeSecret.GetSSLCA(defaultDemoConfigMap)
if err != nil {
t.Errorf("unexpected error getting configmap %v", err)
}

if u.AuthSSLCert.Secret != secret.Secret {
t.Errorf("expected %v but got %v", secret.Secret, u.AuthSSLCert.Secret)
}
if u.ProxySSLClientCert.Secret != clientSecret.Secret {
t.Errorf("expected %v but got %v", secret.Secret, u.AuthSSLCert.Secret)
}
if u.ProxySSLCA.ConfigMap != configMap.ConfigMap {
t.Errorf("expected %v but got %v", secret.Secret, u.AuthSSLCert.Secret)
}
if u.Ciphers != proxySslCiphers {
t.Errorf("expected %v but got %v", proxySslCiphers, u.Ciphers)
}
Expand Down Expand Up @@ -179,6 +220,8 @@ func TestInvalidAnnotations(t *testing.T) {

// Invalid optional Annotations
data[parser.GetAnnotationWithPrefix("proxy-ssl-secret")] = defaultDemoSecret
data[parser.GetAnnotationWithPrefix("proxy-ssl-client-secret")] = defaultDemoSecret
data[parser.GetAnnotationWithPrefix("proxy-ssl-ca-configmap")] = defaultDemoConfigMap
data[parser.GetAnnotationWithPrefix("proxy-ssl-protocols")] = "TLSv111 SSLv1"
data[parser.GetAnnotationWithPrefix("proxy-ssl-server-name")] = sslServerName
data[parser.GetAnnotationWithPrefix("proxy-ssl-session-reuse")] = sslServerName
Expand Down Expand Up @@ -237,6 +280,9 @@ func TestEquals(t *testing.T) {
t.Errorf("Expected false")
}
cfg2.AuthSSLCert = sslCert1
// TODO: Different client certs

// TODO: Different CAs

// Different Ciphers
cfg1.Ciphers = "DEFAULT"
Expand Down
4 changes: 2 additions & 2 deletions internal/ingress/controller/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -749,9 +749,9 @@ func (n *NGINXController) getBackendServers(ingresses []*ingress.Ingress) ([]*in
}

if !n.store.GetBackendConfiguration().ProxySSLLocationOnly {
if server.ProxySSL.CAFileName == "" {
if server.ProxySSL.CAFileName == "" && server.ProxySSL.ProxySSLCA.CAFileName == "" {
server.ProxySSL = anns.ProxySSL
if server.ProxySSL.Secret != "" && server.ProxySSL.CAFileName == "" {
if (server.ProxySSL.Secret != "" && server.ProxySSL.CAFileName == "") && (server.ProxySSL.ProxySSLCA.ConfigMap != "" && server.ProxySSL.ProxySSLCA.CAFileName == "") {
klog.V(3).Infof("Secret %q has no 'ca.crt' key, client cert authentication disabled for Ingress %q",
server.ProxySSL.Secret, ingKey)
}
Expand Down
8 changes: 8 additions & 0 deletions internal/ingress/controller/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,14 @@ func (fakeIngressStore) GetAuthCertificate(string) (*resolver.AuthSSLCert, error
return nil, fmt.Errorf("test error")
}

func (fakeIngressStore) GetSSLClientCert(string) (*resolver.SSLClientCert, error) {
return nil, fmt.Errorf("test error")
}

func (fakeIngressStore) GetSSLCA(string) (*resolver.SSLCA, error) {
return nil, fmt.Errorf("test error")
}

func (fakeIngressStore) GetDefaultBackend() defaults.Backend {
return defaults.Backend{}
}
Expand Down
Loading
Loading