diff --git a/charms/jimm-k8s/config.yaml b/charms/jimm-k8s/config.yaml index f38330a71..2ab14cb54 100644 --- a/charms/jimm-k8s/config.yaml +++ b/charms/jimm-k8s/config.yaml @@ -62,6 +62,10 @@ options: type: string default: https://jaas.ai/models description: URL of the Juju Dashboard for this controller. + postgres-secret-storage: + default: false + type: boolean + description: Enables the use of Postgres for secret storage if Vault is not related. public-key: type: string description: The public part of JIMM's macaroon bakery keypair. diff --git a/charms/jimm-k8s/src/charm.py b/charms/jimm-k8s/src/charm.py index 82a69ae51..52c6e7bf2 100755 --- a/charms/jimm-k8s/src/charm.py +++ b/charms/jimm-k8s/src/charm.py @@ -275,6 +275,9 @@ def _update_workload(self, event): config_values["JIMM_WATCH_CONTROLLERS"] = "1" config_values["JIMM_ENABLE_JWKS_ROTATOR"] = "1" + if self.config.get("postgres-secret-storage", False): + config_values["INSECURE_SECRET_STORAGE"] = "enabled" # Value doesn't matter, checks env var exists. + # remove empty configuration values config_values = {key: value for key, value in config_values.items() if value} diff --git a/charms/jimm-k8s/tests/unit/test_charm.py b/charms/jimm-k8s/tests/unit/test_charm.py index 36d9213ed..d0af7eada 100644 --- a/charms/jimm-k8s/tests/unit/test_charm.py +++ b/charms/jimm-k8s/tests/unit/test_charm.py @@ -21,7 +21,20 @@ "public-key": "izcYsQy3TePp6bLjqOo3IRPFvkQd2IKtyODGqC6SdFk=", "private-key": "ly/dzsI9Nt/4JxUILQeAX79qZ4mygDiuYGqc2ZEiDEc=", "vault-access-address": "10.0.1.123", - "audit-log-retention-period-in-days": "10", +} + +EXPECTED_ENV = { + "CANDID_URL": "test-candid-url", + "JIMM_DASHBOARD_LOCATION": "https://jaas.ai/models", + "JIMM_DNS_NAME": "juju-jimm-k8s-0.juju-jimm-k8s-endpoints.None.svc.cluster.local", + "JIMM_ENABLE_JWKS_ROTATOR": "1", + "JIMM_LISTEN_ADDR": ":8080", + "JIMM_LOG_LEVEL": "info", + "JIMM_UUID": "1234567890", + "JIMM_WATCH_CONTROLLERS": "1", + "PRIVATE_KEY": "ly/dzsI9Nt/4JxUILQeAX79qZ4mygDiuYGqc2ZEiDEc=", + "PUBLIC_KEY": "izcYsQy3TePp6bLjqOo3IRPFvkQd2IKtyODGqC6SdFk=", + "JIMM_AUDIT_LOG_RETENTION_PERIOD_IN_DAYS": "0", } @@ -71,19 +84,7 @@ def test_on_pebble_ready(self): "startup": "disabled", "override": "replace", "command": "/root/jimmsrv", - "environment": { - "CANDID_URL": "test-candid-url", - "JIMM_DASHBOARD_LOCATION": "https://jaas.ai/models", - "JIMM_DNS_NAME": "juju-jimm-k8s-0.juju-jimm-k8s-endpoints.None.svc.cluster.local", - "JIMM_ENABLE_JWKS_ROTATOR": "1", - "JIMM_LISTEN_ADDR": ":8080", - "JIMM_LOG_LEVEL": "info", - "JIMM_UUID": "1234567890", - "JIMM_WATCH_CONTROLLERS": "1", - "JIMM_AUDIT_LOG_RETENTION_PERIOD_IN_DAYS": "10", - "PRIVATE_KEY": "ly/dzsI9Nt/4JxUILQeAX79qZ4mygDiuYGqc2ZEiDEc=", - "PUBLIC_KEY": "izcYsQy3TePp6bLjqOo3IRPFvkQd2IKtyODGqC6SdFk=", - }, + "environment": EXPECTED_ENV, } } }, @@ -110,19 +111,37 @@ def test_on_config_changed(self): "startup": "disabled", "override": "replace", "command": "/root/jimmsrv", - "environment": { - "CANDID_URL": "test-candid-url", - "JIMM_DASHBOARD_LOCATION": "https://jaas.ai/models", - "JIMM_DNS_NAME": "juju-jimm-k8s-0.juju-jimm-k8s-endpoints.None.svc.cluster.local", - "JIMM_ENABLE_JWKS_ROTATOR": "1", - "JIMM_LISTEN_ADDR": ":8080", - "JIMM_LOG_LEVEL": "info", - "JIMM_UUID": "1234567890", - "JIMM_WATCH_CONTROLLERS": "1", - "JIMM_AUDIT_LOG_RETENTION_PERIOD_IN_DAYS": "10", - "PRIVATE_KEY": "ly/dzsI9Nt/4JxUILQeAX79qZ4mygDiuYGqc2ZEiDEc=", - "PUBLIC_KEY": "izcYsQy3TePp6bLjqOo3IRPFvkQd2IKtyODGqC6SdFk=", - }, + "environment": EXPECTED_ENV, + } + } + }, + ) + + def test_postgres_secret_storage_config(self): + container = self.harness.model.unit.get_container("jimm") + self.harness.charm.on.jimm_pebble_ready.emit(container) + + self.harness.update_config(MINIMAL_CONFIG) + self.harness.update_config({"postgres-secret-storage": True}) + self.harness.set_leader(True) + + # Emit the pebble-ready event for jimm + self.harness.charm.on.jimm_pebble_ready.emit(container) + + # Check the that the plan was updated + plan = self.harness.get_container_pebble_plan("jimm") + expected_env = EXPECTED_ENV.copy() + expected_env.update({"INSECURE_SECRET_STORAGE": "enabled"}) + self.assertEqual( + plan.to_dict(), + { + "services": { + "jimm": { + "summary": "JAAS Intelligent Model Manager", + "startup": "disabled", + "override": "replace", + "command": "/root/jimmsrv", + "environment": expected_env, } } }, @@ -146,7 +165,8 @@ def test_bakery_configuration(self): # Emit the pebble-ready event for jimm self.harness.charm.on.jimm_pebble_ready.emit(container) - + expected_env = EXPECTED_ENV.copy() + expected_env.update({"BAKERY_AGENT_FILE": "/root/config/agent.json"}) # Check the that the plan was updated plan = self.harness.get_container_pebble_plan("jimm") self.assertEqual( @@ -158,20 +178,7 @@ def test_bakery_configuration(self): "startup": "disabled", "override": "replace", "command": "/root/jimmsrv", - "environment": { - "BAKERY_AGENT_FILE": "/root/config/agent.json", - "CANDID_URL": "test-candid-url", - "JIMM_DASHBOARD_LOCATION": "https://jaas.ai/models", - "JIMM_AUDIT_LOG_RETENTION_PERIOD_IN_DAYS": "0", - "JIMM_DNS_NAME": "juju-jimm-k8s-0.juju-jimm-k8s-endpoints.None.svc.cluster.local", - "JIMM_ENABLE_JWKS_ROTATOR": "1", - "JIMM_LISTEN_ADDR": ":8080", - "JIMM_LOG_LEVEL": "info", - "JIMM_UUID": "1234567890", - "JIMM_WATCH_CONTROLLERS": "1", - "PRIVATE_KEY": "ly/dzsI9Nt/4JxUILQeAX79qZ4mygDiuYGqc2ZEiDEc=", - "PUBLIC_KEY": "izcYsQy3TePp6bLjqOo3IRPFvkQd2IKtyODGqC6SdFk=", - }, + "environment": expected_env, } } }, @@ -189,6 +196,34 @@ def test_bakery_configuration(self): }, ) + def test_audit_log_retention_config(self): + container = self.harness.model.unit.get_container("jimm") + self.harness.charm.on.jimm_pebble_ready.emit(container) + + self.harness.update_config(MINIMAL_CONFIG) + self.harness.update_config({"audit-log-retention-period-in-days": "10"}) + + # Emit the pebble-ready event for jimm + self.harness.charm.on.jimm_pebble_ready.emit(container) + expected_env = EXPECTED_ENV.copy() + expected_env.update({"JIMM_AUDIT_LOG_RETENTION_PERIOD_IN_DAYS": "10"}) + # Check the that the plan was updated + plan = self.harness.get_container_pebble_plan("jimm") + self.assertEqual( + plan.to_dict(), + { + "services": { + "jimm": { + "summary": "JAAS Intelligent Model Manager", + "startup": "disabled", + "override": "replace", + "command": "/root/jimmsrv", + "environment": expected_env, + } + } + }, + ) + def test_dashboard_relation_joined(self): harness = Harness(JimmOperatorCharm) self.addCleanup(harness.cleanup) diff --git a/charms/jimm/config.yaml b/charms/jimm/config.yaml index 85f552a59..f8a105f77 100644 --- a/charms/jimm/config.yaml +++ b/charms/jimm/config.yaml @@ -62,6 +62,10 @@ options: type: string default: https://jaas.ai/models description: URL of the Juju Dashboard for this controller. + postgres-secret-storage: + default: false + type: boolean + description: Enables the use of Postgres for secret storage if Vault is not related. public-key: type: string description: The public part of JIMM's macaroon bakery keypair. diff --git a/charms/jimm/src/charm.py b/charms/jimm/src/charm.py index 7b2bd0d77..235ae85de 100755 --- a/charms/jimm/src/charm.py +++ b/charms/jimm/src/charm.py @@ -135,6 +135,9 @@ def _on_config_changed(self, _): "audit_retention_period": self.config.get("audit-log-retention-period-in-days", ""), } + if self.config.get("postgres-secret-storage", False): + args["insecure_secret_storage"] = "enabled" # Value doesn't matter, only checks env var exists. + with open(self._env_filename(), "wt") as f: f.write(self._render_template("jimm.env", **args)) if self._ready(): diff --git a/charms/jimm/templates/jimm.env b/charms/jimm/templates/jimm.env index 05d2a5ccb..83bf03953 100644 --- a/charms/jimm/templates/jimm.env +++ b/charms/jimm/templates/jimm.env @@ -16,3 +16,6 @@ PRIVATE_KEY={{private_key}} PUBLIC_KEY={{public_key}} {% endif %} JIMM_AUDIT_LOG_RETENTION_PERIOD_IN_DAYS={{audit_retention_period}} +{%- if insecure_secret_storage %} +INSECURE_SECRET_STORAGE=enabled +{% endif %} diff --git a/charms/jimm/tests/test_charm.py b/charms/jimm/tests/test_charm.py index a09991029..3ffe9579f 100644 --- a/charms/jimm/tests/test_charm.py +++ b/charms/jimm/tests/test_charm.py @@ -552,6 +552,34 @@ def test_openfga_relation_changed(self): self.assertEqual(lines[3].strip(), "OPENFGA_STORE=test-store") self.assertEqual(lines[4].strip(), "OPENFGA_TOKEN=test-secret-token") + def test_insecure_secret_storage(self): + """Test that the flag for insecure secret storage is only generated when explictly requested.""" + config_file = os.path.join(self.harness.charm.charm_dir, "juju-jimm.env") + self.harness.update_config( + { + "candid-url": "https://candid.example.com", + "controller-admins": "user1 user2 group1", + "dns-name": "jimm.example.com", + "log-level": "debug", + "uuid": "caaa4ba4-e2b5-40dd-9bf3-2bd26d6e17aa", + "public-key": "izcYsQy3TePp6bLjqOo3IRPFvkQd2IKtyODGqC6SdFk=", + "private-key": "ly/dzsI9Nt/4JxUILQeAX79qZ4mygDiuYGqc2ZEiDEc=", + } + ) + self.assertTrue(os.path.exists(config_file)) + with open(config_file) as f: + lines = f.readlines() + os.unlink(config_file) + self.assertEqual(len(lines), 18) + self.assertEqual(len([match for match in lines if "INSECURE_SECRET_STORAGE" in match]), 0) + self.harness.update_config({"postgres-secret-storage": True}) + self.assertTrue(os.path.exists(config_file)) + with open(config_file) as f: + lines = f.readlines() + os.unlink(config_file) + self.assertEqual(len(lines), 19) + self.assertEqual(len([match for match in lines if "INSECURE_SECRET_STORAGE" in match]), 1) + class VersionHTTPRequestHandler(BaseHTTPRequestHandler): def __init__(self, *args, **kwargs): diff --git a/cmd/jimmsrv/main.go b/cmd/jimmsrv/main.go index db811d837..70721965c 100644 --- a/cmd/jimmsrv/main.go +++ b/cmd/jimmsrv/main.go @@ -85,7 +85,9 @@ func start(ctx context.Context, s *service.Service) error { zapctx.Info(ctx, "attempting to start JWKS rotator") s.Go(func() error { err := jimmsvc.StartJWKSRotator(ctx, time.NewTicker(time.Hour).C, time.Now().UTC().AddDate(0, 3, 0)) - zapctx.Error(ctx, "failed to start JWKS rotator", zap.Error(err)) + if err != nil { + zapctx.Error(ctx, "failed to start JWKS rotator", zap.Error(err)) + } return err }) } diff --git a/docker-compose.yaml b/docker-compose.yaml index b49ab3c51..b01cf5e7e 100644 --- a/docker-compose.yaml +++ b/docker-compose.yaml @@ -43,10 +43,13 @@ services: # Not needed for local test (yet). # BAKERY_AGENT_FILE: "" JIMM_ADMINS: "jimm@candid.localhost" + # Note: You can comment out the Vault ENV vars below and instead use INSECURE_SECRET_STORAGE to place secrets in Postgres. VAULT_ADDR: "http://vault:8200" VAULT_PATH: "/jimm-kv/" VAULT_SECRET_FILE: "/vault/approle.json" VAULT_AUTH_PATH: "/auth/approle/login" + # Note: By default we should use Vault as that is the primary means of secret storage. + # INSECURE_SECRET_STORAGE: "enabled" # JIMM_DASHBOARD_LOCATION: "" JIMM_DNS_NAME: "jimm.localhost" JIMM_WATCH_CONTROLLERS: "" diff --git a/internal/db/db.go b/internal/db/db.go index 76eaadf4b..e6717dd0c 100644 --- a/internal/db/db.go +++ b/internal/db/db.go @@ -70,7 +70,7 @@ func (d *Database) Migrate(ctx context.Context, force bool) error { } for { - v := dbmodel.Version{Component: dbmodel.Component} + v := dbmodel.Version{Component: dbmodel.Component, Major: 1, Minor: 0} if err := db.FirstOrCreate(&v).Error; err != nil { return errors.E(op, dbError(err)) } @@ -80,10 +80,11 @@ func (d *Database) Migrate(ctx context.Context, force bool) error { atomic.StoreUint32(&d.migrated, 1) return nil } - if v.Major != dbmodel.Major && !force && v.Major != 0 { + if v.Major != dbmodel.Major && !force { return errors.E(op, errors.CodeServerConfiguration, fmt.Sprintf("database has incompatible version %d.%d", v.Major, v.Minor)) } // The major versions are unchanged, the database can be migrated. + v.Minor += 1 schema, err := dbmodel.SQL.ReadFile(path.Join("sql", db.Name(), fmt.Sprintf("%d_%d.sql", v.Major, v.Minor))) if err != nil { return errors.E(op, err) diff --git a/internal/db/export_test.go b/internal/db/export_test.go new file mode 100644 index 000000000..2d5d6efba --- /dev/null +++ b/internal/db/export_test.go @@ -0,0 +1,10 @@ +// Copyright 2023 Canonical Ltd. + +package db + +var ( + JwksKind = jwksKind + JwksPublicKeyTag = jwksPublicKeyTag + JwksPrivateKeyTag = jwksPrivateKeyTag + JwksExpiryTag = jwksExpiryTag +) diff --git a/internal/db/secrets.go b/internal/db/secrets.go new file mode 100644 index 000000000..7a62d84f9 --- /dev/null +++ b/internal/db/secrets.go @@ -0,0 +1,281 @@ +// Copyright 2023 Canonical Ltd. + +package db + +import ( + "context" + "encoding/json" + "time" + + "github.com/canonical/jimm/internal/dbmodel" + "github.com/canonical/jimm/internal/errors" + "github.com/juju/names/v4" + "github.com/juju/zaputil/zapctx" + "github.com/lestrrat-go/jwx/v2/jwk" + "go.uber.org/zap" + "gorm.io/gorm/clause" +) + +const ( + // These keys provide consistency across get/put methods. + usernameKey = "username" + passwordKey = "password" + + // These constants are used to create the appropriate identifiers for JWKS related data. + jwksKind = "jwks" + jwksPublicKeyTag = "jwksPublicKey" + jwksPrivateKeyTag = "jwksPrivateKey" + jwksExpiryTag = "jwksExpiry" +) + +// UpsertSecret stores secret information. +// - updates the secret's time and data if it already exists +func (d *Database) UpsertSecret(ctx context.Context, secret *dbmodel.Secret) error { + const op = errors.Op("db.AddSecret") + if err := d.ready(); err != nil { + return errors.E(op, err) + } + // On conflict perform an upset to make the operation resemble a Put. + db := d.DB.WithContext(ctx).Clauses(clause.OnConflict{ + Columns: []clause.Column{{Name: "type"}, {Name: "tag"}}, + DoUpdates: clause.AssignmentColumns([]string{"time", "data"}), + }) + if err := db.Create(secret).Error; err != nil { + return errors.E(op, dbError(err)) + } + return nil +} + +// GetSecret gets the secret with the specified type and tag. +func (d *Database) GetSecret(ctx context.Context, secret *dbmodel.Secret) error { + const op = errors.Op("db.GetSecret") + if err := d.ready(); err != nil { + return errors.E(op, err) + } + if secret.Tag == "" || secret.Type == "" { + return errors.E(op, "missing secret tag and type", errors.CodeBadRequest) + } + db := d.DB.WithContext(ctx) + + db = db.Where("tag = ? AND type = ?", secret.Tag, secret.Type) + + if err := db.First(&secret).Error; err != nil { + err = dbError(err) + if errors.ErrorCode(err) == errors.CodeNotFound { + return errors.E(op, err, "secret not found") + } + return errors.E(op, dbError(err)) + } + return nil +} + +// Delete secret deletes the secret with the specified type and tag. +func (d *Database) DeleteSecret(ctx context.Context, secret *dbmodel.Secret) error { + const op = errors.Op("db.DeleteSecret") + if err := d.ready(); err != nil { + return errors.E(op, err) + } + if secret.Tag == "" || secret.Type == "" { + return errors.E(op, "missing secret tag and type", errors.CodeBadRequest) + } + db := d.DB.WithContext(ctx) + + if err := db.Unscoped().Where("tag = ? AND type = ?", secret.Tag, secret.Type).Delete(&dbmodel.Secret{}).Error; err != nil { + return errors.E(op, dbError(err)) + } + return nil +} + +// Get retrieves the attributes for the given cloud credential from the DB. +func (d *Database) Get(ctx context.Context, tag names.CloudCredentialTag) (map[string]string, error) { + const op = errors.Op("database.Get") + secret := dbmodel.NewSecret(tag.Kind(), tag.String(), nil) + err := d.GetSecret(ctx, &secret) + if err != nil { + zapctx.Error(ctx, "failed to get secret data", zap.Error(err)) + return nil, errors.E(op, err) + } + var attr map[string]string + err = json.Unmarshal(secret.Data, &attr) + if err != nil { + zapctx.Error(ctx, "failed to unmarshal secret data", zap.Error(err)) + return nil, errors.E(op, err) + } + return attr, nil +} + +// Put stores the attributes associated with a cloud-credential in the DB. +func (d *Database) Put(ctx context.Context, tag names.CloudCredentialTag, attr map[string]string) error { + const op = errors.Op("database.Put") + if len(attr) == 0 { + return d.deleteCloudCredential(ctx, tag) + } + dataJson, err := json.Marshal(attr) + if err != nil { + zapctx.Error(ctx, "failed to marshal secret data", zap.Error(err)) + return errors.E(op, err, "failed to marshal secret data") + } + secret := dbmodel.NewSecret(tag.Kind(), tag.String(), dataJson) + return d.UpsertSecret(ctx, &secret) +} + +// deleteCloudCredential removes the attributes associated with the cloud-credential in the DB. +func (d *Database) deleteCloudCredential(ctx context.Context, tag names.CloudCredentialTag) error { + secret := dbmodel.NewSecret(tag.Kind(), tag.String(), nil) + return d.DeleteSecret(ctx, &secret) +} + +// GetControllerCredentials retrieves the credentials for the given controller from the DB. +func (d *Database) GetControllerCredentials(ctx context.Context, controllerName string) (string, string, error) { + const op = errors.Op("database.GetControllerCredentials") + secret := dbmodel.NewSecret(names.ControllerTagKind, controllerName, nil) + err := d.GetSecret(ctx, &secret) + if err != nil { + zapctx.Error(ctx, "failed to get secret data", zap.Error(err)) + return "", "", errors.E(op, err) + } + var secretData map[string]string + err = json.Unmarshal(secret.Data, &secretData) + if err != nil { + zapctx.Error(ctx, "failed to unmarshal secret data", zap.Error(err)) + return "", "", errors.E(op, err) + } + username, ok := secretData[usernameKey] + if !ok { + return "", "", errors.E(op, "missing username") + } + password, ok := secretData[passwordKey] + if !ok { + return "", "", errors.E(op, "missing password") + } + return username, password, nil +} + +// PutControllerCredentials stores the controller credentials in the DB. +func (d *Database) PutControllerCredentials(ctx context.Context, controllerName string, username string, password string) error { + const op = errors.Op("database.PutControllerCredentials") + secretData := make(map[string]string) + secretData[usernameKey] = username + secretData[passwordKey] = password + dataJson, err := json.Marshal(secretData) + if err != nil { + zapctx.Error(ctx, "failed to unmarshal secret data", zap.Error(err)) + return errors.E(op, err, "failed to marshal secret data") + } + secret := dbmodel.NewSecret(names.ControllerTagKind, controllerName, dataJson) + return d.UpsertSecret(ctx, &secret) +} + +// CleanupJWKS removes all secrets associated with the JWKS process. +func (d *Database) CleanupJWKS(ctx context.Context) error { + const op = errors.Op("database.CleanupJWKS") + secret := dbmodel.NewSecret(jwksKind, jwksPublicKeyTag, nil) + err := d.DeleteSecret(ctx, &secret) + if err != nil { + zapctx.Error(ctx, "failed to cleanup jwks public key", zap.Error(err)) + return errors.E(op, err, "failed to cleanup jwks public key") + } + secret = dbmodel.NewSecret(jwksKind, jwksPrivateKeyTag, nil) + err = d.DeleteSecret(ctx, &secret) + if err != nil { + zapctx.Error(ctx, "failed to cleanup jwks private key", zap.Error(err)) + return errors.E(op, err, "failed to cleanup jwks private key") + } + secret = dbmodel.NewSecret(jwksKind, jwksExpiryTag, nil) + err = d.DeleteSecret(ctx, &secret) + if err != nil { + zapctx.Error(ctx, "failed to cleanup jwks expiry info", zap.Error(err)) + return errors.E(op, err, "failed to cleanup jwks expiry info") + } + return nil +} + +// GetJWKS returns the current key set stored within the DB. +func (d *Database) GetJWKS(ctx context.Context) (jwk.Set, error) { + const op = errors.Op("database.GetJWKS") + secret := dbmodel.NewSecret(jwksKind, jwksPublicKeyTag, nil) + err := d.GetSecret(ctx, &secret) + if err != nil { + zapctx.Error(ctx, "failed to get jwks data", zap.Error(err)) + return nil, errors.E(op, err) + } + ks, err := jwk.ParseString(string(secret.Data)) + if err != nil { + zapctx.Error(ctx, "failed to parse jwk data", zap.Error(err)) + return nil, errors.E(op, err) + } + return ks, nil +} + +// GetJWKSPrivateKey returns the current private key for the active JWKS +func (d *Database) GetJWKSPrivateKey(ctx context.Context) ([]byte, error) { + const op = errors.Op("database.GetJWKSPrivateKey") + secret := dbmodel.NewSecret(jwksKind, jwksPrivateKeyTag, nil) + err := d.GetSecret(ctx, &secret) + if err != nil { + zapctx.Error(ctx, "failed to get jwks jwks private key", zap.Error(err)) + return nil, errors.E(op, err) + } + var pem []byte + err = json.Unmarshal(secret.Data, &pem) + if err != nil { + zapctx.Error(ctx, "failed to unmarshal pem data data", zap.Error(err)) + return nil, errors.E(op, err) + } + return pem, nil +} + +// GetJWKSExpiry returns the expiry of the active JWKS. +func (d *Database) GetJWKSExpiry(ctx context.Context) (time.Time, error) { + const op = errors.Op("database.GetJWKSExpiry") + secret := dbmodel.NewSecret(jwksKind, jwksExpiryTag, nil) + err := d.GetSecret(ctx, &secret) + if err != nil { + zapctx.Error(ctx, "failed to get jwks expiry", zap.Error(err)) + return time.Time{}, errors.E(op, err) + } + var expiryTime time.Time + err = json.Unmarshal(secret.Data, &expiryTime) + if err != nil { + zapctx.Error(ctx, "failed to unmarshal jwks expiry data", zap.Error(err)) + return time.Time{}, errors.E(op, err) + } + return expiryTime, nil +} + +// PutJWKS puts a JWKS into the DB. +func (d *Database) PutJWKS(ctx context.Context, jwks jwk.Set) error { + const op = errors.Op("database.PutJWKS") + jwksJson, err := json.Marshal(jwks) + if err != nil { + zapctx.Error(ctx, "failed to marshal jwks", zap.Error(err)) + return errors.E(op, err, "failed to marshal jwks data") + } + secret := dbmodel.NewSecret(jwksKind, jwksPublicKeyTag, jwksJson) + return d.UpsertSecret(ctx, &secret) + +} + +// PutJWKSPrivateKey persists the private key associated with the current JWKS within the DB. +func (d *Database) PutJWKSPrivateKey(ctx context.Context, pem []byte) error { + const op = errors.Op("database.PutJWKSPrivateKey") + privateKeyJson, err := json.Marshal(pem) + if err != nil { + zapctx.Error(ctx, "failed to marshal pem data", zap.Error(err)) + return errors.E(op, err, "failed to marshal jwks private key") + } + secret := dbmodel.NewSecret(jwksKind, jwksPrivateKeyTag, privateKeyJson) + return d.UpsertSecret(ctx, &secret) +} + +// PutJWKSExpiry sets the expiry time for the current JWKS within the DB. +func (d *Database) PutJWKSExpiry(ctx context.Context, expiry time.Time) error { + const op = errors.Op("database.PutJWKSExpiry") + expiryJson, err := json.Marshal(expiry) + if err != nil { + zapctx.Error(ctx, "failed to marshal jwks expiry data", zap.Error(err)) + return errors.E(op, err, "failed to marshal jwks data") + } + secret := dbmodel.NewSecret(jwksKind, jwksExpiryTag, expiryJson) + return d.UpsertSecret(ctx, &secret) +} diff --git a/internal/db/secrets_test.go b/internal/db/secrets_test.go new file mode 100644 index 000000000..a2cdc34e7 --- /dev/null +++ b/internal/db/secrets_test.go @@ -0,0 +1,270 @@ +// Copyright 2023 Canonical Ltd. + +package db_test + +import ( + "context" + "time" + + qt "github.com/frankban/quicktest" + "github.com/google/uuid" + "github.com/juju/names/v4" + "github.com/lestrrat-go/jwx/v2/jwa" + "github.com/lestrrat-go/jwx/v2/jwk" + + "github.com/canonical/jimm/internal/db" + "github.com/canonical/jimm/internal/dbmodel" +) + +var testTime = time.Date(2013, 7, 26, 0, 0, 0, 0, time.UTC) + +func (s *dbSuite) TestInsertSecret(c *qt.C) { + err := s.Database.Migrate(context.Background(), true) + c.Assert(err, qt.Equals, nil) + + u := dbmodel.Secret{ + Time: testTime, + Type: "generic", + Tag: "123", + Data: nil, + } + c.Assert(s.Database.DB.Create(&u).Error, qt.IsNil) + secret := dbmodel.Secret{} + tx := s.Database.DB.First(&secret) + c.Assert(tx.Error, qt.IsNil) + c.Assert(secret.Time, qt.Equals, testTime) + c.Assert(secret.Type, qt.Equals, "generic") + c.Assert(secret.Tag, qt.Equals, "123") + c.Assert(secret.Data, qt.IsNil) +} + +func (s *dbSuite) TestUpsertSecret(c *qt.C) { + err := s.Database.Migrate(context.Background(), true) + c.Assert(err, qt.Equals, nil) + ctx := context.Background() + + u := dbmodel.Secret{ + Time: testTime, + Type: "generic", + Tag: "123", + Data: nil, + } + c.Assert(s.Database.UpsertSecret(ctx, &u), qt.IsNil) + newTime := testTime.Add(time.Hour) + y := dbmodel.Secret{ + Time: newTime, + Type: "generic", + Tag: "123", + Data: []byte("123"), + } + c.Assert(s.Database.UpsertSecret(ctx, &y), qt.IsNil) + secret := dbmodel.Secret{} + tx := s.Database.DB.First(&secret) + c.Assert(tx.Error, qt.IsNil) + c.Assert(secret.Time, qt.Equals, newTime) + c.Assert([]byte(secret.Data), qt.DeepEquals, []byte("123")) +} + +func (s *dbSuite) TestGetSecret(c *qt.C) { + err := s.Database.Migrate(context.Background(), true) + c.Assert(err, qt.Equals, nil) + ctx := context.Background() + + u := dbmodel.Secret{ + Time: testTime, + Type: "generic", + Tag: "123", + Data: nil, + } + c.Assert(s.Database.DB.Create(&u).Error, qt.IsNil) + secret := dbmodel.Secret{Type: "generic", Tag: "123"} + c.Assert(s.Database.GetSecret(ctx, &secret), qt.IsNil) + c.Assert(secret.Time, qt.Equals, testTime) + c.Assert(secret.Type, qt.Equals, "generic") + c.Assert(secret.Tag, qt.Equals, "123") +} + +func (s *dbSuite) TestGetSecretFailsWithoutTypeAndTag(c *qt.C) { + err := s.Database.Migrate(context.Background(), true) + c.Assert(err, qt.Equals, nil) + ctx := context.Background() + secret := dbmodel.Secret{} + c.Assert(s.Database.GetSecret(ctx, &secret), qt.ErrorMatches, "missing secret tag and type") +} + +func (s *dbSuite) TestDeleteSecretFailsWithoutTypeAndTag(c *qt.C) { + err := s.Database.Migrate(context.Background(), true) + c.Assert(err, qt.Equals, nil) + ctx := context.Background() + + secret := dbmodel.Secret{} + c.Assert(s.Database.DeleteSecret(ctx, &secret), qt.ErrorMatches, "missing secret tag and type") +} + +func (s *dbSuite) TestDeleteSecret(c *qt.C) { + err := s.Database.Migrate(context.Background(), true) + c.Assert(err, qt.Equals, nil) + ctx := context.Background() + + u := dbmodel.Secret{ + Time: testTime, + Type: "generic", + Tag: "123", + Data: nil, + } + c.Assert(s.Database.DB.Create(&u).Error, qt.IsNil) + secret := dbmodel.Secret{Type: "generic", Tag: "123"} + c.Assert(s.Database.DeleteSecret(ctx, &secret), qt.IsNil) + var count int64 + c.Assert(s.Database.DB.Model(&dbmodel.Secret{}).Count(&count).Error, qt.IsNil) + c.Assert(count, qt.Equals, int64(0)) +} + +func (s *dbSuite) TestPutAndGetCloudCredential(c *qt.C) { + err := s.Database.Migrate(context.Background(), true) + c.Assert(err, qt.Equals, nil) + ctx := context.Background() + cloudCred := names.NewCloudCredentialTag("foo/bar/bob") + setAttr := map[string]string{"key": "value"} + c.Assert(s.Database.Put(ctx, cloudCred, setAttr), qt.IsNil) + // Verify the type and tag are correct + secret := dbmodel.Secret{} + tx := s.Database.DB.First(&secret) + c.Assert(tx.Error, qt.IsNil) + c.Assert(secret.Type, qt.Equals, names.CloudCredentialTagKind) + c.Assert(secret.Tag, qt.Equals, cloudCred.String()) + // Get CloudCred + attr, err := s.Database.Get(ctx, cloudCred) + c.Assert(err, qt.IsNil) + c.Assert(attr, qt.DeepEquals, setAttr) +} + +func (s *dbSuite) TestPutAndGetControllerCredential(c *qt.C) { + err := s.Database.Migrate(context.Background(), true) + c.Assert(err, qt.Equals, nil) + ctx := context.Background() + controllerName := "beef1beef2-0000-0000-000011112222" + c.Assert(s.Database.PutControllerCredentials(ctx, controllerName, "user", "pass"), qt.IsNil) + // Verify the type and tag are correct + secret := dbmodel.Secret{} + tx := s.Database.DB.First(&secret) + c.Assert(tx.Error, qt.IsNil) + c.Assert(secret.Type, qt.Equals, names.ControllerTagKind) + c.Assert(secret.Tag, qt.Equals, controllerName) + // Get ControllerCred + username, password, err := s.Database.GetControllerCredentials(ctx, controllerName) + c.Assert(err, qt.IsNil) + c.Assert(username, qt.Equals, "user") + c.Assert(password, qt.Equals, "pass") +} + +func getJWKS(c *qt.C) jwk.Set { + set, err := jwk.ParseString(` + { + "keys": [ + { + "alg": "RS256", + "kty": "RSA", + "use": "sig", + "n": "yeNlzlub94YgerT030codqEztjfU_S6X4DbDA_iVKkjAWtYfPHDzz_sPCT1Axz6isZdf3lHpq_gYX4Sz-cbe4rjmigxUxr-FgKHQy3HeCdK6hNq9ASQvMK9LBOpXDNn7mei6RZWom4wo3CMvvsY1w8tjtfLb-yQwJPltHxShZq5-ihC9irpLI9xEBTgG12q5lGIFPhTl_7inA1PFK97LuSLnTJzW0bj096v_TMDg7pOWm_zHtF53qbVsI0e3v5nmdKXdFf9BjIARRfVrbxVxiZHjU6zL6jY5QJdh1QCmENoejj_ytspMmGW7yMRxzUqgxcAqOBpVm0b-_mW3HoBdjQ", + "e": "AQAB", + "kid": "32d2b213-d3fe-436c-9d4c-67a673890620" + }, + { + "alg": "RS256", + "kty": "RSA", + "use": "sig", + "n": "jbglzlub94YgerT030codqEztjfU_S6X4DbDA_iVKkjAWtYfPHDzz_sPCT1Axz6isZdf3lHpq_gYX4Sz-cbe4rjmigxUxr-FgKHQy3HeCdK6hNq9ASQvMK9LBOpXDNn7mei6RZWom4wo3CMvvsY1w8tjtfLb-yQwJPltHxShZq5-ihC9irpLI9xEBTgG12q5lGIFPhTl_7inA1PFK97LuSLnTJzW0bj096v_TMDg7pOWm_zHtF53qbVsI0e3v5nmdKXdFf9BjIARRfVrbxVxiZHjU6zL6jY5QJdh1QCmENoejj_ytspMmGW7yMRxzUqgxcAqOBpVm0b-_mW3HoBdjQ", + "e": "FEGB", + "kid": "32d2b213-d3fe-436c-9d4c-67a673890621" + } + ] + } + `) + c.Assert(err, qt.IsNil) + return set +} + +func (s *dbSuite) TestPutAndGetJWKS(c *qt.C) { + err := s.Database.Migrate(context.Background(), true) + c.Assert(err, qt.Equals, nil) + ctx := context.Background() + jwks := getJWKS(c) + c.Assert(s.Database.PutJWKS(ctx, jwks), qt.IsNil) + // Verify the type and tag are correct + secret := dbmodel.Secret{} + tx := s.Database.DB.First(&secret) + c.Assert(tx.Error, qt.IsNil) + c.Assert(secret.Type, qt.Equals, db.JwksKind) + c.Assert(secret.Tag, qt.Equals, db.JwksPublicKeyTag) + // Get JWKS + gotJwks, err := s.Database.GetJWKS(ctx) + c.Assert(err, qt.IsNil) + ki := gotJwks.Keys(ctx) + gotOneKey := false + for ki.Next(ctx) { + gotOneKey = true + key := ki.Pair().Value.(jwk.Key) + _, err = uuid.Parse(key.KeyID()) + c.Assert(err, qt.IsNil) + c.Assert(key.KeyUsage(), qt.Equals, "sig") + c.Assert(key.Algorithm(), qt.Equals, jwa.RS256) + } + c.Assert(gotOneKey, qt.IsTrue) +} + +func (s *dbSuite) TestPutAndGetJwksExpiry(c *qt.C) { + err := s.Database.Migrate(context.Background(), true) + c.Assert(err, qt.Equals, nil) + ctx := context.Background() + expiryTime := time.Date(2023, 7, 26, 0, 0, 0, 0, time.UTC) + c.Assert(s.Database.PutJWKSExpiry(ctx, expiryTime), qt.IsNil) + // Verify the type and tag are correct + secret := dbmodel.Secret{} + tx := s.Database.DB.First(&secret) + c.Assert(tx.Error, qt.IsNil) + c.Assert(secret.Type, qt.Equals, db.JwksKind) + c.Assert(secret.Tag, qt.Equals, db.JwksExpiryTag) + // Get ControllerCred + gotExpiryTime, err := s.Database.GetJWKSExpiry(ctx) + c.Assert(err, qt.IsNil) + c.Assert(gotExpiryTime, qt.Equals, expiryTime) +} + +func (s *dbSuite) TestPutAndGetJwksPrivateKey(c *qt.C) { + err := s.Database.Migrate(context.Background(), true) + c.Assert(err, qt.Equals, nil) + ctx := context.Background() + pem := []byte("123") + c.Assert(s.Database.PutJWKSPrivateKey(ctx, pem), qt.IsNil) + // Verify the type and tag are correct + secret := dbmodel.Secret{} + tx := s.Database.DB.First(&secret) + c.Assert(tx.Error, qt.IsNil) + c.Assert(secret.Type, qt.Equals, db.JwksKind) + c.Assert(secret.Tag, qt.Equals, db.JwksPrivateKeyTag) + // Get ControllerCred + gotPem, err := s.Database.GetJWKSPrivateKey(ctx) + c.Assert(err, qt.IsNil) + c.Assert(gotPem, qt.DeepEquals, pem) +} + +func (s *dbSuite) TestCleanupJWKS(c *qt.C) { + err := s.Database.Migrate(context.Background(), true) + c.Assert(err, qt.Equals, nil) + ctx := context.Background() + pem := []byte("123") + c.Assert(s.Database.PutJWKSPrivateKey(ctx, pem), qt.IsNil) + expiryTime := time.Date(2023, 7, 26, 0, 0, 0, 0, time.UTC) + c.Assert(s.Database.PutJWKSExpiry(ctx, expiryTime), qt.IsNil) + jwks := getJWKS(c) + c.Assert(s.Database.PutJWKS(ctx, jwks), qt.IsNil) + // Verify 3 secrets exist + var count int64 + c.Assert(s.Database.DB.Model(&dbmodel.Secret{}).Count(&count).Error, qt.IsNil) + c.Assert(count, qt.Equals, int64(3)) + // Verify all JWKS secrets are removed + c.Assert(s.Database.CleanupJWKS(ctx), qt.IsNil) + c.Assert(s.Database.DB.Model(&dbmodel.Secret{}).Count(&count).Error, qt.IsNil) + c.Assert(count, qt.Equals, int64(0)) +} diff --git a/internal/dbmodel/gorm_test.go b/internal/dbmodel/gorm_test.go index 3b07fb247..fc102a0d5 100644 --- a/internal/dbmodel/gorm_test.go +++ b/internal/dbmodel/gorm_test.go @@ -3,11 +3,12 @@ package dbmodel_test import ( + "context" "testing" "gorm.io/gorm" - "github.com/canonical/jimm/internal/dbmodel" + "github.com/canonical/jimm/internal/db" "github.com/canonical/jimm/internal/jimmtest" ) @@ -16,20 +17,7 @@ import ( // debug enabled. If any objects are specified the datbase automatically // performs the migrations for those objects. func gormDB(t testing.TB) *gorm.DB { - vschema, err := dbmodel.SQL.ReadFile("sql/sqlite/versions.sql") - if err != nil { - t.Fatalf("error loading database schema: %s", err) - } - schema, err := dbmodel.SQL.ReadFile("sql/sqlite/0_0.sql") - if err != nil { - t.Fatalf("error loading database schema: %s", err) - } - db := jimmtest.MemoryDB(t, nil) - if err := db.Exec(string(vschema)).Error; err != nil { - t.Fatalf("error perform migrations on test database: %s", err) - } - if err := db.Exec(string(schema)).Error; err != nil { - t.Fatalf("error perform migrations on test database: %s", err) - } - return db + database := db.Database{DB: jimmtest.MemoryDB(t, nil)} + database.Migrate(context.Background(), false) + return database.DB } diff --git a/internal/dbmodel/secrets.go b/internal/dbmodel/secrets.go new file mode 100644 index 000000000..02acf9d1e --- /dev/null +++ b/internal/dbmodel/secrets.go @@ -0,0 +1,27 @@ +package dbmodel + +import "time" + +// A Secret is a generic secret. +type Secret struct { + // ID contains the ID of the entry. + ID uint `gorm:"primarykey"` + + // Time contains the time the secret was created/updated. + Time time.Time + + // Type contains the secret type, i.e. controller, cloudCredential, jwks, etc. + Type string `gorm:"index:idx_secret_name,unique"` + + // Contains an identifier for the secret that is unique for a specific secret type. + Tag string `gorm:"index:idx_secret_name,unique"` + + // Contains the secret data. + Data JSON +} + +// newSecret creates a secret object with the time set to the current time +// and the type and tag fields set from the tag object +func NewSecret(secretType string, secretTag string, data []byte) Secret { + return Secret{Time: time.Now(), Type: secretType, Tag: secretTag, Data: data} +} diff --git a/internal/dbmodel/sql/postgres/0_0.sql b/internal/dbmodel/sql/postgres/1_1.sql similarity index 98% rename from internal/dbmodel/sql/postgres/0_0.sql rename to internal/dbmodel/sql/postgres/1_1.sql index 468d0fc86..df8bf3c73 100644 --- a/internal/dbmodel/sql/postgres/0_0.sql +++ b/internal/dbmodel/sql/postgres/1_1.sql @@ -1,4 +1,4 @@ --- 0_0.sql initialises an empty database. +-- 1_1.sql initialises an empty database. CREATE TABLE IF NOT EXISTS audit_log ( id BIGSERIAL PRIMARY KEY, @@ -285,5 +285,5 @@ CREATE TABLE IF NOT EXISTS groups ( CREATE INDEX IF NOT EXISTS idx_group_deleted_at ON groups (deleted_at); CREATE INDEX IF NOT EXISTS idx_group_name ON groups (name); -UPDATE versions SET major=1, minor=0 WHERE component='jimmdb'; +UPDATE versions SET major=1, minor=1 WHERE component='jimmdb'; diff --git a/internal/dbmodel/sql/postgres/1_2.sql b/internal/dbmodel/sql/postgres/1_2.sql new file mode 100644 index 000000000..9deefe206 --- /dev/null +++ b/internal/dbmodel/sql/postgres/1_2.sql @@ -0,0 +1,12 @@ +-- 1_2.sql is a migration that adds a secrets table. + +CREATE TABLE IF NOT EXISTS secrets ( + id BIGSERIAL PRIMARY KEY, + time TIMESTAMP WITH TIME ZONE, + type TEXT NOT NULL, + tag TEXT NOT NULL, + data JSONB +); +CREATE UNIQUE INDEX IF NOT EXISTS idx_secret_name ON secrets (type, tag); + +UPDATE versions SET major=1, minor=2 WHERE component='jimmdb'; diff --git a/internal/dbmodel/sql/sqlite/0_0.sql b/internal/dbmodel/sql/sqlite/1_1.sql similarity index 98% rename from internal/dbmodel/sql/sqlite/0_0.sql rename to internal/dbmodel/sql/sqlite/1_1.sql index 44f0f2d04..3a5fbc1d8 100644 --- a/internal/dbmodel/sql/sqlite/0_0.sql +++ b/internal/dbmodel/sql/sqlite/1_1.sql @@ -1,4 +1,4 @@ --- 0_0.sql initialises an empty database. +-- 1_1.sql initialises an empty database. CREATE TABLE audit_log ( id INTEGER PRIMARY KEY, @@ -283,4 +283,4 @@ CREATE TABLE groups ( CREATE INDEX idx_group_deleted_at ON groups (deleted_at); -UPDATE versions SET major=1, minor=0 WHERE component='jimmdb'; +UPDATE versions SET major=1, minor=1 WHERE component='jimmdb'; diff --git a/internal/dbmodel/sql/sqlite/1_2.sql b/internal/dbmodel/sql/sqlite/1_2.sql new file mode 100644 index 000000000..6e118daf1 --- /dev/null +++ b/internal/dbmodel/sql/sqlite/1_2.sql @@ -0,0 +1,12 @@ +-- 1_2.sql is a migration that adds a secrets table. + +CREATE TABLE IF NOT EXISTS secrets ( + id INTEGER PRIMARY KEY, + time DATETIME, + type TEXT NOT NULL, + tag TEXT NOT NULL, + data BLOB +); +CREATE UNIQUE INDEX IF NOT EXISTS idx_secret_name ON secrets (type, tag); + +UPDATE versions SET major=1, minor=2 WHERE component='jimmdb'; diff --git a/internal/dbmodel/version.go b/internal/dbmodel/version.go index 86ed78010..550ff3e1b 100644 --- a/internal/dbmodel/version.go +++ b/internal/dbmodel/version.go @@ -20,7 +20,7 @@ const ( // Minor is the minor version of the model described in the dbmodel // package. It should be incremented for any change made to the // database model from database model in a relesed JIMM. - Minor = 0 + Minor = 2 ) type Version struct { diff --git a/internal/dbmodel/version_test.go b/internal/dbmodel/version_test.go index 9b3d28815..533ad83b6 100644 --- a/internal/dbmodel/version_test.go +++ b/internal/dbmodel/version_test.go @@ -6,7 +6,6 @@ import ( "testing" qt "github.com/frankban/quicktest" - "gorm.io/gorm" "github.com/canonical/jimm/internal/dbmodel" ) @@ -17,21 +16,18 @@ func TestVersion(t *testing.T) { var v0 dbmodel.Version result := db.First(&v0, "component = ?", dbmodel.Component) - c.Check(result.Error, qt.Equals, gorm.ErrRecordNotFound) + c.Check(result.Error, qt.IsNil) + c.Check(v0.Major, qt.DeepEquals, dbmodel.Major) + c.Check(v0.Minor, qt.Equals, dbmodel.Minor) v1 := dbmodel.Version{ Component: dbmodel.Component, Major: dbmodel.Major, Minor: dbmodel.Minor, } - result = db.Create(&v1) + result = db.First(&v1, "component = ?", dbmodel.Component) c.Assert(result.Error, qt.IsNil) - c.Check(result.RowsAffected, qt.Equals, int64(1)) - - var v2 dbmodel.Version - result = db.First(&v2, "component = ?", dbmodel.Component) - c.Assert(result.Error, qt.IsNil) - c.Check(v2, qt.DeepEquals, v1) + c.Check(v1, qt.DeepEquals, v1) v3 := dbmodel.Version{ Component: dbmodel.Component, diff --git a/service.go b/service.go index 455490c2e..0d2b0be1d 100644 --- a/service.go +++ b/service.go @@ -299,6 +299,12 @@ func NewService(ctx context.Context, p Params) (*Service, error) { } if vs != nil { s.jimm.CredentialStore = vs + } else { + // Only enable Postgres storage for secrets if explictly enabled. + if _, ok := os.LookupEnv("INSECURE_SECRET_STORAGE"); ok { + zapctx.Warn(ctx, "using plaintext postgres for secret storage") + s.jimm.CredentialStore = &s.jimm.Database + } } s.jimm.Dialer = &jujuclient.Dialer{