From 291bed0a2ee465c66b34aaacf005b5fd5e02b1f5 Mon Sep 17 00:00:00 2001 From: Daisuke Maki Date: Mon, 26 Jul 2021 16:23:35 +0900 Subject: [PATCH 1/6] Implement RFC7797 (TODO: return error if b64 = false and payload contains '.') --- jws/README.md | 3 +- jws/interface.go | 1 + jws/jws.go | 68 ++++++++++++++++++++++++---- jws/jws_test.go | 115 +++++++++++++++++++++++++++++++++++++++++++++++ jws/message.go | 49 +++++++++++++------- jws/option.go | 8 ++++ 6 files changed, 220 insertions(+), 24 deletions(-) diff --git a/jws/README.md b/jws/README.md index 64675165..37220db5 100644 --- a/jws/README.md +++ b/jws/README.md @@ -1,12 +1,13 @@ # github.com/lestrrat-go/jwx/jws [![Go Reference](https://pkg.go.dev/badge/github.com/lestrrat-go/jwx/jws.svg)](https://pkg.go.dev/github.com/lestrrat-go/jwx/jws) -Package jws implements JWS as described in [RFC7515](https://tools.ietf.org/html/rfc7515) +Package jws implements JWS as described in [RFC7515](https://tools.ietf.org/html/rfc7515) and [RFC7797](https://tools.ietf.org/html/rfc7797) * Parse and generate compact or JSON serializations * Sign and verify arbitrary payload * Use any of the keys supported in [github.com/lestrrat-go/jwx/jwk](../jwk) * Add arbitrary fields in the JWS object * Ability to add/replace existing signature methods +* Respect "b64" settings for RFC7797 Examples are located in the examples directory ([jws_example_test.go](../examples/jws_example_test.go)) diff --git a/jws/interface.go b/jws/interface.go index b63a3911..b4e5b075 100644 --- a/jws/interface.go +++ b/jws/interface.go @@ -51,6 +51,7 @@ import ( type Message struct { payload []byte signatures []*Signature + b64 bool // true if payload should be base64 encoded } type Signature struct { diff --git a/jws/jws.go b/jws/jws.go index cf5d5d0e..e8ed63f0 100644 --- a/jws/jws.go +++ b/jws/jws.go @@ -79,6 +79,11 @@ func (s *payloadSigner) PublicHeader() Headers { // the type of key you provided, otherwise an error is returned. // // If you would like to pass custom headers, use the WithHeaders option. +// +// If the headers contain "b64" field, then the boolean value for the field +// is respected when creating the compact serialization form. That is, +// if you specify a header with `{"b64": false}`, then the payload is +// not base64 encoded. func Sign(payload []byte, alg jwa.SignatureAlgorithm, key interface{}, options ...SignOption) ([]byte, error) { var hdrs Headers for _, o := range options { @@ -163,11 +168,14 @@ func SignMulti(payload []byte, options ...Option) ([]byte, error) { // use `Parse` function to get `Message` object. func Verify(buf []byte, alg jwa.SignatureAlgorithm, key interface{}, options ...VerifyOption) ([]byte, error) { var dst *Message + var detachedPayload []byte //nolint:forcetypeassert for _, option := range options { switch option.Ident() { case identMessage{}: dst = option.Value().(*Message) + case identDetachedPayload{}: + detachedPayload = option.Value().([]byte) } } @@ -177,9 +185,9 @@ func Verify(buf []byte, alg jwa.SignatureAlgorithm, key interface{}, options ... } if buf[0] == '{' { - return verifyJSON(buf, alg, key, dst) + return verifyJSON(buf, alg, key, dst, detachedPayload) } - return verifyCompact(buf, alg, key, dst) + return verifyCompact(buf, alg, key, dst, detachedPayload) } // VerifySet uses keys store in a jwk.Set to verify the payload in `buf`. @@ -217,7 +225,7 @@ func VerifySet(buf []byte, set jwk.Set) ([]byte, error) { return nil, errors.New(`failed to verify message with any of the keys in the jwk.Set object`) } -func verifyJSON(signed []byte, alg jwa.SignatureAlgorithm, key interface{}, dst *Message) ([]byte, error) { +func verifyJSON(signed []byte, alg jwa.SignatureAlgorithm, key interface{}, dst *Message, detachedPayload []byte) ([]byte, error) { verifier, err := NewVerifier(alg) if err != nil { return nil, errors.Wrap(err, "failed to create verifier") @@ -228,8 +236,24 @@ func verifyJSON(signed []byte, alg jwa.SignatureAlgorithm, key interface{}, dst return nil, errors.Wrap(err, `failed to unmarshal JSON message`) } + if len(m.payload) != 0 && detachedPayload != nil { + return nil, errors.New(`can't specify detached payload for JWS with payload`) + } + if detachedPayload != nil { + m.payload = detachedPayload + } + + if detachedPayload != nil { + + } + // Pre-compute the base64 encoded version of payload - payload := base64.EncodeToString(m.payload) + var payload string + if m.b64 { + payload = base64.EncodeToString(m.payload) + } else { + payload = string(m.payload) + } buf := pool.GetBytesBuffer() defer pool.ReleaseBytesBuffer(buf) @@ -263,7 +287,23 @@ func verifyJSON(signed []byte, alg jwa.SignatureAlgorithm, key interface{}, dst return nil, errors.New(`could not verify with any of the signatures`) } -func verifyCompact(signed []byte, alg jwa.SignatureAlgorithm, key interface{}, dst *Message) ([]byte, error) { +// get the value of b64 header field. +// If the field does not exist, returns true (default) +// Otherwise return the value specified by the header field. +func getB64Value(hdr Headers) bool { + b64raw, ok := hdr.Get("b64") + if !ok { + return true // default + } + + b64, ok := b64raw.(bool) // default + if !ok { + return false + } + return b64 +} + +func verifyCompact(signed []byte, alg jwa.SignatureAlgorithm, key interface{}, dst *Message, detachedPayload []byte) ([]byte, error) { protected, payload, signature, err := SplitCompact(signed) if err != nil { return nil, errors.Wrap(err, `failed extract from compact serialization format`) @@ -279,6 +319,9 @@ func verifyCompact(signed []byte, alg jwa.SignatureAlgorithm, key interface{}, d verifyBuf.Write(protected) verifyBuf.WriteByte('.') + if len(payload) == 0 && detachedPayload != nil { + payload = detachedPayload + } verifyBuf.Write(payload) decodedSignature, err := base64.Decode(signature) @@ -303,13 +346,22 @@ func verifyCompact(signed []byte, alg jwa.SignatureAlgorithm, key interface{}, d } } } + if err := verifier.Verify(verifyBuf.Bytes(), decodedSignature, key); err != nil { return nil, errors.Wrap(err, `failed to verify message`) } - decodedPayload, err := base64.Decode(payload) - if err != nil { - return nil, errors.Wrap(err, `message verified, failed to decode payload`) + var decodedPayload []byte + if !getB64Value(hdr) { // it's not base64 encode + decodedPayload = payload + } + + if decodedPayload == nil { + v, err := base64.Decode(payload) + if err != nil { + return nil, errors.Wrap(err, `message verified, failed to decode payload`) + } + decodedPayload = v } if dst != nil { diff --git a/jws/jws_test.go b/jws/jws_test.go index 7e361fc8..40082941 100644 --- a/jws/jws_test.go +++ b/jws/jws_test.go @@ -1137,3 +1137,118 @@ func TestWithMessage(t *testing.T) { return } } + +func TestRFC7797(t *testing.T) { + const keysrc = `{"kty":"oct", + "k":"AyM1SysPpbyDfgZld3umj1qzKObwVMkoqQ-EstJQLr_T-1qS0gZH75aKtMN3Yj0iPS4hcgUuTwjAzZr1Z9CAow" + }` + detached := []byte(`$.02`) + + key, err := jwk.ParseKey([]byte(keysrc)) + if !assert.NoError(t, err, `jwk.Parse should succeed`) { + return + } + + t.Run("Invalid payload when b64 = false", func(t *testing.T) { + const payload = `$.02` + hdrs := jws.NewHeaders() + hdrs.Set("b64", false) + hdrs.Set("crit", "b64") + + _, err := jws.Sign([]byte(payload), jwa.HS256, key, jws.WithHeaders(hdrs)) + if !assert.Error(t, err, `jws.Sign should fail`) { + return + } + }) + t.Run("Roundtrip", func(t *testing.T) { + const payload = `hell0w0r1d` + hdrs := jws.NewHeaders() + hdrs.Set("b64", false) + hdrs.Set("crit", "b64") + + signed, err := jws.Sign([]byte(payload), jwa.HS256, key, jws.WithHeaders(hdrs)) + if !assert.NoError(t, err, `jws.Sign should succeed`) { + return + } + + verified, err := jws.Verify(signed, jwa.HS256, key) + if !assert.NoError(t, err, `jws.Verify should succeed`) { + return + } + + if !assert.Equal(t, []byte(payload), verified, `payload should match`) { + return + } + }) + + t.Run("Verify", func(t *testing.T) { + testcases := []struct { + Name string + Input []byte + VerifyOptions []jws.VerifyOption + Error bool + }{ + { + Name: "JSON format", + Input: []byte(`{ + "protected": "eyJhbGciOiJIUzI1NiIsImI2NCI6ZmFsc2UsImNyaXQiOlsiYjY0Il19", + "payload": "$.02", + "signature": "A5dxf2s96_n5FLueVuW1Z_vh161FwXZC4YLPff6dmDY" + }`), + }, + { + Name: "JSON format (detached payload)", + VerifyOptions: []jws.VerifyOption{ + jws.WithDetachedPayload(detached), + }, + Input: []byte(`{ + "protected": "eyJhbGciOiJIUzI1NiIsImI2NCI6ZmFsc2UsImNyaXQiOlsiYjY0Il19", + "signature": "A5dxf2s96_n5FLueVuW1Z_vh161FwXZC4YLPff6dmDY" + }`), + }, + { + Name: "JSON Format (b64 does not match)", + Error: true, + Input: []byte(`{ + "signatures": [ + { + "protected": "eyJhbGciOiJIUzI1NiIsImI2NCI6ZmFsc2UsImNyaXQiOlsiYjY0Il19", + "signature": "A5dxf2s96_n5FLueVuW1Z_vh161FwXZC4YLPff6dmDY" + }, + { + "protected": "eyJhbGciOiJIUzI1NiIsImI2NCI6dHJ1ZSwiY3JpdCI6WyJiNjQiXX0", + "signature": "6BjugbC8MfrT_yy5WxWVFZrEHVPDtpdsV9u-wbzQDV8" + } + ], + "payload":"$.02" + }`), + }, + { + Name: "Compact (detached payload)", + Input: []byte(`eyJhbGciOiJIUzI1NiIsImI2NCI6ZmFsc2UsImNyaXQiOlsiYjY0Il19..A5dxf2s96_n5FLueVuW1Z_vh161FwXZC4YLPff6dmDY`), + VerifyOptions: []jws.VerifyOption{ + jws.WithDetachedPayload(detached), + }, + }, + } + + for _, tc := range testcases { + tc := tc + t.Run(tc.Name, func(t *testing.T) { + payload, err := jws.Verify([]byte(tc.Input), jwa.HS256, key, tc.VerifyOptions...) + if tc.Error { + if !assert.Error(t, err, `jws.Verify should fail`) { + return + } + } else { + if !assert.NoError(t, err, `jws.Verify should succeed`) { + return + } + if !assert.Equal(t, detached, payload, `payload should match`) { + return + } + } + }) + } + }) +} diff --git a/jws/message.go b/jws/message.go index adc49388..1bf84f6c 100644 --- a/jws/message.go +++ b/jws/message.go @@ -2,6 +2,7 @@ package jws import ( "context" + "strings" "github.com/lestrrat-go/jwx/internal/base64" "github.com/lestrrat-go/jwx/internal/json" @@ -79,7 +80,11 @@ func (s *Signature) Sign(payload []byte, signer Signer, key interface{}) ([]byte buf.WriteString(base64.EncodeToString(hdrbuf)) buf.WriteByte('.') - buf.WriteString(base64.EncodeToString(payload)) + if getB64Value(hdrs) { + buf.WriteString(base64.EncodeToString(payload)) + } else { + buf.Write(payload) + } signature, err := signer.Sign(buf.Bytes(), key) if err != nil { @@ -171,20 +176,9 @@ func (m *Message) UnmarshalJSON(buf []byte) error { return errors.Wrap(err, `failed to unmarshal into temporary structure`) } - // Everything in the proxy is base64 encoded, except for signatures.header - if len(proxy.Payload) == 0 { - return errors.New(`"payload" must be non-empty`) - } - - buf, err := base64.DecodeString(proxy.Payload) - if err != nil { - return errors.Wrap(err, `failed to decode payload`) - } - m.payload = buf - if proxy.Signature != nil { if len(proxy.Signatures) > 0 { - return errors.Wrap(err, `invalid format ("signatures" and "signature" keys cannot both be present)`) + return errors.New(`invalid format ("signatures" and "signature" keys cannot both be present)`) } var sigproxy signatureProxy @@ -199,6 +193,7 @@ func (m *Message) UnmarshalJSON(buf []byte) error { proxy.Signatures = append(proxy.Signatures, &sigproxy) } + b64 := true for i, sigproxy := range proxy.Signatures { var sig Signature @@ -210,7 +205,7 @@ func (m *Message) UnmarshalJSON(buf []byte) error { } if len(sigproxy.Protected) > 0 { - buf, err = base64.DecodeString(sigproxy.Protected) + buf, err := base64.DecodeString(sigproxy.Protected) if err != nil { return errors.Wrapf(err, `failed to decode "protected" for signature #%d`, i+1) } @@ -218,13 +213,21 @@ func (m *Message) UnmarshalJSON(buf []byte) error { if err := json.Unmarshal(buf, sig.protected); err != nil { return errors.Wrapf(err, `failed to unmarshal "protected" for signature #%d`, i+1) } + + if i == 0 { + b64 = getB64Value(sig.protected) + } else { + if b64 != getB64Value(sig.protected) { + return errors.Errorf(`b64 value must be the same for all signatures`) + } + } } if len(sigproxy.Signature) == 0 { return errors.Errorf(`"signature" must be non-empty for signature #%d`, i+1) } - buf, err = base64.DecodeString(sigproxy.Signature) + buf, err := base64.DecodeString(sigproxy.Signature) if err != nil { return errors.Wrapf(err, `failed to decode "signature" for signature #%d`, i+1) } @@ -232,6 +235,22 @@ func (m *Message) UnmarshalJSON(buf []byte) error { m.signatures = append(m.signatures, &sig) } + if !b64 { + m.payload = []byte(proxy.Payload) + } else { + // Everything in the proxy is base64 encoded, except for signatures.header + if len(proxy.Payload) == 0 { + return errors.New(`"payload" must be non-empty`) + } + + buf, err := base64.DecodeString(proxy.Payload) + if err != nil { + return errors.Wrap(err, `failed to decode payload`) + } + m.payload = buf + } + m.b64 = b64 + return nil } diff --git a/jws/option.go b/jws/option.go index 1615c8bc..ea1b731b 100644 --- a/jws/option.go +++ b/jws/option.go @@ -7,6 +7,7 @@ import ( type Option = option.Interface type identPayloadSigner struct{} +type identDetachedPayload struct{} type identHeaders struct{} type identMessage struct{} @@ -53,3 +54,10 @@ func (*verifyOption) verifyOption() {} func WithMessage(m *Message) VerifyOption { return &verifyOption{option.New(identMessage{}, m)} } + +// WithDetachedPayload can be used to verify a JWS message with a +// detached payload. If you have to verify using this option, you should +// know exactly how and why this works. +func WithDetachedPayload(v []byte) VerifyOption { + return &verifyOption{option.New(identDetachedPayload{}, v)} +} From 38803a86a2fc05292412fa93122f6323b7f0101e Mon Sep 17 00:00:00 2001 From: Daisuke Maki Date: Mon, 26 Jul 2021 18:48:09 +0900 Subject: [PATCH 2/6] remove unused --- jws/message.go | 1 - 1 file changed, 1 deletion(-) diff --git a/jws/message.go b/jws/message.go index 1bf84f6c..c6e2519f 100644 --- a/jws/message.go +++ b/jws/message.go @@ -2,7 +2,6 @@ package jws import ( "context" - "strings" "github.com/lestrrat-go/jwx/internal/base64" "github.com/lestrrat-go/jwx/internal/json" From 5eb6bdbae59b5cffc497a4aea091657b2019bc99 Mon Sep 17 00:00:00 2001 From: Daisuke Maki Date: Mon, 26 Jul 2021 18:51:59 +0900 Subject: [PATCH 3/6] style tweaks --- .golangci.yml | 1 + jws/jws.go | 5 +---- jws/jws_test.go | 2 +- 3 files changed, 3 insertions(+), 5 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 3b2f8bcc..b66c608d 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -36,6 +36,7 @@ linters: - nestif - nlreturn - paralleltest + - tagliatelle - testpackage - thelper - wrapcheck diff --git a/jws/jws.go b/jws/jws.go index e8ed63f0..1ba49491 100644 --- a/jws/jws.go +++ b/jws/jws.go @@ -239,12 +239,9 @@ func verifyJSON(signed []byte, alg jwa.SignatureAlgorithm, key interface{}, dst if len(m.payload) != 0 && detachedPayload != nil { return nil, errors.New(`can't specify detached payload for JWS with payload`) } - if detachedPayload != nil { - m.payload = detachedPayload - } if detachedPayload != nil { - + m.payload = detachedPayload } // Pre-compute the base64 encoded version of payload diff --git a/jws/jws_test.go b/jws/jws_test.go index 40082941..8e4a1557 100644 --- a/jws/jws_test.go +++ b/jws/jws_test.go @@ -1235,7 +1235,7 @@ func TestRFC7797(t *testing.T) { for _, tc := range testcases { tc := tc t.Run(tc.Name, func(t *testing.T) { - payload, err := jws.Verify([]byte(tc.Input), jwa.HS256, key, tc.VerifyOptions...) + payload, err := jws.Verify(tc.Input, jwa.HS256, key, tc.VerifyOptions...) if tc.Error { if !assert.Error(t, err, `jws.Verify should fail`) { return From 924ec74d9383cd9cd21c4ea74240063873c24ddc Mon Sep 17 00:00:00 2001 From: Daisuke Maki Date: Mon, 26 Jul 2021 18:54:26 +0900 Subject: [PATCH 4/6] upgrade golangci-lint accidentally added directives for newer golangci-lint. might as well upgrade it then --- .github/workflows/lint.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index 877887b6..d928ef42 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -8,7 +8,7 @@ jobs: - uses: actions/checkout@v2 - uses: golangci/golangci-lint-action@v2 with: - version: v1.39.0 + version: v1.41.1 - name: Run go vet run: | go vet ./... From d53288b413824376c6163e1a0defc519b5625601 Mon Sep 17 00:00:00 2001 From: Daisuke Maki Date: Mon, 26 Jul 2021 19:04:19 +0900 Subject: [PATCH 5/6] check for b64 = false and payload contain '.' --- jws/message.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/jws/message.go b/jws/message.go index c6e2519f..97ecab66 100644 --- a/jws/message.go +++ b/jws/message.go @@ -1,6 +1,7 @@ package jws import ( + "bytes" "context" "github.com/lestrrat-go/jwx/internal/base64" @@ -82,6 +83,9 @@ func (s *Signature) Sign(payload []byte, signer Signer, key interface{}) ([]byte if getB64Value(hdrs) { buf.WriteString(base64.EncodeToString(payload)) } else { + if bytes.ContainsRune(payload, '.') { + return nil, nil, errors.New(`payload must not contain a "." when b64 = false`) + } buf.Write(payload) } From 10d0771b9b8cfe04f758a17f2bf3b428bb81502d Mon Sep 17 00:00:00 2001 From: Daisuke Maki Date: Mon, 26 Jul 2021 19:52:13 +0900 Subject: [PATCH 6/6] Update Changes --- Changes | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/Changes b/Changes index 88f01f02..f13147e0 100644 --- a/Changes +++ b/Changes @@ -1,6 +1,12 @@ Changes ======= +v1.2.5 (UNRELEASED) +[New features] + * Implement RFC7797. The value of the header field `b64` changes + how the payload is treated in JWS + * Implement detached payloads for JWS + v1.2.4 15 Jul 2021 [Bug fixes] * We had the same off-by-one in another place and jumped the gun on