Skip to content

Commit

Permalink
Add upstream TLS trust from CM bundles (#1171)
Browse files Browse the repository at this point in the history 
* Add upstream TLS trust from CM bundles

* Lint warnings

* Get Secret and ConfigMaps from system namespace

* Use separate CA for ConfigMap

* Use networking.TrustBundleLabelKey

* Update verbosity logs in happy path

* Review improvements

* add error when trust-bundle is empty
  • Loading branch information
@ReToCode
ReToCode authored Dec 19, 2023
1 parent 3c0f5c2 commit 00304db
Show file tree
Hide file tree
Showing 7 changed files with 495 additions and 76 deletions.
13 changes: 0 additions & 13 deletions pkg/config/config.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,10 +65,6 @@ const (
// to indicate that http2 should not be enabled for it.
disableHTTP2AnnotationKey = "kourier.knative.dev/disable-http2"

// ServingNamespaceEnv is an env variable specifying where the serving is deployed.
// e.g. OpenShift deploys Kourier in different namespace so `system.Namespace()` does not work.
ServingNamespaceEnv = "SERVING_NAMESPACE"

// trustedHopsCount Configure the number of additional ingress proxy hops from the
// right side of the x-forwarded-for HTTP header to trust.
trustedHopsCount = "trusted-hops-count"
Expand Down Expand Up @@ -98,15 +94,6 @@ func GatewayNamespace() string {
return namespace
}

// ServingNamespace returns the namespace where the serving is deployed.
func ServingNamespace() string {
namespace := os.Getenv(ServingNamespaceEnv)
if namespace == "" {
return system.Namespace()
}
return namespace
}

// GetDisableHTTP2 specifies whether http2 is going to be disabled
func GetDisableHTTP2(annotations map[string]string) (val string) {
return disableHTTP2Annotation.Value(annotations)
Expand Down
6 changes: 3 additions & 3 deletions pkg/generator/caches_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -289,7 +289,7 @@ func TestLocalTLSListener(t *testing.T) {
Name: "test-ca",
},
Data: map[string][]byte{
certificates.CaCertName: cert,
certificates.CaCertName: secretCert,
},
}

Expand Down Expand Up @@ -494,7 +494,7 @@ func createTestDataForIngress(
externalSNIMatches: []*envoy.SNIMatch{{
Hosts: []string{"foo.example.com"},
CertSource: types.NamespacedName{Namespace: "secretns", Name: "secretname"},
CertificateChain: cert,
CertificateChain: secretCert,
PrivateKey: privateKey}},
}

Expand Down Expand Up @@ -530,7 +530,7 @@ func TestValidateIngress(t *testing.T) {
externalSNIMatches: []*envoy.SNIMatch{{
Hosts: []string{"foo.example.com"},
CertSource: types.NamespacedName{Namespace: "secretns", Name: "secretname"},
CertificateChain: cert,
CertificateChain: secretCert,
PrivateKey: privateKey}},
}

Expand Down
135 changes: 105 additions & 30 deletions pkg/generator/ingress_translator.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ package generator
import (
"context"
"crypto/tls"
"crypto/x509"
"encoding/pem"
"fmt"
"os"
"strings"
Expand All @@ -31,18 +33,21 @@ import (
tlsv3 "github.com/envoyproxy/go-control-plane/envoy/extensions/transport_sockets/tls/v3"
envoymatcherv3 "github.com/envoyproxy/go-control-plane/envoy/type/matcher/v3"
"github.com/envoyproxy/go-control-plane/pkg/wellknown"
"go.uber.org/zap"
"google.golang.org/protobuf/types/known/anypb"
corev1 "k8s.io/api/core/v1"
apierrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/types"
pkgconfig "knative.dev/net-kourier/pkg/config"
envoy "knative.dev/net-kourier/pkg/envoy/api"
"knative.dev/net-kourier/pkg/reconciler/ingress/config"
"knative.dev/networking/pkg/apis/networking"
"knative.dev/networking/pkg/apis/networking/v1alpha1"
"knative.dev/networking/pkg/certificates"
netconfig "knative.dev/networking/pkg/config"
"knative.dev/pkg/kmeta"
"knative.dev/pkg/logging"
"knative.dev/pkg/system"
"knative.dev/pkg/tracker"
)

Expand All @@ -58,22 +63,25 @@ type translatedIngress struct {
}

type IngressTranslator struct {
secretGetter func(ns, name string) (*corev1.Secret, error)
endpointsGetter func(ns, name string) (*corev1.Endpoints, error)
serviceGetter func(ns, name string) (*corev1.Service, error)
tracker tracker.Interface
secretGetter func(ns, name string) (*corev1.Secret, error)
nsConfigmapGetter func(label string) ([]*corev1.ConfigMap, error)
endpointsGetter func(ns, name string) (*corev1.Endpoints, error)
serviceGetter func(ns, name string) (*corev1.Service, error)
tracker tracker.Interface
}

func NewIngressTranslator(
secretGetter func(ns, name string) (*corev1.Secret, error),
nsConfigmapGetter func(label string) ([]*corev1.ConfigMap, error),
endpointsGetter func(ns, name string) (*corev1.Endpoints, error),
serviceGetter func(ns, name string) (*corev1.Service, error),
tracker tracker.Interface) IngressTranslator {
return IngressTranslator{
secretGetter: secretGetter,
endpointsGetter: endpointsGetter,
serviceGetter: serviceGetter,
tracker: tracker,
secretGetter: secretGetter,
nsConfigmapGetter: nsConfigmapGetter,
endpointsGetter: endpointsGetter,
serviceGetter: serviceGetter,
tracker: tracker,
}
}

Expand Down Expand Up @@ -108,6 +116,18 @@ func (translator *IngressTranslator) translateIngress(ctx context.Context, ingre

cfg := config.FromContext(ctx)

var err error
var trustChain []byte
if cfg.Network.SystemInternalTLSEnabled() {
trustChain, err = translator.buildTrustChain(logger)
if err != nil {
return nil, err
}
if trustChain == nil {
return nil, fmt.Errorf("failed to build trust-chain, as no valid CA certificate was provided. Please make sure to provide a valid trust-bundle before enabling `system-internal-tls`")
}
}

for i, rule := range ingress.Spec.Rules {
ruleName := fmt.Sprintf("(%s/%s).Rules[%d]", ingress.Namespace, ingress.Name, i)

Expand Down Expand Up @@ -144,10 +164,9 @@ func (translator *IngressTranslator) translateIngress(ctx context.Context, ingre
// Match the ingress' port with a port on the Service to find the target.
// Also find out if the target supports HTTP2.
var (
externalPort = int32(80)
targetPort = int32(80)
http2 = false
httpsPortUsed = false
externalPort = int32(80)
targetPort = int32(80)
http2 = false
)
for _, port := range service.Spec.Ports {
if port.Port == split.ServicePort.IntVal || port.Name == split.ServicePort.StrVal {
Expand All @@ -157,9 +176,6 @@ func (translator *IngressTranslator) translateIngress(ctx context.Context, ingre
if port.Name == "http2" || port.Name == "h2c" {
http2 = true
}
if port.Port == split.ServicePort.IntVal && port.Name == "https" {
httpsPortUsed = true
}
}

// Disable HTTP2 if the annotation is specified.
Expand Down Expand Up @@ -196,14 +212,10 @@ func (translator *IngressTranslator) translateIngress(ctx context.Context, ingre

var transportSocket *envoycorev3.TransportSocket

// TODO: drop this configmap check - issues/968
// We could determine whether system-internal-tls is enabled or disabled via the flag only,
// but all conformance tests need to be updated to have the port name so we check the configmap as well.

// As Ingress with RewriteHost points to ExternalService(kourier-internal), we don't enable TLS.
if (cfg.Network.SystemInternalTLSEnabled() || httpsPortUsed) && httpPath.RewriteHost == "" {
// As Ingress with RewriteHost points to ExternalService(kourier-internal), we don't enable upstream TLS.
if (cfg.Network.SystemInternalTLSEnabled()) && httpPath.RewriteHost == "" {
var err error
transportSocket, err = translator.createUpstreamTransportSocket(http2, split.ServiceNamespace)
transportSocket, err = translator.createUpstreamTransportSocket(http2, split.ServiceNamespace, trustChain)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -318,16 +330,12 @@ func (translator *IngressTranslator) translateIngressTLS(ingressTLS v1alpha1.Ing
return sniMatch, nil
}

func (translator *IngressTranslator) createUpstreamTransportSocket(http2 bool, namespace string) (*envoycorev3.TransportSocket, error) {
caSecret, err := translator.secretGetter(pkgconfig.ServingNamespace(), netconfig.ServingRoutingCertName)
if err != nil {
return nil, fmt.Errorf("failed to fetch activator CA secret: %w", err)
}
func (translator *IngressTranslator) createUpstreamTransportSocket(http2 bool, namespace string, trustChain []byte) (*envoycorev3.TransportSocket, error) {
var alpnProtocols string
if http2 {
alpnProtocols = "h2"
}
tlsAny, err := anypb.New(createUpstreamTLSContext(caSecret.Data[certificates.CaCertName], namespace, alpnProtocols))
tlsAny, err := anypb.New(createUpstreamTLSContext(trustChain, namespace, alpnProtocols))
if err != nil {
return nil, err
}
Expand All @@ -339,7 +347,7 @@ func (translator *IngressTranslator) createUpstreamTransportSocket(http2 bool, n
}, nil
}

func createUpstreamTLSContext(caCertificate []byte, namespace string, alpnProtocols ...string) *tlsv3.UpstreamTlsContext {
func createUpstreamTLSContext(trustChain []byte, namespace string, alpnProtocols ...string) *tlsv3.UpstreamTlsContext {
return &tlsv3.UpstreamTlsContext{
CommonTlsContext: &tlsv3.CommonTlsContext{
AlpnProtocols: alpnProtocols,
Expand All @@ -351,7 +359,7 @@ func createUpstreamTLSContext(caCertificate []byte, namespace string, alpnProtoc
ValidationContext: &tlsv3.CertificateValidationContext{
TrustedCa: &envoycorev3.DataSource{
Specifier: &envoycorev3.DataSource_InlineBytes{
InlineBytes: caCertificate,
InlineBytes: trustChain,
},
},
MatchTypedSubjectAltNames: []*tlsv3.SubjectAltNameMatcher{{
Expand All @@ -377,6 +385,52 @@ func createUpstreamTLSContext(caCertificate []byte, namespace string, alpnProtoc
}
}

// CA can optionally be in `ca.crt` in the `routing-serving-certs` secret
// and/or configured using a trust-bundle via ConfigMap that has the defined label `knative-ca-trust-bundle`.
// Our upstream TLS context needs to trust them all.
func (translator *IngressTranslator) buildTrustChain(logger *zap.SugaredLogger) ([]byte, error) {
var trustChain []byte

routingCA, err := translator.secretGetter(system.Namespace(), netconfig.ServingRoutingCertName)
if err != nil {
return nil, fmt.Errorf("failed to fetch Secret %s/%s: %w", system.Namespace(), netconfig.ServingRoutingCertName, err)
}
routingCABytes := routingCA.Data[certificates.CaCertName]
if len(routingCABytes) > 0 {
if err = checkCertBundle(routingCABytes); err != nil {
logger.Warnf("CA from Secret %s/%s[%s] is invalid and will be ignored: %v",
system.Namespace(), netconfig.ServingRoutingCertName, certificates.CaCertName, err)
} else {
logger.Debugf("Adding CA from Secret %s/%s[%s] to trust chain", system.Namespace(), netconfig.ServingRoutingCertName, certificates.CaCertName)
trustChain = routingCABytes
}
}

cms, err := translator.nsConfigmapGetter(networking.TrustBundleLabelKey)
if err != nil {
return nil, fmt.Errorf("failed to fetch Configmaps with label: %s in namespace: %s: %w", networking.TrustBundleLabelKey, system.Namespace(), err)
}

newline := []byte("\n")
for _, cm := range cms {
for _, bundle := range cm.Data {
if err = checkCertBundle([]byte(bundle)); err != nil {
logger.Warnf("CA bundle from Configmap %s/%s is invalid and will be ignored: %v",
system.Namespace(), cm.Name, err)
} else {
logger.Debugf("Adding CA bundle from Configmap %s/%s to trust chain", system.Namespace(), cm.Name)
if len(trustChain) > 0 {
// Make sure we always have at least one newline between bundles, multiple ones are ok
trustChain = append(trustChain, newline...)
}
trustChain = append(trustChain, []byte(bundle)...)
}
}
}

return trustChain, nil
}

func trackSecret(t tracker.Interface, ns, name string, ingress *v1alpha1.Ingress) error {
return t.TrackReference(tracker.Reference{
Kind: "Secret",
Expand Down Expand Up @@ -466,3 +520,24 @@ func domainsForRule(rule v1alpha1.IngressRule) []string {
}
return domains
}

func checkCertBundle(certs []byte) error {
for block, rest := pem.Decode(certs); block != nil || len(rest) > 0; block, rest = pem.Decode(rest) {
if block != nil {
switch block.Type {
case "CERTIFICATE":
_, err := x509.ParseCertificate(block.Bytes)
if err != nil {
return fmt.Errorf("failed to parse certificate. Invalid certificate found: %s, %v", block.Bytes, err.Error())
}

default:
return fmt.Errorf("failed to parse bundle. Bundle contains something other than a certificate. Type: %s, block: %s", block.Type, block.Bytes)
}
} else {
// if the last certificate is parsed, and we still have rest, there are additional unwanted things in the CM
return fmt.Errorf("failed to parse bundle. Bundle contains something other than a certificate: %s", rest)
}
}
return nil
}
Loading

0 comments on commit 00304db

Please sign in to comment.