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..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. | +| `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 3cae6e08..51a98c73 100644 --- a/internal/adc/translator/apisixconsumer.go +++ b/internal/adc/translator/apisixconsumer.go @@ -20,6 +20,8 @@ package translator import ( "fmt" "strconv" + "strings" + "unicode" "github.com/pkg/errors" k8stypes "k8s.io/apimachinery/pkg/types" @@ -307,16 +309,23 @@ 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 %s/%s: %w", string(clockSkewRaw), consumerNamespace, cfg.SecretRef.Name, err) + } + } if clockSkew < 0 { clockSkew = _hmacAuthClockSkewDefaultValue } + // header names are RFC 7230 tokens, so a comma or any space is always a separator signedHeadersRaw := sec.Data["signed_headers"] - signedHeaders := make([]string, 0, len(signedHeadersRaw)) - for _, b := range signedHeadersRaw { - signedHeaders = append(signedHeaders, string(b)) - } + signedHeaders := strings.FieldsFunc(string(signedHeadersRaw), func(r rune) bool { + return r == ',' || unicode.IsSpace(r) + }) var keepHeader bool keepHeaderRaw, ok := sec.Data["keep_headers"] @@ -355,7 +364,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 %s/%s: %w", string(maxReqBodyRaw), consumerNamespace, cfg.SecretRef.Name, err) + } + } if maxReqBody < 0 { maxReqBody = _hmacAuthMaxReqBodyDefaultValue } diff --git a/internal/adc/translator/apisixconsumer_test.go b/internal/adc/translator/apisixconsumer_test.go index ff42eef7..3dfb9650 100644 --- a/internal/adc/translator/apisixconsumer_test.go +++ b/internal/adc/translator/apisixconsumer_test.go @@ -23,13 +23,88 @@ 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(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: 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) { + 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) + + cfg := result.Consumers[0].Plugins["hmac-auth"].(*adctypes.HMACAuthConsumerConfig) + require.Equal(t, tc.expected, cfg.SignedHeaders) + }) + } +} + +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(), tc.key) + require.Contains(t, err.Error(), "default/hmac") + }) + } +} + func TestTranslateApisixConsumer_UsesMetadataLabelsWithoutOverwritingControllerLabels(t *testing.T) { translator := NewTranslator(logr.Discard()) tctx := provider.NewDefaultTranslateContext(context.Background())