diff --git a/base.go b/base.go index 1bde8d0..f9e4780 100644 --- a/base.go +++ b/base.go @@ -54,6 +54,13 @@ func (hrr httpMessage) Context() context.Context { return hrr.Req.Context() } +func (hrr httpMessage) isDebug() bool { + if dbgval, ok := hrr.Context().Value(ctxKeyAddDebug).(bool); ok { + return dbgval + } + return false +} + /* calculateSignatureBase calculates the 'signature base' - the data used as the input to signing or verifying The signature base is an ASCII string containing the canonicalized HTTP message components covered by the signature. @@ -232,12 +239,10 @@ func deriveComponentValueRequest(req *http.Request, component componentID) (stri // deriveTargetURI resolves to an absolute form as required by RFC 9110 and referenced by the http signatures spec. // The target URI excludes the reference's fragment component, if any, since fragment identifiers are reserved for client-side processing func deriveTargetURI(req *http.Request) string { - if req.URL.IsAbs() { - return req.RequestURI - } scheme := "https" if req.TLS == nil { scheme = "http" } + return fmt.Sprintf("%s://%s%s%s", scheme, req.Host, req.URL.RawPath, req.URL.RawQuery) } diff --git a/go.mod b/go.mod index 4bc8174..ca025e3 100644 --- a/go.mod +++ b/go.mod @@ -3,7 +3,6 @@ module github.com/remitly-oss/httpsig-go go 1.21.4 require ( - github.com/dunglas/httpsfv v1.0.2 // indirect - github.com/google/go-cmp v0.6.0 // indirect - golang.org/x/exp v0.0.0-20240222234643-814bf88cf225 // indirect + github.com/dunglas/httpsfv v1.0.2 + github.com/google/go-cmp v0.7.0 ) diff --git a/go.sum b/go.sum index 8b3c2dd..239a929 100644 --- a/go.sum +++ b/go.sum @@ -1,6 +1,4 @@ github.com/dunglas/httpsfv v1.0.2 h1:iERDp/YAfnojSDJ7PW3dj1AReJz4MrwbECSSE59JWL0= github.com/dunglas/httpsfv v1.0.2/go.mod h1:zID2mqw9mFsnt7YC3vYQ9/cjq30q41W+1AnDwH8TiMg= -github.com/google/go-cmp v0.6.0 h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI= -github.com/google/go-cmp v0.6.0/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY= -golang.org/x/exp v0.0.0-20240222234643-814bf88cf225 h1:LfspQV/FYTatPTr/3HzIcmiUFH7PGP+OQ6mgDYo3yuQ= -golang.org/x/exp v0.0.0-20240222234643-814bf88cf225/go.mod h1:CxmFvTBINI24O/j8iY7H1xHzx2i4OsyguNBmN/uPtqc= +github.com/google/go-cmp v0.7.0 h1:wk8382ETsv4JYUZwIsn6YpYiWiBsYLSJiTsyBybVuN8= +github.com/google/go-cmp v0.7.0/go.mod h1:pXiqmnSA92OHEEa9HXL2W4E7lf9JzCmGVUdgjX3N/iU= diff --git a/keyutil/jwk.go b/keyutil/jwk.go index 7b8e3a8..982ad75 100644 --- a/keyutil/jwk.go +++ b/keyutil/jwk.go @@ -11,6 +11,11 @@ import ( "os" ) +const ( + KeyTypeEC = "EC" + KeyTypeOct = "oct" +) + func ReadJWKFile(jwkFile string) (JWK, error) { keyBytes, err := os.ReadFile(jwkFile) if err != nil { @@ -23,49 +28,61 @@ func ReadJWK(jwkBytes []byte) (JWK, error) { base := jwk{} err := json.Unmarshal(jwkBytes, &base) if err != nil { - return JWK{}, fmt.Errorf("Failed to json parse JWK public key: %w", err) + return JWK{}, fmt.Errorf("Failed to json parse JWK: %w", err) } - return JWK{ + jwk := JWK{ KeyType: base.KeyType, Algorithm: base.Algo, KeyID: base.KeyID, - raw: json.RawMessage(jwkBytes), - }, nil + } + switch base.KeyType { + case KeyTypeEC: + jec := jwkEC{} + err := json.Unmarshal(jwkBytes, &jec) + if err != nil { + return JWK{}, fmt.Errorf("Failed to json parse JWK: %w", err) + } + jwk.jtype = jec + case KeyTypeOct: + jsym := jwkSymmetric{} + err := json.Unmarshal(jwkBytes, &jsym) + if err != nil { + return JWK{}, fmt.Errorf("Failed to json parse JWK: %w", err) + } + jwk.jtype = jsym + default: + return JWK{}, fmt.Errorf("Unsupported key type/kty - '%s'", base.KeyType) + } + + return jwk, nil } // ReadJWKFromPEM converts a PEM encoded private key to JWK. Use 'fields' to set additional fields like kty, and kid. alg is set based on the passed in PrivateKey. -func ReadJWKFromPEM(fields JWK, pkeyBytes []byte) (JWK, error) { +func ReadJWKFromPEM(pkeyBytes []byte) (JWK, error) { pkey, err := ReadPrivateKey(pkeyBytes) if err != nil { return JWK{}, err } - return FromPrivateKey(fields, pkey) + return FromPrivateKey(pkey) } // FromPrivateKey creates a JWK from a crypto.PrivateKey. Use 'fields' to set additional fields like kty, and kid. alg is set based on the passed in PrivateKey. -func FromPrivateKey(fields JWK, pkey crypto.PrivateKey) (JWK, error) { +func FromPrivateKey(pkey crypto.PrivateKey) (JWK, error) { switch key := pkey.(type) { case *ecdsa.PrivateKey: - jwk := jwkEC{ + jec := jwkEC{ jwk: jwk{ - KeyType: "EC", - Algo: fields.Algorithm, - KeyID: fields.KeyID, + KeyType: KeyTypeEC, }, Curve: key.Curve.Params().Name, - X: octet{key.X}, - Y: octet{key.Y}, - D: octet{key.D}, - } - out, err := json.Marshal(jwk) - if err != nil { - return JWK{}, fmt.Errorf("Error marshalling JWK: %w", err) + X: &octet{*key.X}, + Y: &octet{*key.Y}, + D: &octet{*key.D}, } + return JWK{ - KeyType: "EC", - Algorithm: fields.Algorithm, - KeyID: fields.KeyID, - raw: out, + KeyType: KeyTypeEC, + jtype: jec, }, nil default: return JWK{}, fmt.Errorf("Unsupported private key type '%T'", pkey) @@ -74,55 +91,60 @@ func FromPrivateKey(fields JWK, pkey crypto.PrivateKey) (JWK, error) { // JWK provides basic data and usage for a JWK. type JWK struct { - KeyType string // 'kty' + KeyType string // 'kty' - "EC", "RSA", "oct" Algorithm string // 'alg' KeyID string // 'kid' - raw json.RawMessage + jtype any // the type of JWK based on KeyType. } func (ji *JWK) PublicKey() (crypto.PublicKey, error) { switch ji.KeyType { - case "EC": // ECC - jwk := jwkEC{} - err := json.Unmarshal(ji.raw, &jwk) - if err != nil { - return JWK{}, fmt.Errorf("Failed to json parse JWK into key type 'EC': %w", err) + case ji.KeyType: // ECC + if jec, ok := ji.jtype.(jwkEC); ok { + return jec.PublicKey() } - return jwk.PublicKey() } - return nil, fmt.Errorf("Unsupported key type for PublicKey'%s'", ji.KeyType) + return nil, fmt.Errorf("Unsupported key type for PublicKey - '%s'", ji.KeyType) +} + +func (ji *JWK) PublicKeyJWK() (JWK, error) { + switch ji.KeyType { + case KeyTypeEC: + if jec, ok := ji.jtype.(jwkEC); ok { + jec.jwk.Algo = ji.Algorithm + jec.jwk.KeyID = ji.KeyID + return jec.PublicKeyJWK() + } + } + + return JWK{}, fmt.Errorf("Unsupported key type for PublicKey'%s'", ji.KeyType) } func (ji *JWK) PrivateKey() (crypto.PrivateKey, error) { switch ji.KeyType { - case "EC": - jwk := jwkEC{} - err := json.Unmarshal(ji.raw, &jwk) - if err != nil { - return JWK{}, fmt.Errorf("Failed to json parse JWK into key type 'EC': %w", err) + case KeyTypeEC: + if jec, ok := ji.jtype.(jwkEC); ok { + return jec.PrivateKey() } - return jwk.PrivateKey() } - return nil, fmt.Errorf("Unsupported key type PrivateKey '%s'", ji.KeyType) + return nil, fmt.Errorf("Unsupported key type PrivateKey - '%s'", ji.KeyType) } func (ji *JWK) SecretKey() ([]byte, error) { switch ji.KeyType { - case "oct": - jwk := jwkSymmetric{} - err := json.Unmarshal(ji.raw, &jwk) - if err != nil { - return nil, fmt.Errorf("Failed to json parse JWK into key type 'oct': %w", err) + case KeyTypeOct: + if jsym, ok := ji.jtype.(jwkSymmetric); ok { + return jsym.Key(), nil } - return jwk.Key(), nil + } return nil, fmt.Errorf("Unsupported key type for Secret '%s'", ji.KeyType) } // octet represents the data for base64 URL encoded data as specified by JWKs. type octet struct { - *big.Int + big.Int } func (ob octet) MarshalJSON() ([]byte, error) { @@ -145,7 +167,7 @@ func (ob *octet) UnmarshalJSON(data []byte) error { x := new(big.Int) x.SetBytes(rawBytes) - *ob = octet{x} + *ob = octet{*x} return nil } @@ -159,9 +181,9 @@ type jwk struct { type jwkEC struct { jwk Curve string `json:"crv"` // The curve used with the key e.g. P-256 - X octet `json:"x"` // x coordinate of the curve. - Y octet `json:"y"` // y coordinate of the curve. - D octet `json:"d,omitempty"` // For private keys. + X *octet `json:"x"` // x coordinate of the curve. + Y *octet `json:"y"` // y coordinate of the curve. + D *octet `json:"d,omitempty"` // For private keys. } func (ec *jwkEC) params() (crv elliptic.Curve, byteLen int, e error) { @@ -193,12 +215,29 @@ func (ec *jwkEC) PublicKey() (*ecdsa.PublicKey, error) { return &ecdsa.PublicKey{ Curve: crv, - X: ec.X.Int, - Y: ec.Y.Int, + X: &ec.X.Int, + Y: &ec.Y.Int, + }, nil +} + +func (ec *jwkEC) PublicKeyJWK() (JWK, error) { + return JWK{ + KeyType: ec.KeyType, + Algorithm: ec.Algo, + KeyID: ec.KeyID, + jtype: jwkEC{ + jwk: ec.jwk, + Curve: ec.Curve, + X: ec.X, + Y: ec.Y, + }, }, nil } func (ec *jwkEC) PrivateKey() (*ecdsa.PrivateKey, error) { + if ec.D == nil { + return nil, fmt.Errorf("JWK does not contain a private key") + } pubkey, err := ec.PublicKey() if err != nil { return nil, err @@ -214,19 +253,31 @@ func (ec *jwkEC) PrivateKey() (*ecdsa.PrivateKey, error) { return &ecdsa.PrivateKey{ PublicKey: *pubkey, - D: ec.D.Int, + D: &ec.D.Int, }, nil } type jwkSymmetric struct { jwk - K octet `json:"k" ` // Symmetric key + K *octet `json:"k" ` // Symmetric key } func (js *jwkSymmetric) Key() []byte { return js.K.Bytes() } -func (jwk JWK) MarshalJSON() ([]byte, error) { - return jwk.raw, nil +func (j JWK) MarshalJSON() ([]byte, error) { + // Set the Algo and KeyID in case the JWK fields have changed + switch jt := j.jtype.(type) { + case jwkEC: + jt.jwk.Algo = j.Algorithm + jt.jwk.KeyID = j.KeyID + return json.Marshal(jt) + case jwkSymmetric: + jt.jwk.Algo = j.Algorithm + jt.jwk.KeyID = j.KeyID + return json.Marshal(jt) + } + + return json.Marshal(j.jtype) } diff --git a/keyutil/jwk_test.go b/keyutil/jwk_test.go index 5e6fd28..a013121 100644 --- a/keyutil/jwk_test.go +++ b/keyutil/jwk_test.go @@ -5,6 +5,7 @@ import ( "crypto/ecdsa" "crypto/elliptic" "crypto/rand" + "encoding/json" "strings" "testing" @@ -78,11 +79,13 @@ func TestJWKMarshalRoundTrip(t *testing.T) { inputType string expectedErrContains string keyid string + algorithm string }{ { name: "EC Key Round Trip", inputType: "EC", keyid: "mykey_123", + algorithm: "myalgo", }, } @@ -97,10 +100,10 @@ func TestJWKMarshalRoundTrip(t *testing.T) { t.Fatal(err) } } - original, err := FromPrivateKey(JWK{ - KeyID: tc.keyid, - }, pk) + original, err := FromPrivateKey(pk) + original.KeyID = tc.keyid + original.Algorithm = tc.algorithm if err != nil { if tc.expectedErrContains != "" { if !strings.Contains(err.Error(), tc.expectedErrContains) { @@ -112,7 +115,7 @@ func TestJWKMarshalRoundTrip(t *testing.T) { } // Marshal JWK to JSON - jsonBytes, err := original.MarshalJSON() + jsonBytes, err := json.Marshal(original) if err != nil { t.Fatalf("Failed to marshal JWK: %v", err) } diff --git a/signatures.go b/signatures.go index 0d74557..8043e58 100644 --- a/signatures.go +++ b/signatures.go @@ -94,7 +94,7 @@ func sign(hrr httpMessage, sp sigParameters) error { r.FillBytes(sigBytes[0:32]) s.FillBytes(sigBytes[32:64]) } else { - return fmt.Errorf("Invalid private key. Requires ed25519.PrivateKey") + return fmt.Errorf("Invalid private key. Requires *ecdsa.PrivateKey") } case Algo_ECDSA_P384_SHA384: if eccpk, ok := sp.PrivateKey.(*ecdsa.PrivateKey); ok { @@ -108,7 +108,7 @@ func sign(hrr httpMessage, sp sigParameters) error { r.FillBytes(sigBytes[0:48]) s.FillBytes(sigBytes[48:96]) } else { - return fmt.Errorf("Invalid private key. Requires ed25519.PrivateKey") + return fmt.Errorf("Invalid private key. Requires *ecdsa.PrivateKey") } case Algo_ED25519: if edpk, ok := sp.PrivateKey.(ed25519.PrivateKey); ok { diff --git a/sigtest/helpers.go b/sigtest/helpers.go index 859f581..1beda29 100644 --- a/sigtest/helpers.go +++ b/sigtest/helpers.go @@ -76,7 +76,7 @@ func ReadTestPrivateKey(t testing.TB, pkFile string, hint ...string) crypto.Priv return pkey } -func Diff(t *testing.T, expected, actual interface{}, msg string, opts ...cmp.Option) bool { +func Diff(t *testing.T, expected, actual any, msg string, opts ...cmp.Option) bool { if diff := cmp.Diff(expected, actual, opts...); diff != "" { t.Errorf("%s (-want +got):\n%s", msg, diff) return true diff --git a/verify.go b/verify.go index 6936bf2..a38ee14 100644 --- a/verify.go +++ b/verify.go @@ -33,6 +33,8 @@ var ( // DefaultRequiredFields covers the request body with 'content-digest' the method and full URI. // As per the specification Date is not covered in favor of using the 'created' metadata parameter. DefaultRequiredFields = Fields("content-digest", "@method", "@target-uri") + + ctxKeyAddDebug = struct{}{} ) // KeySpec is the per-key information needed to verify a signature. @@ -82,16 +84,20 @@ type VerifyProfile struct { CreatedValidDuration time.Duration // Duration allowed for between time.Now and the created time ExpiredSkew time.Duration // Maximum duration allowed between time.Now and the expired time DateFieldSkew time.Duration // Maximum duration allowed between Date field and created - } type VerifyResult struct { Verified bool Label string KeySpecer KeySpecer + DebugInfo VerifyDebugInfo // Present if the verifier debug flag is set and the signature was valid. MetadataProvider } +type VerifyDebugInfo struct { + SignatureBase string // The signature base derived from the request. +} + type Verifier struct { keys KeyFetcher profile VerifyProfile @@ -170,10 +176,22 @@ func (ver *Verifier) verify(hrr httpMessage) (VerifyResult, error) { if err != nil { return vr, err } + vr.Label = sig.Label vr.MetadataProvider = sig.Input.MetadataValues /* verify and validate */ - keyspec, err := ver.verifySignature(hrr, sig) + base, err := calculateSignatureBase(hrr, sig.Input) + if err != nil { + return vr, err + } + + if hrr.isDebug() { + vr.DebugInfo = VerifyDebugInfo{ + SignatureBase: string(base.base), + } + } + keyspec, err := ver.verifySignature(hrr, sig, base) + vr.KeySpecer = keyspec if err != nil { return vr, err } @@ -184,8 +202,10 @@ func (ver *Verifier) verify(hrr httpMessage) (VerifyResult, error) { return VerifyResult{ Verified: true, + Label: sig.Label, KeySpecer: keyspec, MetadataProvider: sig.Input.MetadataValues, + DebugInfo: vr.DebugInfo, }, nil } @@ -286,14 +306,10 @@ func unmarshalSignature(sigs signaturesSFV, label string) (extractedSignature, e return sigInfo, nil } -func (ver *Verifier) verifySignature(r httpMessage, sig extractedSignature) (KeySpecer, error) { - base, err := calculateSignatureBase(r, sig.Input) - if err != nil { - return nil, err - } - +func (ver *Verifier) verifySignature(r httpMessage, sig extractedSignature, base signatureBase) (KeySpecer, error) { var specer KeySpecer var ks KeySpec + var err error // Get keyspec if slices.Contains(sig.Input.MetadataParams, MetaKeyID) { keyid, err := sig.Input.MetadataValues.KeyID() @@ -454,3 +470,7 @@ func (mp metadataProviderFromParams) Tag() (string, error) { } return "", fmt.Errorf("No tag value") } + +func SetAddDebugInfo(ctx context.Context) context.Context { + return context.WithValue(ctx, ctxKeyAddDebug, true) +} diff --git a/verify_test.go b/verify_test.go index 1bccb3f..4c0dfe3 100644 --- a/verify_test.go +++ b/verify_test.go @@ -13,12 +13,12 @@ import ( func TestVerify(t *testing.T) { testcases := []struct { - Name string - RequestFile string - Label string - Keys httpsig.KeyFetcher - Expected httpsig.VerifyResult - ExpectedKeySpec httpsig.KeySpec + Name string + RequestFile string + Label string + AddDebugInfo bool + Keys httpsig.KeyFetcher + Expected httpsig.VerifyResult }{ { Name: "OneValid", @@ -38,31 +38,66 @@ func TestVerify(t *testing.T) { httpsig.MetaKeyID: "test-key-rsa-pss", httpsig.MetaCreated: int64(1618884473), httpsig.MetaNonce: "b3k2pp5k7z-50gnwp.yemd", - }, + }}, + KeySpecer: httpsig.KeySpec{ + KeyID: "test-key-rsa-pss", + Algo: httpsig.Algo_RSA_PSS_SHA512, + PubKey: keyutil.MustReadPublicKeyFile("testdata/test-key-rsa-pss.pub"), }, }, - ExpectedKeySpec: httpsig.KeySpec{ - KeyID: "test-key-rsa-pss", - Algo: httpsig.Algo_RSA_PSS_SHA512, - PubKey: keyutil.MustReadPublicKeyFile("testdata/test-key-rsa-pss.pub"), + }, + { + Name: "OneValidDebug", + Label: "sig-b21", + RequestFile: "verify_request1.txt", + AddDebugInfo: true, + Keys: keyman.NewKeyFetchInMemory(map[string]httpsig.KeySpec{ + "test-key-rsa-pss": { + KeyID: "test-key-rsa-pss", + Algo: httpsig.Algo_RSA_PSS_SHA512, + PubKey: keyutil.MustReadPublicKeyFile("testdata/test-key-rsa-pss.pub"), + }, + }), + Expected: httpsig.VerifyResult{ + Verified: true, + Label: "sig-b21", + MetadataProvider: &fixedMetadataProvider{map[httpsig.Metadata]any{ + httpsig.MetaKeyID: "test-key-rsa-pss", + httpsig.MetaCreated: int64(1618884473), + httpsig.MetaNonce: "b3k2pp5k7z-50gnwp.yemd", + }}, + KeySpecer: httpsig.KeySpec{ + KeyID: "test-key-rsa-pss", + Algo: httpsig.Algo_RSA_PSS_SHA512, + PubKey: keyutil.MustReadPublicKeyFile("testdata/test-key-rsa-pss.pub"), + }, + DebugInfo: httpsig.VerifyDebugInfo{ + SignatureBase: `"@signature-params": ();created=1618884473;keyid="test-key-rsa-pss";nonce="b3k2pp5k7z-50gnwp.yemd"`, + }, }, }, } for _, tc := range testcases { t.Run(tc.Name, func(t *testing.T) { - actual, err := httpsig.Verify(sigtest.ReadRequest(t, tc.RequestFile), tc.Keys, httpsig.VerifyProfile{SignatureLabel: tc.Label}) + req := sigtest.ReadRequest(t, tc.RequestFile) + if tc.AddDebugInfo { + req = req.WithContext(httpsig.SetAddDebugInfo(req.Context())) + } + actual, err := httpsig.Verify(req, tc.Keys, httpsig.VerifyProfile{SignatureLabel: tc.Label}) if err != nil { t.Fatal(err) } // VerifyResult is returned even when error is also returned. + // Because VerifryResult embed Metadataprovider we first need diff ignoring the MetadataProvider + sigtest.Diff(t, tc.Expected, actual, "Did not match", + cmp.FilterPath(func(p cmp.Path) bool { + return p.String() == "MetadataProvider" + }, cmp.Ignore())) + + // Then diff the metadata provider sigtest.Diff(t, tc.Expected, actual, "Did not match", getCmdOpts()...) - actualKS, err := actual.KeySpecer.KeySpec() - if err != nil { - t.Fatal(err) - } - sigtest.Diff(t, tc.ExpectedKeySpec, actualKS, "Key spec did not match") }) } } @@ -176,10 +211,12 @@ func metaVal[E comparable](f1 func() (E, error)) any { func getCmdOpts() []cmp.Option { return []cmp.Option{ - cmp.Transformer("Metadata", TransformMeta), + // This gets used for *ANY* struct assignable to MetadataProvider including other structres + // that embed it! + cmp.Transformer("MetadataProvider", TransformMeta), } - } + func TransformMeta(md httpsig.MetadataProvider) map[string]any { out := map[string]any{}