Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Split webhook payload validation logic into separate ValidatePayload* functions #656

Merged
merged 2 commits into from
Aug 10, 2018
Merged

Split webhook payload validation logic into separate ValidatePayload* functions #656

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
db887e4
Split webhook payload validation logic into separate ValidatePayload*…
jfheller Aug 9, 2018
a67a059
go fmt webhook/client_test.go
jfheller Aug 10, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
83 changes: 66 additions & 17 deletions webhook/client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,68 @@ func parseSignatureHeader(header string) (*signedHeader, error) {
return sh, nil
}

// ValidatePayload validates the payload against the Stripe-Signature header
// using the specified signing secret. Returns an error if the body or
// Stripe-Signature header provided are unreadable, if the signature doesn't
// match, or if the timestamp for the signature is older than DefaultTolerance.
//
// NOTE: Stripe will only send Webhook signing headers after you have retrieved
// your signing secret from the Stripe dashboard:
// https://dashboard.stripe.com/webhooks
//
func ValidatePayload(payload []byte, header string, secret string) error {
return ValidatePayloadWithTolerance(payload, header, secret, DefaultTolerance)
}

// ValidatePayloadWithTolerance validates the payload against the Stripe-Signature header
// using the specified signing secret and tolerance window. Returns an error if the body
// or Stripe-Signature header provided are unreadable, if the signature doesn't match, or
// if the timestamp for the signature is older than the specified tolerance.
//
// NOTE: Stripe will only send Webhook signing headers after you have retrieved
// your signing secret from the Stripe dashboard:
// https://dashboard.stripe.com/webhooks
//
func ValidatePayloadWithTolerance(payload []byte, header string, secret string, tolerance time.Duration) error {
return validatePayload(payload, header, secret, tolerance, true)
}

// ValidatePayloadIgnoringTolerance validates the payload against the Stripe-Signature header
// header using the specified signing secret. Returns an error if the body or
// Stripe-Signature header provided are unreadable or if the signature doesn't match.
// Does not check the signature's timestamp.
//
// NOTE: Stripe will only send Webhook signing headers after you have retrieved
// your signing secret from the Stripe dashboard:
// https://dashboard.stripe.com/webhooks
//
func ValidatePayloadIgnoringTolerance(payload []byte, header string, secret string) error {
return validatePayload(payload, header, secret, 0*time.Second, false)
}

func validatePayload(payload []byte, sigHeader string, secret string, tolerance time.Duration, enforceTolerance bool) error {

header, err := parseSignatureHeader(sigHeader)
if err != nil {
return err
}

expectedSignature := ComputeSignature(header.timestamp, payload, secret)
expiredTimestamp := time.Since(header.timestamp) > tolerance
if enforceTolerance && expiredTimestamp {
return ErrTooOld
}

// Check all given v1 signatures, multiple signatures will be sent temporarily in the case of a rolled signature secret
for _, sig := range header.signatures {
if hmac.Equal(expectedSignature, sig) {
return nil
}
}

return ErrNoValidSignature
}

