Skip to content

Commit af4cc84

Browse files
committed
refactor: address review feedback on frontendValidation mTLS
- ssl: validate that the CA ConfigMap contains a PEM CERTIFICATE block, rejecting empty/whitespace-only/malformed data before it reaches the downstream mTLS trust anchor. - translator: reject caCertificateRefs with a non-core API group, matching the listener-status validation. - status: scope frontendValidation checks to Terminate listeners. - test: fix unknown-plugin error matching for api7ee (Dashboard wraps the message across lines); add group/malformed-CA translator test cases.
1 parent 03f7ea1 commit af4cc84

5 files changed

Lines changed: 75 additions & 9 deletions

File tree

internal/adc/translator/gateway.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import (
2323
"strings"
2424

2525
"github.com/pkg/errors"
26+
corev1 "k8s.io/api/core/v1"
2627
"k8s.io/apimachinery/pkg/types"
2728
gatewayv1 "sigs.k8s.io/gateway-api/apis/v1"
2829

@@ -153,7 +154,10 @@ func (t *Translator) translateFrontendValidation(tctx *provider.TranslateContext
153154

154155
cas := make([]string, 0, len(listener.TLS.FrontendValidation.CACertificateRefs))
155156
for _, ref := range listener.TLS.FrontendValidation.CACertificateRefs {
156-
// Core support is limited to ConfigMap references.
157+
// Core support is limited to ConfigMap references in the core API group.
158+
if ref.Group != "" && string(ref.Group) != corev1.GroupName {
159+
return nil, fmt.Errorf("unsupported frontendValidation caCertificateRef group %q in listener %s, only the core group is supported", ref.Group, listener.Name)
160+
}
157161
if ref.Kind != "" && string(ref.Kind) != internaltypes.KindConfigMap {
158162
return nil, fmt.Errorf("unsupported frontendValidation caCertificateRef kind %q in listener %s, only ConfigMap is supported", ref.Kind, listener.Name)
159163
}

internal/adc/translator/gateway_test.go

Lines changed: 38 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,15 @@ import (
3333
"github.com/apache/apisix-ingress-controller/internal/provider"
3434
)
3535

36-
const testCACert = "-----BEGIN CERTIFICATE-----\nMIIBCA-test-ca\n-----END CERTIFICATE-----"
36+
const testCACert = `-----BEGIN CERTIFICATE-----
37+
MIIBQzCB6qADAgECAgEBMAoGCCqGSM49BAMCMBIxEDAOBgNVBAMTB3Rlc3QtY2Ew
38+
HhcNNzAwMTAxMDAwMDAwWhcNMzgwMTE5MDMxNDA4WjASMRAwDgYDVQQDEwd0ZXN0
39+
LWNhMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEJo4AsM30ZHN+mYeHjqwceGBz
40+
V2bMz1+OyNXuaPYVrSF7HShZhanOYNHb6QLNhjGxMsBDQHVLolPjyTQJp9R5GqMx
41+
MC8wDgYDVR0PAQH/BAQDAgIEMB0GA1UdDgQWBBRzjh0YVmnpN/cFJziO0aYySuti
42+
4DAKBggqhkjOPQQDAgNIADBFAiEA7fEGiQA7wX0LrrkRH4KplAPOgVV5Kvm/1dv1
43+
3TLq9ssCIHKkv2dhydRvv36KC1WsRDcrl7W+7YmEnCS9PZfb8agM
44+
-----END CERTIFICATE-----`
3745

3846
func newTLSGateway(frontendValidation *gatewayv1.FrontendTLSValidation) *gatewayv1.Gateway {
3947
return &gatewayv1.Gateway{
@@ -136,4 +144,33 @@ func TestTranslateSecret_FrontendValidation(t *testing.T) {
136144
_, err := tr.translateSecret(tctx, gateway.Spec.Listeners[0], gateway)
137145
require.Error(t, err)
138146
})
147+
148+
t.Run("unsupported CA ref group returns error", func(t *testing.T) {
149+
tr := &Translator{Log: logr.Discard()}
150+
gateway := newTLSGateway(&gatewayv1.FrontendTLSValidation{
151+
CACertificateRefs: []gatewayv1.ObjectReference{
152+
{Group: "example.com", Kind: "ConfigMap", Name: "ca-cm"},
153+
},
154+
})
155+
tctx := newTranslateContextWithTLS()
156+
157+
_, err := tr.translateSecret(tctx, gateway.Spec.Listeners[0], gateway)
158+
require.Error(t, err)
159+
})
160+
161+
t.Run("malformed CA data returns error", func(t *testing.T) {
162+
tr := &Translator{Log: logr.Discard()}
163+
gateway := newTLSGateway(&gatewayv1.FrontendTLSValidation{
164+
CACertificateRefs: []gatewayv1.ObjectReference{
165+
{Kind: "ConfigMap", Name: "ca-cm"},
166+
},
167+
})
168+
tctx := newTranslateContextWithTLS()
169+
tctx.ConfigMaps[types.NamespacedName{Namespace: "default", Name: "ca-cm"}] = &corev1.ConfigMap{
170+
Data: map[string]string{corev1.ServiceAccountRootCAKey: " not a pem cert "},
171+
}
172+
173+
_, err := tr.translateSecret(tctx, gateway.Spec.Listeners[0], gateway)
174+
require.Error(t, err)
175+
})
139176
}

internal/controller/utils.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -941,7 +941,9 @@ func getListenerStatus(
941941
}
942942
}
943943

944-
if listener.TLS.FrontendValidation != nil {
944+
// frontendValidation (downstream mTLS) only applies to Terminate listeners.
945+
if listener.TLS.FrontendValidation != nil &&
946+
(listener.TLS.Mode == nil || *listener.TLS.Mode == gatewayv1.TLSModeTerminate) {
945947
validateListenerFrontendValidation(ctx, mrgc, gateway, listener.TLS.FrontendValidation, &conditionResolvedRefs, &conditionProgrammed)
946948
}
947949
}

internal/ssl/util.go

Lines changed: 28 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -91,13 +91,36 @@ func ExtractCAFromConfigMap(cm *corev1.ConfigMap) ([]byte, error) {
9191
if cm == nil {
9292
return nil, ErrMissingCert
9393
}
94-
if ca, ok := cm.Data[corev1.ServiceAccountRootCAKey]; ok && ca != "" {
95-
return []byte(ca), nil
94+
var ca []byte
95+
if v, ok := cm.Data[corev1.ServiceAccountRootCAKey]; ok && v != "" {
96+
ca = []byte(v)
97+
} else if v, ok := cm.BinaryData[corev1.ServiceAccountRootCAKey]; ok && len(v) > 0 {
98+
ca = v
9699
}
97-
if ca, ok := cm.BinaryData[corev1.ServiceAccountRootCAKey]; ok && len(ca) > 0 {
98-
return ca, nil
100+
if len(ca) == 0 {
101+
return nil, ErrMissingCert
102+
}
103+
// Reject whitespace-only or otherwise malformed data so an invalid trust
104+
// anchor never reaches the downstream mTLS configuration.
105+
if !hasCertificatePEMBlock(ca) {
106+
return nil, ErrInvalidPEM
107+
}
108+
return ca, nil
109+
}
110+
111+
// hasCertificatePEMBlock reports whether data contains at least one PEM-encoded
112+
// CERTIFICATE block.
113+
func hasCertificatePEMBlock(data []byte) bool {
114+
for {
115+
var block *pem.Block
116+
block, data = pem.Decode(data)
117+
if block == nil {
118+
return false
119+
}
120+
if block.Type == "CERTIFICATE" {
121+
return true
122+
}
99123
}
100-
return nil, ErrMissingCert
101124
}
102125

103126
// ExtractHostsFromCertificate parses the certificate PEM block and returns the DNS names.

test/e2e/crds/v2/status.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,7 @@ spec:
145145
And(
146146
ContainSubstring(`status: "False"`),
147147
ContainSubstring(`reason: SyncFailed`),
148-
ContainSubstring(`(non-existent-plugin) not found`),
148+
MatchRegexp(`(?s)\(non-existent-plugin\)\s+not found`),
149149
),
150150
)
151151
}

0 commit comments

Comments
 (0)