From 1cde58774c949da2ac4a648415c32eac1bb3335f Mon Sep 17 00:00:00 2001 From: Abhishek Choudhary Date: Sun, 19 Jul 2026 10:41:46 +0545 Subject: [PATCH] fix: parse hmac-auth signed_headers from Secret as a header list translateConsumerHMACAuthPlugin ranged over the raw bytes of the signed_headers Secret value, emitting one single-character string per byte instead of the configured header names. The data-plane signature policy then bound nonsensical headers, silently voiding the operator's intended integrity control. Only the secretRef path was affected; the inline Value path already passed a []string. Split the value on commas and trim entries. Also surface strconv.ParseInt failures for clock_skew and max_req_body instead of discarding them, so a typo no longer coerces silently to a default. --- internal/adc/translator/apisixconsumer.go | 28 ++++++++-- .../adc/translator/apisixconsumer_test.go | 51 +++++++++++++++++++ 2 files changed, 74 insertions(+), 5 deletions(-) diff --git a/internal/adc/translator/apisixconsumer.go b/internal/adc/translator/apisixconsumer.go index 3cae6e08..cb7524d5 100644 --- a/internal/adc/translator/apisixconsumer.go +++ b/internal/adc/translator/apisixconsumer.go @@ -20,6 +20,7 @@ package translator import ( "fmt" "strconv" + "strings" "github.com/pkg/errors" k8stypes "k8s.io/apimachinery/pkg/types" @@ -307,15 +308,25 @@ func (t *Translator) translateConsumerHMACAuthPlugin(tctx *provider.TranslateCon } clockSkewRaw := sec.Data["clock_skew"] - clockSkew, _ := strconv.ParseInt(string(clockSkewRaw), 10, 64) + var clockSkew int64 + if len(clockSkewRaw) > 0 { + var err error + clockSkew, err = strconv.ParseInt(string(clockSkewRaw), 10, 64) + if err != nil { + return nil, fmt.Errorf("hmac-auth: invalid clock_skew %q in secret: %w", string(clockSkewRaw), err) + } + } if clockSkew < 0 { clockSkew = _hmacAuthClockSkewDefaultValue } + // comma-separated header names, not raw bytes signedHeadersRaw := sec.Data["signed_headers"] - signedHeaders := make([]string, 0, len(signedHeadersRaw)) - for _, b := range signedHeadersRaw { - signedHeaders = append(signedHeaders, string(b)) + var signedHeaders []string + for _, h := range strings.Split(string(signedHeadersRaw), ",") { + if h = strings.TrimSpace(h); h != "" { + signedHeaders = append(signedHeaders, h) + } } var keepHeader bool @@ -355,7 +366,14 @@ func (t *Translator) translateConsumerHMACAuthPlugin(tctx *provider.TranslateCon } maxReqBodyRaw := sec.Data["max_req_body"] - maxReqBody, _ := strconv.ParseInt(string(maxReqBodyRaw), 10, 64) + var maxReqBody int64 + if len(maxReqBodyRaw) > 0 { + var err error + maxReqBody, err = strconv.ParseInt(string(maxReqBodyRaw), 10, 64) + if err != nil { + return nil, fmt.Errorf("hmac-auth: invalid max_req_body %q in secret: %w", string(maxReqBodyRaw), err) + } + } if maxReqBody < 0 { maxReqBody = _hmacAuthMaxReqBodyDefaultValue } diff --git a/internal/adc/translator/apisixconsumer_test.go b/internal/adc/translator/apisixconsumer_test.go index ff42eef7..4c255a66 100644 --- a/internal/adc/translator/apisixconsumer_test.go +++ b/internal/adc/translator/apisixconsumer_test.go @@ -23,13 +23,64 @@ import ( "github.com/go-logr/logr" "github.com/stretchr/testify/require" + corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + k8stypes "k8s.io/apimachinery/pkg/types" + adctypes "github.com/apache/apisix-ingress-controller/api/adc" apiv2 "github.com/apache/apisix-ingress-controller/api/v2" "github.com/apache/apisix-ingress-controller/internal/controller/label" "github.com/apache/apisix-ingress-controller/internal/provider" ) +func hmacConsumerWithSecret(name string) *apiv2.ApisixConsumer { + return &apiv2.ApisixConsumer{ + ObjectMeta: metav1.ObjectMeta{Name: "demo", Namespace: "default"}, + Spec: apiv2.ApisixConsumerSpec{ + AuthParameter: &apiv2.ApisixConsumerAuthParameter{ + HMACAuth: &apiv2.ApisixConsumerHMACAuth{ + SecretRef: &corev1.LocalObjectReference{Name: name}, + }, + }, + }, + } +} + +func TestTranslateApisixConsumer_HMACAuthSignedHeadersFromSecret(t *testing.T) { + translator := NewTranslator(logr.Discard()) + tctx := provider.NewDefaultTranslateContext(context.Background()) + tctx.Secrets[k8stypes.NamespacedName{Namespace: "default", Name: "hmac"}] = &corev1.Secret{ + Data: map[string][]byte{ + "key_id": []byte("my-key"), + "secret_key": []byte("my-secret"), + "signed_headers": []byte("X-Date, Host"), + }, + } + + result, err := translator.TranslateApisixConsumer(tctx, hmacConsumerWithSecret("hmac")) + require.NoError(t, err) + require.Len(t, result.Consumers, 1) + + cfg := result.Consumers[0].Plugins["hmac-auth"].(*adctypes.HMACAuthConsumerConfig) + require.Equal(t, []string{"X-Date", "Host"}, cfg.SignedHeaders) +} + +func TestTranslateApisixConsumer_HMACAuthRejectsInvalidClockSkew(t *testing.T) { + translator := NewTranslator(logr.Discard()) + tctx := provider.NewDefaultTranslateContext(context.Background()) + tctx.Secrets[k8stypes.NamespacedName{Namespace: "default", Name: "hmac"}] = &corev1.Secret{ + Data: map[string][]byte{ + "key_id": []byte("my-key"), + "secret_key": []byte("my-secret"), + "clock_skew": []byte("3O0"), // typo: letter O + }, + } + + _, err := translator.TranslateApisixConsumer(tctx, hmacConsumerWithSecret("hmac")) + require.Error(t, err) + require.Contains(t, err.Error(), "clock_skew") +} + func TestTranslateApisixConsumer_UsesMetadataLabelsWithoutOverwritingControllerLabels(t *testing.T) { translator := NewTranslator(logr.Discard()) tctx := provider.NewDefaultTranslateContext(context.Background())