diff --git a/app/auth/plugins/twilio_signature/incoming.go b/app/auth/plugins/twilio_signature/incoming.go index 0a8132ee..4855c19e 100644 --- a/app/auth/plugins/twilio_signature/incoming.go +++ b/app/auth/plugins/twilio_signature/incoming.go @@ -6,8 +6,11 @@ import ( "crypto/sha1" "encoding/base64" "fmt" + "mime" "net/http" + "net/url" "sort" + "strings" "github.com/winhowes/AuthTranslator/app/auth" "github.com/winhowes/AuthTranslator/app/secrets" @@ -41,24 +44,58 @@ func (t *TwilioSignatureAuth) ParseParams(m map[string]interface{}) (interface{} return p, nil } -func canonicalString(r *http.Request) string { +func canonicalString(r *http.Request, body []byte) string { // Use the full URL including query string as seen by the proxy base := r.URL.String() - // Include POST form parameters sorted by key - if err := r.ParseForm(); err == nil { - keys := make([]string, 0, len(r.PostForm)) - for k := range r.PostForm { - keys = append(keys, k) + + form, ok := parseCanonicalForm(r, body) + if !ok || len(form) == 0 { + return base + } + + keys := make([]string, 0, len(form)) + totalLen := len(base) + for k, vals := range form { + keys = append(keys, k) + for _, v := range vals { + totalLen += len(k) + len(v) } - sort.Strings(keys) - for _, k := range keys { - vals := r.PostForm[k] - for _, v := range vals { - base += k + v - } + } + sort.Strings(keys) + + var builder strings.Builder + builder.Grow(totalLen) + builder.WriteString(base) + for _, k := range keys { + vals := form[k] + for _, v := range vals { + builder.WriteString(k) + builder.WriteString(v) } } - return base + return builder.String() +} + +func parseCanonicalForm(r *http.Request, body []byte) (url.Values, bool) { + if r.PostForm != nil { + return r.PostForm, true + } + + switch r.Method { + case http.MethodPost, http.MethodPut, http.MethodPatch: + default: + return nil, false + } + + mediaType, _, err := mime.ParseMediaType(r.Header.Get("Content-Type")) + if err != nil || mediaType != "application/x-www-form-urlencoded" { + return nil, false + } + form, err := url.ParseQuery(string(body)) + if err != nil { + return nil, false + } + return form, true } func (t *TwilioSignatureAuth) Authenticate(ctx context.Context, r *http.Request, params interface{}) bool { @@ -70,11 +107,13 @@ func (t *TwilioSignatureAuth) Authenticate(ctx context.Context, r *http.Request, if sig == "" { return false } - // Ensure body is read and restored for ParseForm - if _, err := authplugins.GetBody(r); err != nil { + // Use the shared body cache so signature validation does not consume the + // request body before the proxy forwards it upstream. + body, err := authplugins.GetBody(r) + if err != nil { return false } - base := canonicalString(r) + base := canonicalString(r, body) for _, ref := range cfg.Secrets { secret, err := secrets.LoadSecret(ctx, ref) if err != nil { diff --git a/app/auth/plugins/twilio_signature/twilio_signature_test.go b/app/auth/plugins/twilio_signature/twilio_signature_test.go index 82e6d86f..8f0d9997 100644 --- a/app/auth/plugins/twilio_signature/twilio_signature_test.go +++ b/app/auth/plugins/twilio_signature/twilio_signature_test.go @@ -5,14 +5,15 @@ import ( "crypto/hmac" "crypto/sha1" "encoding/base64" - authplugins "github.com/winhowes/AuthTranslator/app/auth" "io" "net/http" "net/url" "sort" + "strconv" "strings" "testing" + authplugins "github.com/winhowes/AuthTranslator/app/auth" _ "github.com/winhowes/AuthTranslator/app/secrets/plugins" ) @@ -58,6 +59,102 @@ func TestTwilioSignatureAuth(t *testing.T) { } } +func TestCanonicalStringUsesCachedBody(t *testing.T) { + body := []byte("B=two&A=one&A=uno") + r := &http.Request{ + Method: http.MethodPost, + URL: &url.URL{Path: "/callback"}, + Header: http.Header{"Content-Type": []string{"application/x-www-form-urlencoded; charset=utf-8"}}, + Body: io.NopCloser(strings.NewReader("unread")), + } + + got := canonicalString(r, body) + if got != "/callbackAoneAunoBtwo" { + t.Fatalf("unexpected canonical string %q", got) + } + + remaining, err := io.ReadAll(r.Body) + if err != nil { + t.Fatalf("unexpected body read error: %v", err) + } + if string(remaining) != "unread" { + t.Fatalf("canonicalString consumed request body: %q", string(remaining)) + } +} + +func TestCanonicalStringNoFormReturnsURL(t *testing.T) { + r := &http.Request{ + Method: http.MethodPost, + URL: &url.URL{Path: "/callback", RawQuery: "q=1"}, + Header: http.Header{"Content-Type": []string{"application/x-www-form-urlencoded"}}, + } + + if got := canonicalString(r, nil); got != "/callback?q=1" { + t.Fatalf("unexpected canonical string %q", got) + } +} + +func TestParseCanonicalFormBranches(t *testing.T) { + preParsed := &http.Request{PostForm: url.Values{"A": []string{"one"}}} + form, ok := parseCanonicalForm(preParsed, nil) + if !ok || form.Get("A") != "one" { + t.Fatalf("expected pre-parsed form, got %v %t", form, ok) + } + + unsupportedMethod := &http.Request{ + Method: http.MethodGet, + Header: http.Header{"Content-Type": []string{"application/x-www-form-urlencoded"}}, + } + if form, ok := parseCanonicalForm(unsupportedMethod, []byte("A=one")); ok || form != nil { + t.Fatalf("expected unsupported method to skip form, got %v %t", form, ok) + } + + unsupportedMedia := &http.Request{ + Method: http.MethodPost, + Header: http.Header{"Content-Type": []string{"text/plain"}}, + } + if form, ok := parseCanonicalForm(unsupportedMedia, []byte("A=one")); ok || form != nil { + t.Fatalf("expected unsupported media type to skip form, got %v %t", form, ok) + } + + malformedForm := &http.Request{ + Method: http.MethodPost, + Header: http.Header{"Content-Type": []string{"application/x-www-form-urlencoded"}}, + } + if form, ok := parseCanonicalForm(malformedForm, []byte("A=%zz")); ok || form != nil { + t.Fatalf("expected malformed form to fail, got %v %t", form, ok) + } +} + +func TestTwilioSignatureAuthenticatePreservesBody(t *testing.T) { + form := url.Values{"Body": []string{"hello"}, "From": []string{"+15551234567"}} + urlStr := "/path" + sig := sign(urlStr, form, "tok") + body := form.Encode() + r := &http.Request{Method: "POST", URL: &url.URL{Path: urlStr}, Header: http.Header{ + "X-Twilio-Signature": []string{sig}, + "Content-Type": []string{"application/x-www-form-urlencoded"}, + }, Body: io.NopCloser(strings.NewReader(body))} + + p := TwilioSignatureAuth{} + t.Setenv("TOK", "tok") + cfg, err := p.ParseParams(map[string]interface{}{"secrets": []string{"env:TOK"}}) + if err != nil { + t.Fatal(err) + } + if !p.Authenticate(context.Background(), r, cfg) { + t.Fatal("expected authentication to succeed") + } + + remaining, err := io.ReadAll(r.Body) + if err != nil { + t.Fatalf("unexpected body read error: %v", err) + } + if string(remaining) != body { + t.Fatalf("expected body to remain %q, got %q", body, string(remaining)) + } +} + func TestTwilioSignatureAuthFail(t *testing.T) { form := url.Values{"Foo": []string{"bar"}} urlStr := "/cb" @@ -216,3 +313,22 @@ func TestTwilioSignatureSecretLoadError(t *testing.T) { t.Fatal("expected auth failure when secret load fails") } } + +func BenchmarkCanonicalStringLargeForm(b *testing.B) { + form := make(url.Values, 2000) + for i := 0; i < 2000; i++ { + form.Set("Field"+strconv.Itoa(i), strings.Repeat("x", 24)) + } + body := []byte(form.Encode()) + r := &http.Request{ + Method: http.MethodPost, + URL: &url.URL{Path: "/callback", RawQuery: "q=1"}, + Header: http.Header{"Content-Type": []string{"application/x-www-form-urlencoded"}}, + } + + b.ReportAllocs() + b.ResetTimer() + for i := 0; i < b.N; i++ { + _ = canonicalString(r, body) + } +}