From ee9dc91767bb25912628ef8e0bebdda52a16875c Mon Sep 17 00:00:00 2001 From: Grant Shively Date: Wed, 14 Apr 2021 00:50:52 -0700 Subject: [PATCH] bundle: refactor sign/verify to use interface (#3336) It is now possible to register a custom implementation of the sign and verify functions. Signed-off-by: Grant Shively --- .gitignore | 1 + bundle/bundle.go | 5 +++ bundle/bundle_test.go | 66 ++++++++++++++++++++++++++++++++++++++ bundle/keys.go | 10 ++++++ bundle/sign.go | 59 +++++++++++++++++++++++++++++++++- bundle/sign_test.go | 33 +++++++++++++++++++ bundle/verify.go | 62 ++++++++++++++++++++++++++++++++++- bundle/verify_test.go | 32 ++++++++++++++++++ cmd/build.go | 8 +++-- cmd/flags.go | 4 +++ cmd/sign.go | 7 ++-- docs/content/management.md | 14 ++++++++ 12 files changed, 294 insertions(+), 7 deletions(-) diff --git a/.gitignore b/.gitignore index 0ec8e28e1c..88cc88c36f 100644 --- a/.gitignore +++ b/.gitignore @@ -3,6 +3,7 @@ .vscode .idea *~ +*.swp # build artifacts coverage.txt diff --git a/bundle/bundle.go b/bundle/bundle.go index dca6d25919..bb94ba11ea 100644 --- a/bundle/bundle.go +++ b/bundle/bundle.go @@ -53,6 +53,7 @@ type Bundle struct { // SignaturesConfig represents an array of JWTs that encapsulate the signatures for the bundle. type SignaturesConfig struct { Signatures []string `json:"signatures,omitempty"` + Plugin string `json:"plugin,omitempty"` } // isEmpty returns if the SignaturesConfig is empty. @@ -806,6 +807,10 @@ func (b *Bundle) GenerateSignature(signingConfig *SigningConfig, keyID string, u b.Signatures = SignaturesConfig{} } + if signingConfig.Plugin != "" { + b.Signatures.Plugin = signingConfig.Plugin + } + b.Signatures.Signatures = []string{string(token)} return nil diff --git a/bundle/bundle_test.go b/bundle/bundle_test.go index c07e1ebab9..ba80b2655e 100644 --- a/bundle/bundle_test.go +++ b/bundle/bundle_test.go @@ -216,6 +216,10 @@ func TestReadWithSignatures(t *testing.T) { signedTokenHS256 := `eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCIsImtpZCI6ImZvbyJ9.eyJmaWxlcyI6W3sibmFtZSI6Ii5tYW5pZmVzdCIsImhhc2giOiI1MDdhMmMzOGExNDQxZGI1OGQyY2I4Nzk4MmM0MmFhOTFhNDM0MmVmNDIyYTZiNTQyZWRkZWJlZWY2ZjA0MTJmIiwiYWxnb3JpdGhtIjoiU0hBLTI1NiJ9LHsibmFtZSI6ImEvYi9jL2RhdGEuanNvbiIsImhhc2giOiI0MmNmZTY3NjhiNTdiYjVmNzUwM2MxNjVjMjhkZDA3YWM1YjgxMzU1NGViYzg1MGYyY2MzNTg0M2U3MTM3YjFkIiwiYWxnb3JpdGhtIjoiU0hBLTI1NiJ9LHsibmFtZSI6Imh0dHAvcG9saWN5L3BvbGljeS5yZWdvIiwiaGFzaCI6ImE2MTVlZWFlZTIxZGU1MTc5ZGUwODBkZThjMzA1MmM4ZGE5MDExMzg0MDZiYTcxYzM4YzAzMjg0NWY3ZDU0ZjQiLCJhbGdvcml0aG0iOiJTSEEtMjU2In1dLCJpYXQiOjE1OTIyNDgwMjcsImlzcyI6IkpXVFNlcnZpY2UiLCJrZXlpZCI6ImZvbyIsInNjb3BlIjoid3JpdGUifQ.grzWHYvyVS6LfWy0oiFTEJThKooOAwic8sexYaflzOM` otherSignedTokenHS256 := `eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCIsImtpZCI6ImZvbyJ9.eyJmaWxlcyI6W3sibmFtZSI6ImEvYi9jL2RhdGEuanNvbiIsImhhc2giOiJmOWNhYzA3MTQ3MDVkMjBkMWEyMDg4MDE4NWNkZWQ2ZTBmNmQwNDA2NjJkMmViYjA5NjFkM2Q5ZjMxN2Q4YWNiIn1dLCJpYXQiOjE1OTIyNDgwMjcsImlzcyI6IkpXVFNlcnZpY2UiLCJzY29wZSI6IndyaXRlIn0.WJhnUjwaVvckSgOd4QcVvKThN6oc99NiPiwHKYnoG7c` + defaultSigner, _ := GetSigner(defaultSignerID) + defaultVerifier, _ := GetVerifier(defaultVerifierID) + RegisterSigner("_bar", defaultSigner) + RegisterVerifier("_bar", defaultVerifier) tests := map[string]struct { files [][2]string @@ -270,6 +274,16 @@ func TestReadWithSignatures(t *testing.T) { NewVerificationConfig(map[string]*KeyConfig{"foo": {Key: "secret", Algorithm: "HS256"}}, "", "write", []string{".*", "a/b/c/data.json", "http/policy/policy.rego"}), false, nil, }, + "customer_signer_verifier": { + [][2]string{ + {"/.signatures.json", fmt.Sprintf(`{"signatures": ["%v"],"plugin":"_bar"}`, signedTokenHS256)}, + {"/.manifest", `{"revision": "quickbrownfaux"}`}, + {"/a/b/c/data.json", "[1,2,3]"}, + {"/http/policy/policy.rego", `package example`}, + }, + NewVerificationConfig(map[string]*KeyConfig{"foo": {Key: "secret", Algorithm: "HS256"}}, "", "write", []string{".*", "a/b/c/data.json", "http/policy/policy.rego"}), + false, nil, + }, } for name, tc := range tests { @@ -860,6 +874,58 @@ func TestGenerateSignature(t *testing.T) { } } +func TestGenerateSignatureWithPlugin(t *testing.T) { + signatures := SignaturesConfig{Signatures: []string{"some_token"}, Plugin: "_foo"} + + bundle := Bundle{ + Data: map[string]interface{}{ + "foo": map[string]interface{}{ + "bar": []interface{}{json.Number("1"), json.Number("2"), json.Number("3")}, + "baz": true, + "qux": "hello", + }, + }, + Modules: []ModuleFile{ + { + URL: "/foo/corge/corge.rego", + Path: "/foo/corge/corge.rego", + Parsed: ast.MustParseModule(`package foo.corge`), + Raw: []byte("package foo.corge\n"), + }, + }, + Wasm: []byte("modules-compiled-as-wasm-binary"), + Manifest: Manifest{ + Revision: "quickbrownfaux", + }, + Signatures: signatures, + } + + defaultSigner, _ := GetSigner(defaultSignerID) + defaultVerifier, _ := GetVerifier(defaultVerifierID) + RegisterSigner("_foo", defaultSigner) + RegisterVerifier("_foo", defaultVerifier) + sc := NewSigningConfig("secret", "HS256", "").WithPlugin("_foo") + + err := bundle.GenerateSignature(sc, "", false) + if err != nil { + t.Fatal("Unexpected error:", err) + } + + if reflect.DeepEqual(signatures, bundle.Signatures) { + t.Fatal("Expected signatures to be different") + } + + current := bundle.Signatures + err = bundle.GenerateSignature(sc, "", false) + if err != nil { + t.Fatal("Unexpected error:", err) + } + + if !reflect.DeepEqual(current, bundle.Signatures) { + t.Fatal("Expected signatures to be same") + } +} + func TestFormatModulesRaw(t *testing.T) { bundle1 := Bundle{ diff --git a/bundle/keys.go b/bundle/keys.go index e1b916d25c..60e50e3c18 100644 --- a/bundle/keys.go +++ b/bundle/keys.go @@ -76,6 +76,7 @@ func (vc *VerificationConfig) GetPublicKey(id string) (*KeyConfig, error) { // SigningConfig represents the key configuration used to generate a signed bundle type SigningConfig struct { + Plugin string Key string Algorithm string ClaimsPath string @@ -88,12 +89,21 @@ func NewSigningConfig(key, alg, claimsPath string) *SigningConfig { } return &SigningConfig{ + Plugin: defaultSignerID, Key: key, Algorithm: alg, ClaimsPath: claimsPath, } } +// WithPlugin sets the signing plugin in the signing config +func (s *SigningConfig) WithPlugin(plugin string) *SigningConfig { + if plugin != "" { + s.Plugin = plugin + } + return s +} + // GetPrivateKey returns the private key or secret from the signing config func (s *SigningConfig) GetPrivateKey() (interface{}, error) { var priv string diff --git a/bundle/sign.go b/bundle/sign.go index 673d364f6e..13d6667f58 100644 --- a/bundle/sign.go +++ b/bundle/sign.go @@ -7,15 +7,48 @@ package bundle import ( "encoding/json" + "fmt" "github.com/open-policy-agent/opa/internal/jwx/jwa" "github.com/open-policy-agent/opa/internal/jwx/jws" ) +const defaultSignerID = "_default" + +var signers map[string]Signer + +// Signer is the interface expected for implementations that generate bundle signatures. +type Signer interface { + GenerateSignedToken([]FileInfo, *SigningConfig, string) (string, error) +} + +// GenerateSignedToken will retrieve the Signer implementation based on the Plugin specified +// in SigningConfig, and call its implementation of GenerateSignedToken. The signer generates +// a signed token given the list of files to be included in the payload and the bundle +// signing config. The keyID if non-empty, represents the value for the "keyid" claim in the token. +func GenerateSignedToken(files []FileInfo, sc *SigningConfig, keyID string) (string, error) { + var plugin string + // for backwards compatibility, check if there is no plugin specified, and use default + if sc.Plugin == "" { + plugin = defaultSignerID + } else { + plugin = sc.Plugin + } + signer, err := GetSigner(plugin) + if err != nil { + return "", err + } + return signer.GenerateSignedToken(files, sc, keyID) +} + +// DefaultSigner is the default bundle signing implementation. It signs bundles by generating +// a JWT and signing it using a locally-accessible private key. +type DefaultSigner struct{} + // GenerateSignedToken generates a signed token given the list of files to be // included in the payload and the bundle signing config. The keyID if non-empty, // represents the value for the "keyid" claim in the token -func GenerateSignedToken(files []FileInfo, sc *SigningConfig, keyID string) (string, error) { +func (*DefaultSigner) GenerateSignedToken(files []FileInfo, sc *SigningConfig, keyID string) (string, error) { payload, err := generatePayload(files, sc, keyID) if err != nil { return "", err @@ -71,3 +104,27 @@ func generatePayload(files []FileInfo, sc *SigningConfig, keyID string) ([]byte, } return json.Marshal(payload) } + +// GetSigner returns the Signer registered under the given id +func GetSigner(id string) (Signer, error) { + signer, ok := signers[id] + if !ok { + return nil, fmt.Errorf("no signer exists under id %s", id) + } + return signer, nil +} + +// RegisterSigner registers a Signer under the given id +func RegisterSigner(id string, s Signer) error { + if id == defaultSignerID { + return fmt.Errorf("signer id %s is reserved, use a different id", id) + } + signers[id] = s + return nil +} + +func init() { + signers = map[string]Signer{ + defaultSignerID: &DefaultSigner{}, + } +} diff --git a/bundle/sign_test.go b/bundle/sign_test.go index 9c46beb5c8..0a3adca327 100644 --- a/bundle/sign_test.go +++ b/bundle/sign_test.go @@ -113,6 +113,7 @@ func TestGenerateSignedTokenWithClaims(t *testing.T) { keyid := "foo" sc := NewSigningConfig("secret", "HS256", filepath.Join(rootDir, "claims.json")) + token, err := GenerateSignedToken(input, sc, keyid) if err != nil { t.Fatalf("Unexpected error %v", err) @@ -189,3 +190,35 @@ func TestGeneratePayload(t *testing.T) { t.Fatal("Unexpected claim \"keyid\" in token") } } + +type CustomSigner struct{} + +func (*CustomSigner) GenerateSignedToken(files []FileInfo, sc *SigningConfig, keyID string) (string, error) { + return "", nil +} + +func TestCustomSigner(t *testing.T) { + custom := &CustomSigner{} + err := RegisterSigner(defaultSignerID, custom) + if err == nil { + t.Fatalf("Expected error when registering with default ID") + } + RegisterSigner("_test", custom) + defaultSigner, err := GetSigner(defaultSignerID) + if err != nil { + t.Fatalf("Unexpected error %v", err) + } + if _, isDefault := defaultSigner.(*DefaultSigner); !isDefault { + t.Fatalf("Expected DefaultSigner to be registered at key %s", defaultSignerID) + } + customSigner, err := GetSigner("_test") + if err != nil { + t.Fatalf("Unexpected error %v", err) + } + if _, isCustom := customSigner.(*CustomSigner); !isCustom { + t.Fatalf("Expected CustomSigner to be registered at key _test") + } + if _, err = GetSigner("_unregistered"); err == nil { + t.Fatalf("Expected error when no Signer exists at provided key") + } +} diff --git a/bundle/verify.go b/bundle/verify.go index d26b8e1703..f76baf2c8e 100644 --- a/bundle/verify.go +++ b/bundle/verify.go @@ -20,9 +20,45 @@ import ( "github.com/pkg/errors" ) +const defaultVerifierID = "_default" + +var verifiers map[string]Verifier + +// Verifier is the interface expected for implementations that verify bundle signatures. +type Verifier interface { + VerifyBundleSignature(SignaturesConfig, *VerificationConfig) (map[string]FileInfo, error) +} + +// VerifyBundleSignature will retrieve the Verifier implementation based +// on the Plugin specified in SignaturesConfig, and call its implementation +// of VerifyBundleSignature. VerifyBundleSignature verifies the bundle signature +// using the given public keys or secret. If a signature is verified, it keeps +// track of the files specified in the JWT payload +func VerifyBundleSignature(sc SignaturesConfig, bvc *VerificationConfig) (map[string]FileInfo, error) { + // default implementation does not return a nil for map, so don't + // do it here either + files := make(map[string]FileInfo) + var plugin string + // for backwards compatibility, check if there is no plugin specified, and use default + if sc.Plugin == "" { + plugin = defaultVerifierID + } else { + plugin = sc.Plugin + } + verifier, err := GetVerifier(plugin) + if err != nil { + return files, err + } + return verifier.VerifyBundleSignature(sc, bvc) +} + +// DefaultVerifier is the default bundle verification implementation. It verifies bundles by checking +// the JWT signature using a locally-accessible public key. +type DefaultVerifier struct{} + // VerifyBundleSignature verifies the bundle signature using the given public keys or secret. // If a signature is verified, it keeps track of the files specified in the JWT payload -func VerifyBundleSignature(sc SignaturesConfig, bvc *VerificationConfig) (map[string]FileInfo, error) { +func (*DefaultVerifier) VerifyBundleSignature(sc SignaturesConfig, bvc *VerificationConfig) (map[string]FileInfo, error) { files := make(map[string]FileInfo) if len(sc.Signatures) == 0 { @@ -171,3 +207,27 @@ func VerifyBundleFile(path string, data bytes.Buffer, files map[string]FileInfo) delete(files, path) return nil } + +// GetVerifier returns the Verifier registered under the given id +func GetVerifier(id string) (Verifier, error) { + verifier, ok := verifiers[id] + if !ok { + return nil, fmt.Errorf("no verifier exists under id %s", id) + } + return verifier, nil +} + +// RegisterVerifier registers a Verifier under the given id +func RegisterVerifier(id string, v Verifier) error { + if id == defaultVerifierID { + return fmt.Errorf("verifier id %s is reserved, use a different id", id) + } + verifiers[id] = v + return nil +} + +func init() { + verifiers = map[string]Verifier{ + defaultVerifierID: &DefaultVerifier{}, + } +} diff --git a/bundle/verify_test.go b/bundle/verify_test.go index 07f24ee25a..a5f3096035 100644 --- a/bundle/verify_test.go +++ b/bundle/verify_test.go @@ -285,3 +285,35 @@ func TestVerifyBundleFile(t *testing.T) { }) } } + +type CustomVerifier struct{} + +func (*CustomVerifier) VerifyBundleSignature(sc SignaturesConfig, bvc *VerificationConfig) (map[string]FileInfo, error) { + return map[string]FileInfo{}, nil +} + +func TestCustomVerifier(t *testing.T) { + custom := &CustomVerifier{} + err := RegisterVerifier(defaultVerifierID, custom) + if err == nil { + t.Fatalf("Expected error when registering with default ID") + } + RegisterVerifier("_test", custom) + defaultVerifier, err := GetVerifier(defaultVerifierID) + if err != nil { + t.Fatalf("Unexpected error %v", err) + } + if _, isDefault := defaultVerifier.(*DefaultVerifier); !isDefault { + t.Fatalf("Expected DefaultVerifier to be registered at key %s", defaultVerifierID) + } + customVerifier, err := GetVerifier("_test") + if err != nil { + t.Fatalf("Unexpected error %v", err) + } + if _, isCustom := customVerifier.(*CustomVerifier); !isCustom { + t.Fatalf("Expected CustomVerifier to be registered at key _test") + } + if _, err = GetVerifier("_unregistered"); err == nil { + t.Fatalf("Expected error when no Verifier exists at provided key") + } +} diff --git a/cmd/build.go b/cmd/build.go index 3c99b2e2d1..9ec5d4f5b8 100644 --- a/cmd/build.go +++ b/cmd/build.go @@ -40,6 +40,7 @@ type buildParams struct { pubKeyID string claimsFile string excludeVerifyFiles []string + plugin string } func newBuildParams() buildParams { @@ -231,6 +232,7 @@ against OPA v0.22.0: // bundle signing config addSigningKeyFlag(buildCommand.Flags(), &buildParams.key) + addSigningPluginFlag(buildCommand.Flags(), &buildParams.plugin) addClaimsFileFlag(buildCommand.Flags(), &buildParams.claimsFile) RootCommand.AddCommand(buildCommand) @@ -246,7 +248,7 @@ func dobuild(params buildParams, args []string) error { return err } - bsc := buildSigningConfig(params.key, params.algorithm, params.claimsFile) + bsc := buildSigningConfig(params.key, params.algorithm, params.claimsFile, params.plugin) if bvc != nil || bsc != nil { if !params.bundleMode { @@ -324,10 +326,10 @@ func buildVerificationConfig(pubKey, pubKeyID, alg, scope string, excludeFiles [ return bundle.NewVerificationConfig(map[string]*keys.Config{pubKeyID: keyConfig}, pubKeyID, scope, excludeFiles), nil } -func buildSigningConfig(key, alg, claimsFile string) *bundle.SigningConfig { +func buildSigningConfig(key, alg, claimsFile, plugin string) *bundle.SigningConfig { if key == "" { return nil } - return bundle.NewSigningConfig(key, alg, claimsFile) + return bundle.NewSigningConfig(key, alg, claimsFile).WithPlugin(plugin) } diff --git a/cmd/flags.go b/cmd/flags.go index 65484cbc78..c904f3b4e1 100644 --- a/cmd/flags.go +++ b/cmd/flags.go @@ -98,6 +98,10 @@ func addSigningKeyFlag(fs *pflag.FlagSet, key *string) { fs.StringVarP(key, "signing-key", "", "", "set the secret (HMAC) or path of the PEM file containing the private key (RSA and ECDSA)") } +func addSigningPluginFlag(fs *pflag.FlagSet, plugin *string) { + fs.StringVarP(plugin, "signing-plugin", "", "", "name of the plugin to use for signing/verification (see https://openpolicyagent.org/docs/latest/management/#signature-plugin") +} + func addVerificationKeyFlag(fs *pflag.FlagSet, key *string) { fs.StringVarP(key, "verification-key", "", "", "set the secret (HMAC) or path of the PEM file containing the public key (RSA and ECDSA)") } diff --git a/cmd/sign.go b/cmd/sign.go index eeccbbc5fe..c9514853b3 100644 --- a/cmd/sign.go +++ b/cmd/sign.go @@ -32,6 +32,7 @@ type signCmdParams struct { claimsFile string outputFilePath string bundleMode bool + plugin string } const ( @@ -150,6 +151,7 @@ https://www.openpolicyagent.org/docs/latest/management/#signature-format. addSigningKeyFlag(signCommand.Flags(), &cmdParams.key) addClaimsFileFlag(signCommand.Flags(), &cmdParams.claimsFile) addSigningAlgFlag(signCommand.Flags(), &cmdParams.algorithm, defaultTokenSigningAlg) + addSigningPluginFlag(signCommand.Flags(), &cmdParams.plugin) signCommand.Flags().StringVarP(&cmdParams.outputFilePath, "output-file-path", "o", ".", "set the location for the .signatures.json file") @@ -172,8 +174,9 @@ func doSign(args []string, params signCmdParams) error { return err } - // generate the signed token - token, err := bundle.GenerateSignedToken(files, buildSigningConfig(params.key, params.algorithm, params.claimsFile), "") + signingConfig := buildSigningConfig(params.key, params.algorithm, params.claimsFile, params.plugin) + + token, err := bundle.GenerateSignedToken(files, signingConfig, "") if err != nil { return err } diff --git a/docs/content/management.md b/docs/content/management.md index e73c4ca2b2..4744a77da3 100644 --- a/docs/content/management.md +++ b/docs/content/management.md @@ -446,7 +446,21 @@ OPA out-of-band * `iss`: unused for verification even if present in payload +#### Signature Plugin +OPA supports the option to implement your own bundle signing and verification logic. This will be unnecessary +for most and is intended for advanced use cases, such as leveraging key-related services from cloud providers. +To implement your own signing and verification logic, you'll need to [extend OPA](../extensions). Here is +[an example](https://github.com/open-policy-agent/contrib/tree/master/custom_bundle_signing) to get you started. + +When registering custom signing and verification plugins, you will need to register the Signer and the Verifier +under the same plugin key, because the plugin key is stored in the signed bundle and informs OPA which Verifier +is capable of verifying the bundle, e.g. + +```go +bundle.RegisterSigner("custom", &CustomSigner{}) +bundle.RegisterVerifier("custom", &CustomVerifier{}) +``` ## Decision Logs