diff --git a/module/services/accounts/code/pkg/adapters/grpc_auth_interceptor.go b/module/services/accounts/code/pkg/adapters/grpc_auth_interceptor.go index 2f0b35204..c702d0047 100644 --- a/module/services/accounts/code/pkg/adapters/grpc_auth_interceptor.go +++ b/module/services/accounts/code/pkg/adapters/grpc_auth_interceptor.go @@ -16,8 +16,6 @@ import ( "google.golang.org/grpc/metadata" "google.golang.org/grpc/status" - "github.com/google/uuid" - "accounts/pkg/auth" "accounts/pkg/business" policyv1 "accounts/pkg/gen/saas/policy/v1" @@ -455,12 +453,19 @@ func stampForwardedGRPCIdentity(ctx context.Context, md metadata.MD) (context.Co func stampRequestIdentity(ctx context.Context, identity auth.RequestIdentity, assurance auth.Assurance) context.Context { subject := identity.EffectiveSubjectID() orgID := identity.OrgID.String() + // WithVerifiedDatabaseIdentity binds wool's organization as well as the + // database scope, so every verified door carries it — a delegated read, a + // module parent and the work-context authority all install a verified + // identity without passing through this transport. + // + // The user id is still stamped HERE, unconditionally, because the two have + // different availability: database scope needs BOTH uuids and that function + // binds nothing without them, while a verified user who has not selected an + // organization yet is an ordinary request whose logs and rate-limit key + // should still name them. ctx = auth.WithVerifiedDatabaseIdentity(ctx, subject, orgID) ctx = context.WithValue(ctx, wool.UserIDKey, subject) ctx = context.WithValue(ctx, wool.UserAuthIDKey, subject) - if identity.OrgID != uuid.Nil { - ctx = context.WithValue(ctx, wool.OrgIDKey, orgID) - } ctx = auth.WithVerifiedRequestIdentity(ctx, identity) ctx = auth.WithVerifiedActor(ctx, identity.Delegation) ctx = auth.WithVerifiedSessionID(ctx, identity.SessionID) diff --git a/module/services/accounts/code/pkg/auth/database_identity.go b/module/services/accounts/code/pkg/auth/database_identity.go index 2b129fcb4..3cef9020d 100644 --- a/module/services/accounts/code/pkg/auth/database_identity.go +++ b/module/services/accounts/code/pkg/auth/database_identity.go @@ -4,6 +4,8 @@ import ( "context" "errors" + "github.com/codefly-dev/core/wool" + "github.com/google/uuid" ) @@ -37,6 +39,25 @@ func WithVerifiedDatabaseIdentity(ctx context.Context, userID, tenantID string) if ctx == nil || userErr != nil || tenantErr != nil || user == uuid.Nil || tenant == uuid.Nil { return ctx } + // Propagate the same pair onto wool's identity dimension, so everything + // downstream — logs, traces, the rate-limit key, an organization-defaulted + // read — sees whose request this is without being handed it. + // + // Set as context values rather than through wool.Wool.WithOrgID, which + // cannot work: `with` assigns to the Wool's OWN ctx field, and wool.Get + // returns a fresh Wool per call, so the derived context is discarded and no + // later wool.Get(ctx).OrgID() can see it. The keys are exported and + // wool.Wool.lookup reads them off the context, so this is the propagation + // the accessors were written for. + // + // It is bound HERE and nowhere else for the same reason the verified + // identity is: this is the one door, it has already parsed both UUIDs, and + // the invalid path returned above — so wool's organization can only ever + // carry a verified value. That matters now that it is not merely an + // observability field: a key service selects an organization's envelope key + // by it. + ctx = context.WithValue(ctx, wool.OrgIDKey, tenant.String()) + ctx = context.WithValue(ctx, wool.UserIDKey, user.String()) return context.WithValue(ctx, verifiedDatabaseIdentityKey{}, verifiedDatabaseIdentity{ tenantID: tenant.String(), userID: user.String(), diff --git a/module/services/accounts/code/pkg/auth/wool_identity_sources_test.go b/module/services/accounts/code/pkg/auth/wool_identity_sources_test.go new file mode 100644 index 000000000..907fa79b2 --- /dev/null +++ b/module/services/accounts/code/pkg/auth/wool_identity_sources_test.go @@ -0,0 +1,76 @@ +package auth_test + +import ( + "go/ast" + "go/parser" + "go/token" + "io/fs" + "path/filepath" + "sort" + "strings" + "testing" + + "github.com/stretchr/testify/require" +) + +// sourceSitesSettingWoolIdentity returns every non-test file under the service +// that writes wool's identity keys, as a repository-relative path. +// +// Derived from the syntax tree rather than grepped: a grep for the key name also +// matches a read, and the question here is specifically who WRITES. +func sourceSitesSettingWoolIdentity(t *testing.T) []string { + t.Helper() + identityKeys := map[string]bool{"OrgIDKey": true, "UserIDKey": true} + found := map[string]bool{} + + root := "../.." + require.NoError(t, filepath.WalkDir(root, func(path string, entry fs.DirEntry, err error) error { + if err != nil { + return err + } + if entry.IsDir() { + switch entry.Name() { + case "node_modules", "gen", "generated", "testdata": + return fs.SkipDir + } + return nil + } + if !strings.HasSuffix(path, ".go") || strings.HasSuffix(path, "_test.go") { + return nil + } + file, parseErr := parser.ParseFile(token.NewFileSet(), path, nil, 0) + if parseErr != nil { + return nil + } + ast.Inspect(file, func(node ast.Node) bool { + call, ok := node.(*ast.CallExpr) + if !ok { + return true + } + // context.WithValue(ctx, wool.OrgIDKey, …) + if selector, ok := call.Fun.(*ast.SelectorExpr); ok && selector.Sel.Name == "WithValue" && len(call.Args) == 3 { + if key, ok := call.Args[1].(*ast.SelectorExpr); ok && identityKeys[key.Sel.Name] { + if pkg, ok := key.X.(*ast.Ident); ok && pkg.Name == "wool" { + found[filepath.ToSlash(filepath.Clean(path))] = true + } + } + } + // w.WithOrgID(…) / w.WithUserID(…) — a no-op as the API is built, + // but a writer in intent, so it counts. + if selector, ok := call.Fun.(*ast.SelectorExpr); ok { + if selector.Sel.Name == "WithOrgID" || selector.Sel.Name == "WithUserID" { + found[filepath.ToSlash(filepath.Clean(path))] = true + } + } + return true + }) + return nil + })) + + sites := make([]string, 0, len(found)) + for path := range found { + sites = append(sites, strings.TrimPrefix(path, "../../")) + } + sort.Strings(sites) + return sites +} diff --git a/module/services/accounts/code/pkg/auth/wool_identity_test.go b/module/services/accounts/code/pkg/auth/wool_identity_test.go new file mode 100644 index 000000000..98984fe1d --- /dev/null +++ b/module/services/accounts/code/pkg/auth/wool_identity_test.go @@ -0,0 +1,84 @@ +package auth_test + +import ( + "context" + "testing" + + "github.com/codefly-dev/core/wool" + "github.com/google/uuid" + "github.com/stretchr/testify/require" + + "accounts/pkg/auth" +) + +// The verified door is the one place wool's identity is bound, so everything +// downstream — logs, traces, the rate-limit key, an organization-defaulted read +// — sees whose request this is without being handed it. +func TestVerifiedDatabaseIdentityPopulatesWool(t *testing.T) { + user, org := uuid.NewString(), uuid.NewString() + + ctx := auth.WithVerifiedDatabaseIdentity(context.Background(), user, org) + + gotOrg, ok := wool.Get(ctx).OrgID() + require.True(t, ok, "wool must carry the organization after the verified door") + require.Equal(t, org, gotOrg) + gotUser, ok := wool.Get(ctx).UserID() + require.True(t, ok) + require.Equal(t, user, gotUser) +} + +// The invalid path returns the context untouched, so a malformed +// trusted-forwarding value cannot put an organization into wool either — the +// same fail-closed behaviour the verified identity already had. +func TestAMalformedIdentityPopulatesNothing(t *testing.T) { + for name, identity := range map[string][2]string{ + "unparseable org": {uuid.NewString(), "not-a-uuid"}, + "unparseable user": {"not-a-uuid", uuid.NewString()}, + "nil org": {uuid.NewString(), uuid.Nil.String()}, + "nil user": {uuid.Nil.String(), uuid.NewString()}, + "both empty": {"", ""}, + } { + t.Run(name, func(t *testing.T) { + ctx := auth.WithVerifiedDatabaseIdentity(context.Background(), identity[0], identity[1]) + + _, ok := wool.Get(ctx).OrgID() + require.False(t, ok, "a malformed identity must leave wool empty") + _, _, verified := auth.VerifiedDatabaseIdentity(ctx) + require.False(t, verified) + }) + } +} + +// wool's organization is now read by things that matter (the rate-limit key, an +// organization-defaulted audit read), so it must only ever carry a verified +// value. This pins that the verified door is the ONLY writer: the key is +// exported, so anything may set it, and a second writer — especially one fed by +// caller-controlled input — would make wool's organization forgeable. +// +// It is deliberately NOT an authority source even so: pkg/auth states that +// transport headers and wool values are insufficient database or +// authorization-cache authority, and this does not change that. The point of one +// writer is that the observability dimension cannot be made to lie. +func TestOnlyTheVerifiedDoorWritesWoolIdentity(t *testing.T) { + // Two writers, and each one's reason is why the list is pinned rather than + // merely bounded: + // + // database_identity.go — the verified door. Binds the organization (and + // the user) on every path that installs a verified identity, which is + // the only way wool's organization can be trusted at all. + // grpc_auth_interceptor.go — the user id only, unconditionally, because a + // verified user who has not selected an organization binds no database + // scope and so reaches no door above, yet should still be named in logs + // and in the rate-limit key. + // + // A THIRD writer is what this refuses. The keys are exported, so anything + // may set them, and a writer fed by caller-controlled input would make the + // dimension forgeable. + writers := sourceSitesSettingWoolIdentity(t) + require.Equal(t, []string{ + "pkg/adapters/grpc_auth_interceptor.go", + "pkg/auth/database_identity.go", + }, writers, + "wool's identity keys have exactly two writers, both named above; a third makes the dimension "+ + "forgeable, and the rate limiter and the organization-defaulted audit read both consult it") +} diff --git a/module/services/accounts/code/pkg/keyservice/keyservice.go b/module/services/accounts/code/pkg/keyservice/keyservice.go index 13fade11f..dac1eb8f7 100644 --- a/module/services/accounts/code/pkg/keyservice/keyservice.go +++ b/module/services/accounts/code/pkg/keyservice/keyservice.go @@ -124,3 +124,44 @@ func (e *statusError) Error() string { return fmt.Sprintf("%s request returned %d", e.service, e.status) } func (e *statusError) HTTPStatusCode() int { return e.status } + +// KeyFactory binds another key of the same key service, named by a reference the +// platform recorded for an organization. +// +// The reference is OPAQUE above this interface: only the backend knows whether +// it is a Vault transit key name or a cloud resource name, which is what keeps +// the vendor out of the caller and out of the database. +type KeyFactory interface { + SealerFor(ctx context.Context, keyRef string) (Sealer, error) +} + +// OrgKeyBinding is what the platform recorded about an organization's key. +type OrgKeyBinding struct { + // KeyRef is the key, as the bound backend names one. + KeyRef string + // CustomerHeld marks a key this deployment does not control. + CustomerHeld bool + // RevokedReason is set once the key is gone — destroyed on request, or + // revoked by the customer. Every credential sealed under it is unreadable + // from that moment, which is the point. + RevokedReason string +} + +// Revoked reports whether the key is gone. +func (b OrgKeyBinding) Revoked() bool { return b.RevokedReason != "" } + +// OrgKeyBindings answers which key seals an organization's credentials. A nil +// binding means the organization has none of its own and is sealed under the +// deployment's key, which is the default and stays the default. +type OrgKeyBindings interface { + OrgKeyBinding(ctx context.Context, orgID string) (*OrgKeyBinding, error) +} + +// ErrKeyRevoked reports that an organization's key is gone, so its credentials +// are unreadable by design. +// +// It is deliberately NOT business.ErrInvalidSecretEnvelope: callers treat that +// sentinel as a credential the user should re-enter, and telling a customer to +// reconnect a source whose data they instructed us to destroy is the opposite of +// what crypto-shredding is for. +var ErrKeyRevoked = errors.New("key service: the organization's key has been revoked") diff --git a/module/services/accounts/code/pkg/keyservice/kms.go b/module/services/accounts/code/pkg/keyservice/kms.go index df381ea5b..cd26f90db 100644 --- a/module/services/accounts/code/pkg/keyservice/kms.go +++ b/module/services/accounts/code/pkg/keyservice/kms.go @@ -494,6 +494,9 @@ func (k *KMSSealer) Open(ctx context.Context, purpose, payload string) (string, } func (k *KMSSealer) MAC(ctx context.Context, plaintext string) (string, error) { + if !k.macKeyBound() { + return "", errors.New("key service: this sealer is bound to an organization's key and may not compute the keyed hash; API-key lookup is by that hash, so it is always the deployment's own") + } var result struct { MAC string `json:"mac"` } @@ -579,3 +582,35 @@ func (w *KMSKeyWrap) UnwrapSigningKey(ctx context.Context, wrapped string) (ed25 } return ed25519.NewKeyFromSeed(seed), nil } + +// SealerFor binds another Cloud KMS key, for an organization whose credentials +// are sealed under a key of its own. +// +// The key is validated and round-tripped exactly as the deployment's own is: +// purpose, algorithm, and a seal/open probe. An organization pointed at a key +// that is well-formed and simply not usable must fail when the binding is read, +// not at the first credential that needs it. +func (k *KMSSealer) SealerFor(ctx context.Context, keyRef string) (Sealer, error) { + if !cryptoKeyName.MatchString(keyRef) { + return nil, fmt.Errorf("key service: an organization's Cloud KMS key must be a cryptoKey resource name (projects//locations//keyRings//cryptoKeys/), got %q", keyRef) + } + if keyRef == k.envelopeKey { + return nil, fmt.Errorf("key service: %s is this deployment's own envelope key and cannot be an organization's", keyRef) + } + sealer, err := newKMSEnvelope(ctx, k.client, keyRef) + if err != nil { + return nil, err + } + // The keyed hash never moves off the deployment's key, so a per-organization + // sealer carries no MAC key and MAC on it is a programming error rather than + // a silent hash under the wrong key. + return sealer, nil +} + +// MAC refuses on a sealer bound to an organization's key. +// +// API-key lookup is BY the hash, so a hash computed under an organization's key +// could never be found: the lookup happens before the organization is known. The +// deployment's own sealer is the only one that may compute it, and this refuses +// rather than quietly producing a value nothing will ever match. +func (k *KMSSealer) macKeyBound() bool { return k.macVersion != "" } diff --git a/module/services/accounts/code/pkg/keyservice/tenant_cipher.go b/module/services/accounts/code/pkg/keyservice/tenant_cipher.go new file mode 100644 index 000000000..25d2d57aa --- /dev/null +++ b/module/services/accounts/code/pkg/keyservice/tenant_cipher.go @@ -0,0 +1,244 @@ +package keyservice + +import ( + "context" + "errors" + "fmt" + "sync" +) + +// TenantCipher seals an organization's credentials under that organization's own +// key when it has one, and under the deployment's key when it does not. +// +// Having none is the default and stays the default: "the customer does not care" +// is the common case, and a deployment that had to provision a key per +// organization before it could store anything would be worse for everyone. A +// binding is an OVERRIDE. +// +// Which organization a value belongs to comes from the ROW being sealed — +// sub.OrgID, source.OrgID, the org a provider is configured for — not from the +// request context, and not threaded down from a caller's caller. Two reasons it +// cannot be the context: +// +// - the delivery paths have none. The outbound webhook sender opens a +// subscription's signing secret as app_webhook_worker, outside any tenant +// transaction, so there is no verified organization to read; the row is all +// there is. +// - the key should follow the DATA. A value's organization is a property of +// the row, and sealing it under whatever organization the current request +// happens to be scoped to would be a different question with the same +// answer most of the time, which is the worst kind of coincidence. +// +// Where the context DOES carry a verified organization, the two must agree, and +// requireConsistentScope refuses when they do not: the row's organization and +// the transaction's scope disagreeing means one of them is wrong, and guessing +// which seals a customer's credential under another customer's key. Row-level +// security binds the transaction, never this argument, so nothing else catches +// it. +// +// An absent organization is refused outright. A fallback to the deployment's key +// would betray exactly the customer who went to the trouble of bringing their +// own. +type TenantCipher struct { + // deployment seals for an organization with no key of its own, and is the + // only thing that computes the keyed hash. + deployment *Cipher + factory KeyFactory + bindings OrgKeyBindings + + // sealers caches by key reference, not by organization: two organizations + // pointed at one key share a sealer, and building one is a round trip to the + // key service (Cloud KMS validates and round-trips the key), far too much to + // repeat per request. + mu sync.RWMutex + sealers map[string]Sealer +} + +func NewTenantCipher(deployment *Cipher, factory KeyFactory, bindings OrgKeyBindings) (*TenantCipher, error) { + if deployment == nil || factory == nil || bindings == nil { + return nil, errors.New("key service: a tenant cipher needs the deployment cipher, a key factory and a binding source") + } + return &TenantCipher{ + deployment: deployment, factory: factory, bindings: bindings, + sealers: map[string]Sealer{}, + }, nil +} + +// EncryptSecret and DecryptSecret keep the deployment-wide behaviour for values +// that are not an organization's — a person's MFA seed and WebAuthn credential, +// which belong to someone who may be in many organizations. +func (c *TenantCipher) EncryptSecret(ctx context.Context, purpose, plaintext string) (string, error) { + return c.deployment.EncryptSecret(ctx, purpose, plaintext) +} + +func (c *TenantCipher) DecryptSecret(ctx context.Context, purpose, stored string) (string, error) { + return c.deployment.DecryptSecret(ctx, purpose, stored) +} + +// HashKey is always the deployment's: API-key lookup is BY the hash, so a hash +// computed under an organization's key could never be found — the lookup is what +// discovers which organization the key belongs to. +func (c *TenantCipher) HashKey(ctx context.Context, plaintext string) (string, error) { + return c.deployment.HashKey(ctx, plaintext) +} + +func (c *TenantCipher) CandidateHashes(ctx context.Context, plaintext string) ([]string, error) { + return c.deployment.CandidateHashes(ctx, plaintext) +} + +// EncryptTenantSecret seals under the organization's key, or the deployment's +// when it has none. +func (c *TenantCipher) EncryptTenantSecret(ctx context.Context, orgID, purpose, plaintext string) (string, error) { + sealer, err := c.sealerFor(ctx, orgID) + if err != nil { + return "", err + } + if sealer == nil { + return c.deployment.EncryptSecret(ctx, purpose, plaintext) + } + if purpose == "" || plaintext == "" { + return "", errors.New("key service: sealing requires a purpose and a plaintext") + } + payload, err := sealer.Seal(ctx, purpose, plaintext) + if err != nil { + return "", err + } + return Envelope{Backend: sealer.Tag(), Payload: payload}.String(), nil +} + +// DecryptTenantSecret opens a value through whichever bound key sealed it: the +// organization's own, or the deployment's. +// +// A value is routed by what the ENVELOPE names, not by what the binding says +// now, because the two legitimately disagree during a cutover — an organization +// that has just been given its own key still holds credentials sealed under the +// deployment's, and those must keep opening until they are re-sealed. +func (c *TenantCipher) DecryptTenantSecret(ctx context.Context, orgID, purpose, stored string) (string, error) { + if err := requireConsistentScope(ctx, orgID, "opening"); err != nil { + return "", err + } + binding, err := c.bindings.OrgKeyBinding(ctx, orgID) + if err != nil { + return "", fmt.Errorf("key service: read the organization's key binding: %w", err) + } + envelope, err := ParseEnvelope(stored) + if err != nil { + return "", err + } + if binding != nil { + if binding.Revoked() { + // Named, and NOT an invalid envelope: this credential is unreadable + // because the customer's key is gone, which is the outcome they + // asked for. Reporting it as a broken credential would tell them to + // reconnect the source whose data they destroyed. + return "", fmt.Errorf("%w (%s): the credentials sealed under it cannot be read", ErrKeyRevoked, binding.RevokedReason) + } + sealer, err := c.boundSealer(ctx, binding.KeyRef) + if err != nil { + return "", err + } + if sealer.Accepts(envelope) { + return sealer.Open(ctx, purpose, envelope.Payload) + } + } + // Sealed under the deployment's key — either the organization has no key of + // its own, or it has one and this value predates it. + return c.deployment.DecryptSecret(ctx, purpose, stored) +} + +// ResealTenantSecret moves one value onto the organization's current key. A value +// already under it is returned unchanged with false, so a sweep is restart-safe. +func (c *TenantCipher) ResealTenantSecret(ctx context.Context, orgID, purpose, stored string) (string, bool, error) { + sealer, err := c.sealerFor(ctx, orgID) + if err != nil { + return "", false, err + } + envelope, err := ParseEnvelope(stored) + if err != nil { + return "", false, err + } + target := c.deployment.seal + if sealer != nil { + target = sealer + } + if target.Accepts(envelope) { + return stored, false, nil + } + plaintext, err := c.DecryptTenantSecret(ctx, orgID, purpose, stored) + if err != nil { + return "", false, err + } + resealed, err := c.EncryptTenantSecret(ctx, orgID, purpose, plaintext) + if err != nil { + return "", false, err + } + return resealed, true, nil +} + +// sealerFor resolves the organization's sealer, or nil when it has none and the +// deployment's key applies. +func (c *TenantCipher) sealerFor(ctx context.Context, orgID string) (Sealer, error) { + if err := requireConsistentScope(ctx, orgID, "sealing"); err != nil { + return nil, err + } + binding, err := c.bindings.OrgKeyBinding(ctx, orgID) + if err != nil { + return nil, fmt.Errorf("key service: read the organization's key binding: %w", err) + } + if binding == nil { + return nil, nil + } + if binding.Revoked() { + return nil, fmt.Errorf("%w (%s): nothing further may be sealed under it", ErrKeyRevoked, binding.RevokedReason) + } + return c.boundSealer(ctx, binding.KeyRef) +} + +func (c *TenantCipher) boundSealer(ctx context.Context, keyRef string) (Sealer, error) { + c.mu.RLock() + cached, ok := c.sealers[keyRef] + c.mu.RUnlock() + if ok { + return cached, nil + } + sealer, err := c.factory.SealerFor(ctx, keyRef) + if err != nil { + return nil, fmt.Errorf("key service: bind the organization's key: %w", err) + } + c.mu.Lock() + // Another goroutine may have won the race; keep one sealer per key so the + // cache cannot grow a second entry for the same reference. + if existing, raced := c.sealers[keyRef]; raced { + sealer = existing + } else { + c.sealers[keyRef] = sealer + } + c.mu.Unlock() + return sealer, nil +} + +// VerifiedScope reports the organization the caller's transaction is scoped to, +// when it is running inside one. A path with no request identity — a delivery +// worker — reports false, which is not an error. +// +// It is a variable so the wiring supplies it: this package must not import the +// request-identity plumbing, and a test must be able to drive both answers. +var VerifiedScope func(ctx context.Context) (orgID string, ok bool) + +// requireConsistentScope refuses a row whose organization is absent, and one +// that disagrees with the transaction's own scope. +func requireConsistentScope(ctx context.Context, orgID, verb string) error { + if orgID == "" { + return fmt.Errorf("key service: %s an organization's secret requires the organization it belongs to", verb) + } + if VerifiedScope == nil { + return nil + } + scope, ok := VerifiedScope(ctx) + if !ok || scope == orgID { + // No request scope is the delivery-worker case, which is legitimate. + return nil + } + return fmt.Errorf("key service: refusing to %s a secret for organization %s inside a transaction scoped to %s: the row and the scope disagree, and row-level security binds the scope rather than this argument, so one of them is wrong", + verb, orgID, scope) +} diff --git a/module/services/accounts/code/pkg/keyservice/tenant_cipher_test.go b/module/services/accounts/code/pkg/keyservice/tenant_cipher_test.go new file mode 100644 index 000000000..de3375be1 --- /dev/null +++ b/module/services/accounts/code/pkg/keyservice/tenant_cipher_test.go @@ -0,0 +1,289 @@ +package keyservice + +// The per-organization key. What has to hold: +// +// - an organization with no key of its own is sealed under the deployment's, +// and that stays the default; +// - an organization with one is sealed under it, and a value sealed under the +// deployment's key BEFORE the binding existed still opens; +// - a revoked key makes its credentials unreadable, reported as that and not +// as a credential the user should re-enter; +// - an absent organization is REFUSED, never quietly sealed under the +// deployment's key — that would silently betray the one customer who asked +// for their own. + +import ( + "context" + "errors" + "strings" + "testing" + + "github.com/stretchr/testify/require" + + "accounts/pkg/business" +) + +// bindings is an in-memory OrgKeyBindings. +type bindings struct { + rows map[string]*OrgKeyBinding + calls int + err error +} + +func (b *bindings) OrgKeyBinding(_ context.Context, orgID string) (*OrgKeyBinding, error) { + b.calls++ + if b.err != nil { + return nil, b.err + } + return b.rows[orgID], nil +} + +// keyFactory builds a distinct sealer per key reference, counting how often it +// is asked so the cache can be asserted. +type keyFactory struct { + built int + err error +} + +func (f *keyFactory) SealerFor(_ context.Context, keyRef string) (Sealer, error) { + f.built++ + if f.err != nil { + return nil, f.err + } + return testSealer{tag: "test-key-service", key: keyRef}, nil +} + +// testSealer is a sealer whose stored payload names its key, the property every +// real backend has and the one routing depends on. +type testSealer struct{ tag, key string } + +func (s testSealer) Tag() string { return s.tag } +func (s testSealer) Identity() string { return s.tag + ":" + s.key } + +func (s testSealer) Accepts(envelope Envelope) bool { + if envelope.Backend != s.tag { + return false + } + key, _, found := strings.Cut(envelope.Payload, "|") + return found && key == s.key +} + +func (s testSealer) Seal(_ context.Context, purpose, plaintext string) (string, error) { + return s.key + "|" + purpose + "|" + plaintext, nil +} + +func (s testSealer) Open(_ context.Context, purpose, payload string) (string, error) { + parts := strings.SplitN(payload, "|", 3) + if len(parts) != 3 || parts[0] != s.key || parts[1] != purpose { + return "", business.ErrInvalidSecretEnvelope + } + return parts[2], nil +} + +func (s testSealer) MAC(_ context.Context, plaintext string) (string, error) { + return s.key + ":" + plaintext, nil +} + +func newTenantCipher(t *testing.T, rows map[string]*OrgKeyBinding) (*TenantCipher, *bindings, *keyFactory) { + t.Helper() + deployment, err := NewCipher(testSealer{tag: "test-key-service", key: "deployment"}, nil) + require.NoError(t, err) + source := &bindings{rows: rows} + factory := &keyFactory{} + cipher, err := NewTenantCipher(deployment, factory, source) + require.NoError(t, err) + return cipher, source, factory +} + +func TestTenantCipherSealsUnderTheDeploymentKeyWhenAnOrganizationHasNone(t *testing.T) { + cipher, _, factory := newTenantCipher(t, nil) + + stored, err := cipher.EncryptTenantSecret(t.Context(), "org-a", "github-connector:s1", "ghp_token") + require.NoError(t, err) + require.Contains(t, stored, "deployment|", "no binding means the deployment's key, which is the default") + require.Zero(t, factory.built, "no organization key was needed, so none may be built") + + opened, err := cipher.DecryptTenantSecret(t.Context(), "org-a", "github-connector:s1", stored) + require.NoError(t, err) + require.Equal(t, "ghp_token", opened) +} + +func TestTenantCipherSealsUnderTheOrganizationsOwnKey(t *testing.T) { + cipher, _, _ := newTenantCipher(t, map[string]*OrgKeyBinding{ + "org-a": {KeyRef: "acme-key", CustomerHeld: true}, + }) + + stored, err := cipher.EncryptTenantSecret(t.Context(), "org-a", "org-idp:org-a", "idp-secret") + require.NoError(t, err) + require.Contains(t, stored, "acme-key|") + require.NotContains(t, stored, "deployment|") + + opened, err := cipher.DecryptTenantSecret(t.Context(), "org-a", "org-idp:org-a", stored) + require.NoError(t, err) + require.Equal(t, "idp-secret", opened) + + // Another organization's key must not open it, even with the same purpose. + cipherB, _, _ := newTenantCipher(t, map[string]*OrgKeyBinding{ + "org-b": {KeyRef: "other-key"}, + }) + _, err = cipherB.DecryptTenantSecret(t.Context(), "org-b", "org-idp:org-a", stored) + require.Error(t, err) +} + +// The cutover case: a value sealed under the deployment's key before the +// organization was given one must keep opening, or granting a customer a key +// would make their existing credentials unreadable. +func TestTenantCipherStillOpensValuesSealedBeforeTheBindingExisted(t *testing.T) { + cipher, source, _ := newTenantCipher(t, nil) + before, err := cipher.EncryptTenantSecret(t.Context(), "org-a", "webhook-signing:w1", "whsec_old") + require.NoError(t, err) + + // The platform now gives the organization its own key. + source.rows = map[string]*OrgKeyBinding{"org-a": {KeyRef: "acme-key"}} + + opened, err := cipher.DecryptTenantSecret(t.Context(), "org-a", "webhook-signing:w1", before) + require.NoError(t, err, "a value predating the binding must still open") + require.Equal(t, "whsec_old", opened) + + // New values go under the organization's key. + after, err := cipher.EncryptTenantSecret(t.Context(), "org-a", "webhook-signing:w1", "whsec_new") + require.NoError(t, err) + require.Contains(t, after, "acme-key|") + + // And the sweep moves the old one, idempotently. + resealed, changed, err := cipher.ResealTenantSecret(t.Context(), "org-a", "webhook-signing:w1", before) + require.NoError(t, err) + require.True(t, changed) + require.Contains(t, resealed, "acme-key|") + _, changed, err = cipher.ResealTenantSecret(t.Context(), "org-a", "webhook-signing:w1", resealed) + require.NoError(t, err) + require.False(t, changed, "re-sealing what is already under the key must change nothing") +} + +// Crypto-shredding: the credentials are unreadable, and the reason says so +// rather than reading as a credential the user should re-enter. +func TestTenantCipherRefusesARevokedKeyWithoutCondemningTheCredential(t *testing.T) { + cipher, source, _ := newTenantCipher(t, map[string]*OrgKeyBinding{ + "org-a": {KeyRef: "acme-key", CustomerHeld: true}, + }) + stored, err := cipher.EncryptTenantSecret(t.Context(), "org-a", "org-idp:org-a", "idp-secret") + require.NoError(t, err) + + source.rows["org-a"] = &OrgKeyBinding{ + KeyRef: "acme-key", CustomerHeld: true, RevokedReason: "customer instruction", + } + + _, err = cipher.DecryptTenantSecret(t.Context(), "org-a", "org-idp:org-a", stored) + require.ErrorIs(t, err, ErrKeyRevoked) + require.NotErrorIs(t, err, business.ErrInvalidSecretEnvelope, + "a destroyed key is not a credential to re-enter — telling the customer to reconnect would be the opposite of what they asked for") + require.Contains(t, err.Error(), "customer instruction") + + // And nothing further may be sealed under it. + _, err = cipher.EncryptTenantSecret(t.Context(), "org-a", "org-idp:org-a", "new-secret") + require.ErrorIs(t, err, ErrKeyRevoked) +} + +// The property the explicit argument exists for. +func TestTenantCipherRefusesAnAbsentOrganization(t *testing.T) { + cipher, _, _ := newTenantCipher(t, map[string]*OrgKeyBinding{ + "org-a": {KeyRef: "acme-key"}, + }) + + _, err := cipher.EncryptTenantSecret(t.Context(), "", "org-idp:org-a", "secret") + require.Error(t, err, "an absent organization must never fall back to the deployment's key") + require.Contains(t, err.Error(), "requires the organization") + + _, err = cipher.DecryptTenantSecret(t.Context(), "", "org-idp:org-a", "cfs1:test-key-service:acme-key|p|v") + require.Error(t, err) + require.Contains(t, err.Error(), "requires the organization") +} + +// A sealer is built once per key reference, not per request: building one is a +// round trip to the key service. +func TestTenantCipherCachesOneSealerPerKey(t *testing.T) { + cipher, _, factory := newTenantCipher(t, map[string]*OrgKeyBinding{ + "org-a": {KeyRef: "shared-key"}, + "org-b": {KeyRef: "shared-key"}, + }) + for range 5 { + _, err := cipher.EncryptTenantSecret(t.Context(), "org-a", "p", "v") + require.NoError(t, err) + _, err = cipher.EncryptTenantSecret(t.Context(), "org-b", "p", "v") + require.NoError(t, err) + } + require.Equal(t, 1, factory.built, "two organizations on one key share one sealer") +} + +// A binding that cannot be read must refuse, not fall through to the +// deployment's key: "the lookup failed" and "there is no binding" are different +// answers, and conflating them seals a customer's data under our key. +func TestTenantCipherRefusesWhenTheBindingCannotBeRead(t *testing.T) { + cipher, source, _ := newTenantCipher(t, nil) + source.err = errors.New("database unavailable") + + _, err := cipher.EncryptTenantSecret(t.Context(), "org-a", "p", "v") + require.Error(t, err) + require.Contains(t, err.Error(), "database unavailable") + + _, err = cipher.DecryptTenantSecret(t.Context(), "org-a", "p", "cfs1:test-key-service:deployment|p|v") + require.Error(t, err) + require.Contains(t, err.Error(), "database unavailable") +} + +// The keyed hash never moves off the deployment's key, whatever the organization +// has: API-key lookup is BY the hash, and the lookup is what discovers the +// organization. +func TestTenantCipherKeyedHashIsAlwaysTheDeploymentsOwn(t *testing.T) { + cipher, _, _ := newTenantCipher(t, map[string]*OrgKeyBinding{ + "org-a": {KeyRef: "acme-key"}, + }) + hash, err := cipher.HashKey(t.Context(), "cfly_sk_live_example") + require.NoError(t, err) + require.True(t, strings.HasPrefix(hash, "deployment:"), + "the hash must be the deployment's, got %q", hash) +} + +// The row's organization and the transaction's scope must agree. Row-level +// security binds the SCOPE, never the argument, so a disagreement means one of +// them is wrong — and guessing which seals a customer's credential under another +// customer's key. Nothing else in the stack catches this. +func TestTenantCipherRefusesARowWhoseOrganizationDisagreesWithTheScope(t *testing.T) { + cipher, _, _ := newTenantCipher(t, map[string]*OrgKeyBinding{ + "org-a": {KeyRef: "acme-key"}, + "org-b": {KeyRef: "other-key"}, + }) + + scoped := func(org string) context.Context { + return context.WithValue(t.Context(), scopeKeyForTest{}, org) + } + previous := VerifiedScope + VerifiedScope = func(ctx context.Context) (string, bool) { + org, ok := ctx.Value(scopeKeyForTest{}).(string) + return org, ok + } + t.Cleanup(func() { VerifiedScope = previous }) + + // Agreeing is fine. + _, err := cipher.EncryptTenantSecret(scoped("org-a"), "org-a", "org-idp:org-a", "secret") + require.NoError(t, err) + + // Disagreeing is refused, both directions, on both verbs. + _, err = cipher.EncryptTenantSecret(scoped("org-b"), "org-a", "org-idp:org-a", "secret") + require.Error(t, err) + require.Contains(t, err.Error(), "the row and the scope disagree") + + stored, err := cipher.EncryptTenantSecret(scoped("org-a"), "org-a", "org-idp:org-a", "secret") + require.NoError(t, err) + _, err = cipher.DecryptTenantSecret(scoped("org-b"), "org-a", "org-idp:org-a", stored) + require.Error(t, err) + require.Contains(t, err.Error(), "the row and the scope disagree") + + // A path with NO request scope is the delivery worker, and is legitimate: + // the outbound sender opens a subscription's secret as app_webhook_worker, + // outside any tenant transaction, with only the row to go on. + _, err = cipher.DecryptTenantSecret(t.Context(), "org-a", "org-idp:org-a", stored) + require.NoError(t, err, "a delivery worker has no request scope and must still open the row's secret") +} + +type scopeKeyForTest struct{} diff --git a/module/services/accounts/code/pkg/keyservice/vault.go b/module/services/accounts/code/pkg/keyservice/vault.go index 023666544..1e6acad77 100644 --- a/module/services/accounts/code/pkg/keyservice/vault.go +++ b/module/services/accounts/code/pkg/keyservice/vault.go @@ -16,11 +16,16 @@ import ( "accounts/pkg/vaultconnection" ) -// transitKeyName is the Transit key every purpose is sealed and hashed under. -// It is a single name on purpose: the keyed hash behind API-key lookup must stay -// comparable with every hash already stored, so the key it is computed under -// cannot move. -const transitKeyName = "api-keys" +// defaultTransitKeyName is the Transit key this deployment seals under when an +// organization has no key of its own, and the key the keyed hash is ALWAYS +// computed under. +// +// The hash cannot move: API-key lookup is BY that value, so a hash computed +// under another key stops matching every key already stored. The envelope key +// can move — per organization, or on a cutover — because every sealed value +// records which key sealed it. Two different questions, which is why they are +// two fields on the sealer rather than one name. +const defaultTransitKeyName = "api-keys" // VaultSealer is the Vault Transit backend: the AppRole + Transit binding // accounts has always used, behind the key-service seam and otherwise @@ -31,6 +36,10 @@ type VaultSealer struct { token string connection *vaultconnection.Connection client *http.Client + // envelopeKey seals and opens. Empty means defaultTransitKeyName, and a + // value sealed under that key is stored in the pre-per-key payload shape so + // nothing already written moves. + envelopeKey string } // NewVaultSealer binds the cell's Vault through the `vault` configuration @@ -52,12 +61,43 @@ func NewVaultSealerDirect(address, token string) *VaultSealer { func (v *VaultSealer) Tag() string { return TagVaultTransit } // Identity is the tag: see Accepts. -func (v *VaultSealer) Identity() string { return TagVaultTransit } +func (v *VaultSealer) Identity() string { return TagVaultTransit + ":" + v.transitKey() } + +// transitKey is the Transit key this sealer seals under. +func (v *VaultSealer) transitKey() string { + if v.envelopeKey == "" { + return defaultTransitKeyName + } + return v.envelopeKey +} + +// Accepts requires the tag and the Transit key the payload names. +// +// A Vault ciphertext carries its key VERSION (`vault:v:`) and not its key +// NAME, so the payload has to name the key itself or a per-organization key +// could not be told from the deployment's on read — and Vault would answer a +// foreign key's ciphertext with the same failure it gives a corrupt one. +func (v *VaultSealer) Accepts(envelope Envelope) bool { + if envelope.Backend != TagVaultTransit { + return false + } + key, _ := splitVaultPayload(envelope.Payload) + return key == v.transitKey() +} -// Accepts needs only the tag: the Transit key is the `api-keys` constant rather -// than a configured name, so one deployment has exactly one Vault envelope key -// and nothing to disambiguate. -func (v *VaultSealer) Accepts(envelope Envelope) bool { return envelope.Backend == TagVaultTransit } +// splitVaultPayload reads the optional key segment. +// +// The pre-per-key shape is a single base64url ciphertext, which belongs to +// defaultTransitKeyName — that is what every value written before per-organization +// keys existed looks like, and it keeps opening unchanged. base64url contains no +// ":", so a payload carrying one is the keyed shape and the two cannot be +// confused. +func splitVaultPayload(payload string) (key, ciphertext string) { + if name, rest, found := strings.Cut(payload, ":"); found { + return name, rest + } + return defaultTransitKeyName, payload +} // Health hits Vault's /v1/sys/health and returns nil only on the canonical // 200 / 429 / 472 / 473 codes that signal a reachable and un-sealed Vault. Used @@ -89,7 +129,8 @@ func (v *VaultSealer) MAC(ctx context.Context, plaintext string) (string, error) if err != nil { return "", err } - result, err := v.request(ctx, "/v1/transit/hmac/"+transitKeyName, string(body)) + // defaultTransitKeyName, never v.transitKey(): see the constant. + result, err := v.request(ctx, "/v1/transit/hmac/"+defaultTransitKeyName, string(body)) if err != nil { return "", fmt.Errorf("vault hmac request: %w", err) } @@ -111,7 +152,7 @@ func (v *VaultSealer) Seal(ctx context.Context, purpose, plaintext string) (stri if err != nil { return "", err } - result, err := v.request(ctx, "/v1/transit/encrypt/"+transitKeyName, string(body)) + result, err := v.request(ctx, "/v1/transit/encrypt/"+v.transitKey(), string(body)) if err != nil { return "", err } @@ -119,11 +160,19 @@ func (v *VaultSealer) Seal(ctx context.Context, purpose, plaintext string) (stri if !ok || ciphertext == "" { return "", errors.New("vault transit response missing ciphertext") } - return base64.RawURLEncoding.EncodeToString([]byte(ciphertext)), nil + encoded := base64.RawURLEncoding.EncodeToString([]byte(ciphertext)) + if v.envelopeKey == "" { + // The deployment's own key keeps the shape it has always written, so a + // value sealed before per-organization keys existed and one sealed now + // are byte-identical in form. + return encoded, nil + } + return v.envelopeKey + ":" + encoded, nil } func (v *VaultSealer) Open(ctx context.Context, purpose, payload string) (string, error) { - ciphertext, err := base64.RawURLEncoding.DecodeString(payload) + _, encoded := splitVaultPayload(payload) + ciphertext, err := base64.RawURLEncoding.DecodeString(encoded) if err != nil || len(ciphertext) == 0 { return "", fmt.Errorf("invalid secret envelope: %w", business.ErrInvalidSecretEnvelope) } @@ -131,7 +180,7 @@ func (v *VaultSealer) Open(ctx context.Context, purpose, payload string) (string if err != nil { return "", err } - result, err := v.request(ctx, "/v1/transit/decrypt/"+transitKeyName, string(body)) + result, err := v.request(ctx, "/v1/transit/decrypt/"+v.transitKey(), string(body)) if err != nil { return "", err } @@ -243,3 +292,40 @@ func openPurposeBoundPayload(purpose string, plaintext []byte) (string, error) { } return payload.Value, nil } + +// SealerFor binds another Transit key of the same Vault, for an organization +// whose credentials are sealed under a key of its own. +// +// No boot probe here, unlike the Cloud KMS factory: a Transit key is created on +// first encrypt (when the policy grants it), so probing would either create the +// key as a side effect of reading a binding or fail on a key that is legitimately +// not yet used. The first real seal is the check. +func (v *VaultSealer) SealerFor(_ context.Context, keyRef string) (Sealer, error) { + if err := validTransitKeyName(keyRef); err != nil { + return nil, err + } + bound := *v + bound.envelopeKey = keyRef + return &bound, nil +} + +// validTransitKeyName refuses a name that would change the request path rather +// than the key: the name is interpolated into /v1/transit/encrypt/, so a +// slash or a traversal segment would address a different endpoint entirely. +func validTransitKeyName(name string) error { + if name == "" { + return errors.New("key service: a Vault transit key name is required") + } + if name == defaultTransitKeyName { + return fmt.Errorf("key service: %q is this deployment's own transit key and cannot be an organization's", name) + } + for _, r := range name { + switch { + case r >= 'a' && r <= 'z', r >= 'A' && r <= 'Z', r >= '0' && r <= '9': + case r == '-', r == '_', r == '.': + default: + return fmt.Errorf("key service: invalid Vault transit key name %q: letters, digits, dash, underscore and dot only", name) + } + } + return nil +}