// ConstructEvent initializes an Event object from a JSON webhook payload, validating
// the Stripe-Signature header using the specified signing secret. Returns an error
// if the body or Stripe-Signature header provided are unreadable, if the
Expand Down Expand Up @@ -133,27 +195,14 @@ func ConstructEventIgnoringTolerance(payload []byte, header string, secret strin
func constructEvent(payload []byte, sigHeader string, secret string, tolerance time.Duration, enforceTolerance bool) (stripe.Event, error) {
e := stripe.Event{}

if err := json.Unmarshal(payload, &e); err != nil {
return e, fmt.Errorf("Failed to parse webhook body json: %s", err.Error())
}

header, err := parseSignatureHeader(sigHeader)
if err != nil {
if err := validatePayload(payload, sigHeader, secret, tolerance, enforceTolerance); err != nil {
return e, err
}

expectedSignature := ComputeSignature(header.timestamp, payload, secret)
expiredTimestamp := time.Since(header.timestamp) > tolerance
if enforceTolerance && expiredTimestamp {
return e, ErrTooOld
if err := json.Unmarshal(payload, &e); err != nil {
return e, fmt.Errorf("Failed to parse webhook body json: %s", err.Error())
}

// Check all given v1 signatures, multiple signatures will be sent temporarily in the case of a rolled signature secret
for _, sig := range header.signatures {
if hmac.Equal(expectedSignature, sig) {
return e, nil
}
}
return e, nil

return e, ErrNoValidSignature
}
40 changes: 40 additions & 0 deletions webhook/client_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,10 @@ func TestTokenNew(t *testing.T) {
}

p = newSignedPayload()
err = ValidatePayload(p.payload, "", p.secret)
if err != ErrNotSigned {
t.Errorf("Expected ErrNotSigned from missing signature, got %v", err)
}
evt, err = ConstructEvent(p.payload, "", p.secret)
if err != ErrNotSigned {
t.Errorf("Expected ErrNotSigned from missing signature, got %v", err)
Expand All @@ -77,11 +81,19 @@ func TestTokenNew(t *testing.T) {
t.Errorf("Expected ErrInvalidHeader from bad header format, got %v", err)
}

err = ValidatePayload(p.payload, "t=", p.secret)
if err != ErrInvalidHeader {
t.Errorf("Expected ErrInvalidHeader from bad header format, got %v", err)
}
evt, err = ConstructEvent(p.payload, "t=", p.secret)
if err != ErrInvalidHeader {
t.Errorf("Expected ErrInvalidHeader from bad header format, got %v", err)
}

err = ValidatePayload(p.payload, p.header+",v1=bad_signature", p.secret)
if err != nil {
t.Errorf("Received unexpected %v error with an unreadable signature in the header (should be ignored)", err)
}
evt, err = ConstructEvent(p.payload, p.header+",v1=bad_signature", p.secret)
if err != nil {
t.Errorf("Received unexpected %v error with an unreadable signature in the header (should be ignored)", err)
Expand All @@ -90,6 +102,10 @@ func TestTokenNew(t *testing.T) {
p = newSignedPayload(func(p *SignedPayload) {
p.scheme = "v0"
})
err = ValidatePayload(p.payload, p.header, p.secret)
if err != ErrNoValidSignature {
t.Errorf("Expected error from mismatched schema, got %v", err)
}
evt, err = ConstructEvent(p.payload, p.header, p.secret)
if err != ErrNoValidSignature {
t.Errorf("Expected error from mismatched schema, got %v", err)
Expand All @@ -98,6 +114,10 @@ func TestTokenNew(t *testing.T) {
p = newSignedPayload(func(p *SignedPayload) {
p.signature = []byte("deadbeef")
})
err = ValidatePayload(p.payload, p.header, p.secret)
if err != ErrNoValidSignature {
t.Errorf("Expected error from fake signature, got %v", err)
}
evt, err = ConstructEvent(p.payload, p.header, p.secret)
if err != ErrNoValidSignature {
t.Errorf("Expected error from fake signature, got %v", err)
Expand All @@ -112,10 +132,18 @@ func TestTokenNew(t *testing.T) {
t.Errorf("Got the same signature with two different secret keys")
}

err = ValidatePayload(p.payload, headerWithRolledKey, p.secret)
if err != nil {
t.Errorf("Expected to be able to decode webhook with old key after rolling key, but got %v", err)
}
evt, err = ConstructEvent(p.payload, headerWithRolledKey, p.secret)
if err != nil {
t.Errorf("Expected to be able to decode webhook with old key after rolling key, but got %v", err)
}
err = ValidatePayload(p.payload, headerWithRolledKey, p2.secret)
if err != nil {
t.Errorf("Expected to be able to decode webhook with new key after rolling key, but got %v", err)
}
evt, err = ConstructEvent(p.payload, headerWithRolledKey, p2.secret)
if err != nil {
t.Errorf("Expected to be able to decode webhook with new key after rolling key, but got %v", err)
Expand All @@ -124,11 +152,19 @@ func TestTokenNew(t *testing.T) {
p = newSignedPayload(func(p *SignedPayload) {
p.timestamp = time.Now().Add(-15 * time.Second)
})
err = ValidatePayloadWithTolerance(p.payload, p.header, p.secret, 10*time.Second)
if err != ErrTooOld {
t.Errorf("Received %v error when validating timestamp outside of allowed timing window", err)
}
evt, err = ConstructEventWithTolerance(p.payload, p.header, p.secret, 10*time.Second)
if err != ErrTooOld {
t.Errorf("Received %v error when validating timestamp outside of allowed timing window", err)
}

err = ValidatePayloadWithTolerance(p.payload, p.header, p.secret, 20*time.Second)
if err != nil {
t.Errorf("Received %v error when validating timestamp inside allowed timing window", err)
}
evt, err = ConstructEventWithTolerance(p.payload, p.header, p.secret, 20*time.Second)
if err != nil {
t.Errorf("Received %v error when validating timestamp inside allowed timing window", err)
Expand All @@ -137,6 +173,10 @@ func TestTokenNew(t *testing.T) {
p = newSignedPayload(func(p *SignedPayload) {
p.timestamp = time.Unix(12345, 0)
})
err = ValidatePayloadIgnoringTolerance(p.payload, p.header, p.secret)
if err != nil {
t.Errorf("Received %v error when timestamp outside window but no tolerance specified", err)
}
evt, err = ConstructEventIgnoringTolerance(p.payload, p.header, p.secret)
if err != nil {
t.Errorf("Received %v error when timestamp outside window but no tolerance specified", err)
Expand Down