From 9120c588b5dc5ee4e54e919a53e4964dae1796b8 Mon Sep 17 00:00:00 2001 From: Aaron Harper Date: Fri, 23 Aug 2024 14:22:21 -0400 Subject: [PATCH 1/4] Apply JCS before signature validation --- consts.go | 2 +- handler.go | 8 +++++++- handler_test.go | 6 +++--- signature.go | 16 +++++++++++++--- signature_test.go | 10 +++++----- 5 files changed, 29 insertions(+), 13 deletions(-) diff --git a/consts.go b/consts.go index e77fc935..8a51078d 100644 --- a/consts.go +++ b/consts.go @@ -2,5 +2,5 @@ package inngestgo const ( SDKLanguage = "go" - SDKVersion = "0.5.4" + SDKVersion = "0.7.3" ) diff --git a/handler.go b/handler.go index 2d159ff5..61a3d8eb 100644 --- a/handler.go +++ b/handler.go @@ -774,7 +774,13 @@ func (h *handler) trust( return } - w.Header().Add("X-Inngest-Signature", Sign(ctx, time.Now(), []byte(key), byt)) + resSig, err := Sign(ctx, time.Now(), []byte(key), byt) + if err != nil { + _ = publicerr.WriteHTTP(w, err) + return + } + + w.Header().Add("X-Inngest-Signature", resSig) w.WriteHeader(200) _, err = w.Write(byt) if err != nil { diff --git a/handler_test.go b/handler_test.go index a8b7f61d..dbf8e12a 100644 --- a/handler_test.go +++ b/handler_test.go @@ -494,7 +494,7 @@ func TestIntrospection(t *testing.T) { r := require.New(t) reqBody := []byte("") - sig := Sign(context.Background(), time.Now(), []byte(testKey), reqBody) + sig, _ := Sign(context.Background(), time.Now(), []byte(testKey), reqBody) req, err := http.NewRequest(http.MethodGet, server.URL, bytes.NewReader(reqBody)) r.NoError(err) req.Header.Set("X-Inngest-Signature", sig) @@ -531,7 +531,7 @@ func TestIntrospection(t *testing.T) { reqBody := []byte("") invalidKey := "deadbeef" - sig := Sign(context.Background(), time.Now(), []byte(invalidKey), reqBody) + sig, _ := Sign(context.Background(), time.Now(), []byte(invalidKey), reqBody) req, err := http.NewRequest(http.MethodGet, server.URL, bytes.NewReader(reqBody)) r.NoError(err) req.Header.Set("X-Inngest-Signature", sig) @@ -591,7 +591,7 @@ func handlerPost(t *testing.T, url string, r *sdkrequest.Request) *http.Response t.Helper() body := marshalRequest(t, r) - sig := Sign(context.Background(), time.Now(), []byte(testKey), body) + sig, _ := Sign(context.Background(), time.Now(), []byte(testKey), body) req, err := http.NewRequest(http.MethodPost, url, bytes.NewReader(body)) require.NoError(t, err) diff --git a/signature.go b/signature.go index d73a6c44..0bedef18 100644 --- a/signature.go +++ b/signature.go @@ -10,6 +10,8 @@ import ( "regexp" "strconv" "time" + + "github.com/gowebpki/jcs" ) var ( @@ -22,9 +24,14 @@ var ( ) // Sign signs a request body with the given key at the given timestamp. -func Sign(ctx context.Context, at time.Time, key, body []byte) string { +func Sign(ctx context.Context, at time.Time, key, body []byte) (string, error) { key = normalizeKey(key) + body, err := jcs.Transform(body) + if err != nil { + return "", fmt.Errorf("failed to canonicalize body: %w", err) + } + ts := at.Unix() if at.IsZero() { ts = time.Now().Unix() @@ -36,7 +43,7 @@ func Sign(ctx context.Context, at time.Time, key, body []byte) string { // timing attacks. _, _ = mac.Write([]byte(fmt.Sprintf("%d", ts))) sig := hex.EncodeToString(mac.Sum(nil)) - return fmt.Sprintf("t=%d&s=%s", ts, sig) + return fmt.Sprintf("t=%d&s=%s", ts, sig), nil } // validateSignature ensures that the signature for the given body is signed with @@ -58,7 +65,10 @@ func validateSignature(ctx context.Context, sig string, key, body []byte) (bool, return false, ErrExpiredSignature } - actual := Sign(ctx, ts, key, body) + actual, err := Sign(ctx, ts, key, body) + if err != nil { + return false, err + } if actual != sig { return false, ErrInvalidSignature } diff --git a/signature_test.go b/signature_test.go index cdf216f8..bb6c3323 100644 --- a/signature_test.go +++ b/signature_test.go @@ -23,9 +23,9 @@ func TestSign(t *testing.T) { keyA := []byte("signkey-test-12345678") keyB := []byte("signkey-prod-12345678") keyC := []byte("12345678") - a := Sign(ctx, at, keyA, testBody) - b := Sign(ctx, at, keyB, testBody) - c := Sign(ctx, at, keyC, testBody) + a, _ := Sign(ctx, at, keyA, testBody) + b, _ := Sign(ctx, at, keyB, testBody) + c, _ := Sign(ctx, at, keyC, testBody) require.Equal(t, a, b) require.Equal(t, a, c) }) @@ -54,7 +54,7 @@ func TestValidateSignature(t *testing.T) { t.Run("with the wrong key it fails", func(t *testing.T) { at := time.Now() - sig := Sign(ctx, at, []byte(testKey), testBody) + sig, _ := Sign(ctx, at, []byte(testKey), testBody) ok, _, err := ValidateSignature(ctx, sig, "signkey-test-lolwtf", "", testBody) require.False(t, ok) @@ -64,7 +64,7 @@ func TestValidateSignature(t *testing.T) { t.Run("with the same key and within a reasonable time it succeeds", func(t *testing.T) { at := time.Now().Add(-5 * time.Second) - sig := Sign(ctx, at, []byte(testKey), testBody) + sig, _ := Sign(ctx, at, []byte(testKey), testBody) ok, _, err := ValidateSignature(ctx, sig, testKey, "", testBody) require.True(t, ok) From 0131ad89628305216685bb00480e285be1330f1f Mon Sep 17 00:00:00 2001 From: Aaron Harper Date: Fri, 23 Aug 2024 14:32:26 -0400 Subject: [PATCH 2/4] Don't canonicalize non-JSON bodies --- examples/main.go | 7 +++++-- signature.go | 10 +++++++--- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/examples/main.go b/examples/main.go index 2140ba13..ee38c74c 100644 --- a/examples/main.go +++ b/examples/main.go @@ -14,7 +14,9 @@ import ( ) func main() { - h := inngestgo.NewHandler("billing", inngestgo.HandlerOpts{}) + h := inngestgo.NewHandler("billing", inngestgo.HandlerOpts{ + RegisterURL: inngestgo.StrPtr("http://localhost:8090/fn/register"), + }) // CreateFunction is a factory method which creates new Inngest functions (step functions, // or workflows) with a specific configuration. @@ -22,7 +24,7 @@ func main() { inngestgo.FunctionOpts{ ID: "account-created", Name: "Account creation flow", - Retries: inngestgo.IntPtr(5), + Retries: inngestgo.IntPtr(0), }, // Run on every api/account.created event. inngestgo.EventTrigger("api/account.created", nil), @@ -44,6 +46,7 @@ func main() { // Function state is automatically managed, and persists across server restarts, // cloud migrations, and language changes. func AccountCreated(ctx context.Context, input inngestgo.Input[AccountCreatedEvent]) (any, error) { + return nil, nil // Sleep for a second, minute, hour, week across server restarts. step.Sleep(ctx, "initial-delay", time.Second) diff --git a/signature.go b/signature.go index 0bedef18..2985ac44 100644 --- a/signature.go +++ b/signature.go @@ -12,6 +12,7 @@ import ( "time" "github.com/gowebpki/jcs" + "github.com/inngest/inngest/pkg/logger" ) var ( @@ -27,9 +28,12 @@ var ( func Sign(ctx context.Context, at time.Time, key, body []byte) (string, error) { key = normalizeKey(key) - body, err := jcs.Transform(body) - if err != nil { - return "", fmt.Errorf("failed to canonicalize body: %w", err) + var err error + if len(body) > 0 { + body, err = jcs.Transform(body) + if err != nil { + logger.StdlibLogger(ctx).Warn("failed to canonicalize body", "error", err) + } } ts := at.Unix() From 5bdb1c848598051bd3ad3aa4cd75d299dd8017ac Mon Sep 17 00:00:00 2001 From: Aaron Harper Date: Fri, 23 Aug 2024 14:33:31 -0400 Subject: [PATCH 3/4] JSON test data --- signature_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/signature_test.go b/signature_test.go index bb6c3323..02062900 100644 --- a/signature_test.go +++ b/signature_test.go @@ -10,7 +10,7 @@ import ( ) var ( - testBody = []byte(`hey! if you're reading this come work with us: careers@inngest.com`) + testBody = []byte(`{"msg": "hey! if you're reading this come work with us: careers@inngest.com"}`) testKey = "signkey-test-12345678" testKeyFallback = "signkey-test-00000000" ) From 61da3529f7371c84ff4aa31a6c1be11e2364e841 Mon Sep 17 00:00:00 2001 From: Aaron Harper Date: Fri, 23 Aug 2024 14:41:12 -0400 Subject: [PATCH 4/4] Revert example --- examples/main.go | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/examples/main.go b/examples/main.go index ee38c74c..2140ba13 100644 --- a/examples/main.go +++ b/examples/main.go @@ -14,9 +14,7 @@ import ( ) func main() { - h := inngestgo.NewHandler("billing", inngestgo.HandlerOpts{ - RegisterURL: inngestgo.StrPtr("http://localhost:8090/fn/register"), - }) + h := inngestgo.NewHandler("billing", inngestgo.HandlerOpts{}) // CreateFunction is a factory method which creates new Inngest functions (step functions, // or workflows) with a specific configuration. @@ -24,7 +22,7 @@ func main() { inngestgo.FunctionOpts{ ID: "account-created", Name: "Account creation flow", - Retries: inngestgo.IntPtr(0), + Retries: inngestgo.IntPtr(5), }, // Run on every api/account.created event. inngestgo.EventTrigger("api/account.created", nil), @@ -46,7 +44,6 @@ func main() { // Function state is automatically managed, and persists across server restarts, // cloud migrations, and language changes. func AccountCreated(ctx context.Context, input inngestgo.Input[AccountCreatedEvent]) (any, error) { - return nil, nil // Sleep for a second, minute, hour, week across server restarts. step.Sleep(ctx, "initial-delay", time.Second)