From b383614e3d22ef41ffb2241cba066a2ff00ddd84 Mon Sep 17 00:00:00 2001 From: Abhishek Choudhary Date: Sun, 19 Jul 2026 10:41:46 +0545 Subject: [PATCH 1/3] 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()) From 6e300edecfa3649df7e108ade38ebe11e3c098a6 Mon Sep 17 00:00:00 2001 From: Abhishek Choudhary Date: Fri, 24 Jul 2026 14:35:37 +0545 Subject: [PATCH 2/3] fix: align hmac-auth signed_headers handling with upstream Forward-ports apache/apisix-ingress-controller#2809 follow-up 8b165835 so both parse errors name the offending Secret, and brings over the follow-up in apache/apisix-ingress-controller#2824: - accept whitespace as well as commas when splitting signed_headers, so a newline- or space-separated Secret value no longer collapses into one bogus header name - document the Secret-vs-Value format difference on the CRD field - rename the ambiguous hmacConsumerWithSecret parameter - cover the delimiter variants, the empty case and the previously untested max_req_body parse failure --- api/v2/apisixconsumer_types.go | 2 + .../apisix.apache.org_apisixconsumers.yaml | 6 +- docs/en/latest/reference/api-reference.md | 2 +- internal/adc/translator/apisixconsumer.go | 16 ++-- .../adc/translator/apisixconsumer_test.go | 82 ++++++++++++------- 5 files changed, 67 insertions(+), 41 deletions(-) diff --git a/api/v2/apisixconsumer_types.go b/api/v2/apisixconsumer_types.go index c430a0de..25f151dd 100644 --- a/api/v2/apisixconsumer_types.go +++ b/api/v2/apisixconsumer_types.go @@ -165,6 +165,8 @@ type ApisixConsumerJwtAuthValue struct { // ApisixConsumerHMACAuth defines configuration for the HMAC authentication. type ApisixConsumerHMACAuth struct { // SecretRef references a Kubernetes Secret containing the HMAC credentials. + // Unlike Value, the Secret stores signed_headers as a single string listing the + // header names separated by commas or whitespace, for example "X-Date, Host". SecretRef *corev1.LocalObjectReference `json:"secretRef,omitempty" yaml:"secretRef,omitempty"` // Value specifies HMAC authentication credentials. Value *ApisixConsumerHMACAuthValue `json:"value,omitempty" yaml:"value,omitempty"` diff --git a/config/crd/bases/apisix.apache.org_apisixconsumers.yaml b/config/crd/bases/apisix.apache.org_apisixconsumers.yaml index 4b004137..3dd21c64 100644 --- a/config/crd/bases/apisix.apache.org_apisixconsumers.yaml +++ b/config/crd/bases/apisix.apache.org_apisixconsumers.yaml @@ -82,8 +82,10 @@ spec: description: HMACAuth configures the HMAC authentication details. properties: secretRef: - description: SecretRef references a Kubernetes Secret containing - the HMAC credentials. + description: |- + SecretRef references a Kubernetes Secret containing the HMAC credentials. + Unlike Value, the Secret stores signed_headers as a single string listing the + header names separated by commas or whitespace, for example "X-Date, Host". properties: name: default: "" diff --git a/docs/en/latest/reference/api-reference.md b/docs/en/latest/reference/api-reference.md index 5059f1d7..b4b8fb3d 100644 --- a/docs/en/latest/reference/api-reference.md +++ b/docs/en/latest/reference/api-reference.md @@ -896,7 +896,7 @@ ApisixConsumerHMACAuth defines configuration for the HMAC authentication. | Field | Description | | --- | --- | -| `secretRef` _[LocalObjectReference](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.30/#localobjectreference-v1-core)_ | SecretRef references a Kubernetes Secret containing the HMAC credentials. | +| `secretRef` _[LocalObjectReference](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.30/#localobjectreference-v1-core)_ | SecretRef references a Kubernetes Secret containing the HMAC credentials.
Unlike Value, the Secret stores signed_headers as a single string listing the
header names separated by commas or whitespace, for example "X-Date, Host". | | `value` _[ApisixConsumerHMACAuthValue](#apisixconsumerhmacauthvalue)_ | Value specifies HMAC authentication credentials. | diff --git a/internal/adc/translator/apisixconsumer.go b/internal/adc/translator/apisixconsumer.go index cb7524d5..51a98c73 100644 --- a/internal/adc/translator/apisixconsumer.go +++ b/internal/adc/translator/apisixconsumer.go @@ -21,6 +21,7 @@ import ( "fmt" "strconv" "strings" + "unicode" "github.com/pkg/errors" k8stypes "k8s.io/apimachinery/pkg/types" @@ -313,21 +314,18 @@ func (t *Translator) translateConsumerHMACAuthPlugin(tctx *provider.TranslateCon 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) + return nil, fmt.Errorf("hmac-auth: invalid clock_skew %q in secret %s/%s: %w", string(clockSkewRaw), consumerNamespace, cfg.SecretRef.Name, err) } } if clockSkew < 0 { clockSkew = _hmacAuthClockSkewDefaultValue } - // comma-separated header names, not raw bytes + // header names are RFC 7230 tokens, so a comma or any space is always a separator signedHeadersRaw := sec.Data["signed_headers"] - var signedHeaders []string - for _, h := range strings.Split(string(signedHeadersRaw), ",") { - if h = strings.TrimSpace(h); h != "" { - signedHeaders = append(signedHeaders, h) - } - } + signedHeaders := strings.FieldsFunc(string(signedHeadersRaw), func(r rune) bool { + return r == ',' || unicode.IsSpace(r) + }) var keepHeader bool keepHeaderRaw, ok := sec.Data["keep_headers"] @@ -371,7 +369,7 @@ func (t *Translator) translateConsumerHMACAuthPlugin(tctx *provider.TranslateCon 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) + return nil, fmt.Errorf("hmac-auth: invalid max_req_body %q in secret %s/%s: %w", string(maxReqBodyRaw), consumerNamespace, cfg.SecretRef.Name, err) } } if maxReqBody < 0 { diff --git a/internal/adc/translator/apisixconsumer_test.go b/internal/adc/translator/apisixconsumer_test.go index 4c255a66..3dfb9650 100644 --- a/internal/adc/translator/apisixconsumer_test.go +++ b/internal/adc/translator/apisixconsumer_test.go @@ -33,52 +33,76 @@ import ( "github.com/apache/apisix-ingress-controller/internal/provider" ) -func hmacConsumerWithSecret(name string) *apiv2.ApisixConsumer { +func hmacConsumerWithSecret(secretName 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}, + SecretRef: &corev1.LocalObjectReference{Name: secretName}, }, }, }, } } +func hmacSecret(data map[string][]byte) *corev1.Secret { + data["key_id"] = []byte("my-key") + data["secret_key"] = []byte("my-secret") + return &corev1.Secret{Data: data} +} + 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"), - }, - } + for _, tc := range []struct { + name string + raw string + expected []string + }{ + {name: "comma separated", raw: "X-Date,Host", expected: []string{"X-Date", "Host"}}, + {name: "padding and empty entries", raw: " X-Date, , Host, ", expected: []string{"X-Date", "Host"}}, + {name: "newline separated", raw: "X-Date\nHost", expected: []string{"X-Date", "Host"}}, + {name: "space separated", raw: "X-Date Host", expected: []string{"X-Date", "Host"}}, + {name: "single header", raw: "X-Date", expected: []string{"X-Date"}}, + {name: "empty", raw: "", expected: []string{}}, + } { + t.Run(tc.name, func(t *testing.T) { + translator := NewTranslator(logr.Discard()) + tctx := provider.NewDefaultTranslateContext(context.Background()) + tctx.Secrets[k8stypes.NamespacedName{Namespace: "default", Name: "hmac"}] = hmacSecret(map[string][]byte{ + "signed_headers": []byte(tc.raw), + }) - result, err := translator.TranslateApisixConsumer(tctx, hmacConsumerWithSecret("hmac")) - require.NoError(t, err) - require.Len(t, result.Consumers, 1) + 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) + cfg := result.Consumers[0].Plugins["hmac-auth"].(*adctypes.HMACAuthConsumerConfig) + require.Equal(t, tc.expected, 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 - }, - } +func TestTranslateApisixConsumer_HMACAuthRejectsUnparseableNumbers(t *testing.T) { + for _, tc := range []struct { + key string + raw string + }{ + {key: "clock_skew", raw: "3O0"}, // typo: letter O + {key: "max_req_body", raw: "invalid"}, + } { + t.Run(tc.key, func(t *testing.T) { + translator := NewTranslator(logr.Discard()) + tctx := provider.NewDefaultTranslateContext(context.Background()) + tctx.Secrets[k8stypes.NamespacedName{Namespace: "default", Name: "hmac"}] = hmacSecret(map[string][]byte{ + tc.key: []byte(tc.raw), + }) - _, err := translator.TranslateApisixConsumer(tctx, hmacConsumerWithSecret("hmac")) - require.Error(t, err) - require.Contains(t, err.Error(), "clock_skew") + _, err := translator.TranslateApisixConsumer(tctx, hmacConsumerWithSecret("hmac")) + require.Error(t, err) + require.Contains(t, err.Error(), tc.key) + require.Contains(t, err.Error(), "default/hmac") + }) + } } func TestTranslateApisixConsumer_UsesMetadataLabelsWithoutOverwritingControllerLabels(t *testing.T) { From f1600ca9a6eb4dbe28d076ca06879d4aab43f96c Mon Sep 17 00:00:00 2001 From: Abhishek Choudhary Date: Fri, 24 Jul 2026 15:40:37 +0545 Subject: [PATCH 3/3] docs: match crd-ref-docs output for the signed_headers note crd-ref-docs joins consecutive godoc lines with spaces and only emits
for blank-line paragraph breaks, so the hand-written version did not match the generator. --- docs/en/latest/reference/api-reference.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/en/latest/reference/api-reference.md b/docs/en/latest/reference/api-reference.md index b4b8fb3d..25b729c2 100644 --- a/docs/en/latest/reference/api-reference.md +++ b/docs/en/latest/reference/api-reference.md @@ -896,7 +896,7 @@ ApisixConsumerHMACAuth defines configuration for the HMAC authentication. | Field | Description | | --- | --- | -| `secretRef` _[LocalObjectReference](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.30/#localobjectreference-v1-core)_ | SecretRef references a Kubernetes Secret containing the HMAC credentials.
Unlike Value, the Secret stores signed_headers as a single string listing the
header names separated by commas or whitespace, for example "X-Date, Host". | +| `secretRef` _[LocalObjectReference](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.30/#localobjectreference-v1-core)_ | SecretRef references a Kubernetes Secret containing the HMAC credentials. Unlike Value, the Secret stores signed_headers as a single string listing the header names separated by commas or whitespace, for example "X-Date, Host". | | `value` _[ApisixConsumerHMACAuthValue](#apisixconsumerhmacauthvalue)_ | Value specifies HMAC authentication credentials. |