Fix register a key using secret creationTimestamp instead of certificate validity timestamp (#1681)

<!--
Before you open the request please review the following guidelines and
tips to help it be more easily integrated:

 - Describe the scope of your change - i.e. what the change does.
 - Describe any known limitations with your change.
- Please run any tests or examples that can exercise your modified code.

Thank you for contributing! We will try to test and integrate the change
as soon as we can, but be aware we have many GitHub repositories to
manage and can't immediately respond to every request. There is no need
to bump or check in on a pull request (it will clutter the discussion of
the request).

Also don't be worried if the request is closed or not integrated
sometimes the priorities of Bitnami might not match the priorities of
the pull request. Don't fret, the open source community thrives on forks
and GitHub makes it easy to keep your changes in a forked repo.
 -->

**Description of the change**

Register a key and order it based on its secret creation timestamp
instead of its cert starting validity (NotBefore attribute)

<!-- Describe the scope of your change - i.e. what the change does. -->

**Benefits**

<!-- What benefits will be realized by the code change? -->

In case we are bringing our own certificate, if the certificate is
created before sealed secret installation but applied after the
installation, still the latest certificate used will be the certificate
installed by sealed secret during its initialization instead of the
latest secret certificate created.

**Possible drawbacks**

<!-- Describe any known limitations with your change -->

**Applicable issues**

<!-- Enter any applicable Issues here (You can reference an issue using
#) -->
- fixes # #1639

**Additional information**

<!-- If there's anything else that's important and relevant to your pull
request, mention that information here.-->

---------

Signed-off-by: Jérôme GARCIA <[email protected]>
Signed-off-by: Sybernatus <[email protected]>
Signed-off-by: Jérôme GARCIA <[email protected]>

Author: sybernatus

README.md

@@ -620,6 +620,18 @@ It doesn't help that this feature has been historically called "key rotation", w
 Sealed secrets are not automatically rotated and old keys are not deleted
 when new keys are generated. Old `SealedSecret` resources can be still decrypted (that's because old sealing keys are not deleted).
 
+### Key registry init priority order
+
+When the controller starts, it will initialize the key registry. The most recent key is used to seal secrets. By default, this certificate is chosen based on the NotBefore attribute of the certificate. If you want to change the priority order of the keys in the registry, you can use the `--key-order-priority` flag. 
+
+The `--key-order-priority` flag accepts the following values:
+- `CertNotBefore`: (default) The key registry will be ordered based on the NotBefore attribute of the key certificate.
+- `SecretCreationTimestamp`: The key registry will be ordered based on the creation timestamp of the secret.
+
+This flag influences the public key used to encrypt secrets and the certificate retrieved by `kubeseal --fetch-cert`. 
+
+
+
 ### User secret rotation
 
 The *sealing key* renewal and SealedSecret rotation are **not a substitute** for rotating your actual secrets.

cmd/controller/main.go

@@ -20,8 +20,9 @@ import (
 )
 
 const (
-	flagEnvPrefix         = "SEALED_SECRETS"
-	defaultKeyRenewPeriod = 30 * 24 * time.Hour
+	flagEnvPrefix           = "SEALED_SECRETS"
+	defaultKeyRenewPeriod   = 30 * 24 * time.Hour
+	defaultKeyOrderPriority = "CertNotBefore"
 )
 
 var (
@@ -36,6 +37,7 @@ func bindControllerFlags(f *controller.Flags, fs *flag.FlagSet) {
 	fs.StringVar(&f.MyCN, "my-cn", "", "Common name to be used as issuer/subject DN in generated certificate.")
 
 	fs.DurationVar(&f.KeyRenewPeriod, "key-renew-period", defaultKeyRenewPeriod, "New key generation period (automatic rotation deactivated if 0)")
+	fs.StringVar(&f.KeyOrderPriority, "key-order-priority", defaultKeyOrderPriority, "Ordering of keys based on NotBefore certificate attribute or secret creation timestamp.")
 	fs.BoolVar(&f.AcceptV1Data, "accept-deprecated-v1-data", true, "Accept deprecated V1 data field.")
 	fs.StringVar(&f.KeyCutoffTime, "key-cutoff-time", "", "Create a new key if latest one is older than this cutoff time. RFC1123 format with numeric timezone expected.")
 	fs.BoolVar(&f.NamespaceAll, "all-namespaces", true, "Scan all namespaces or only the current namespace (default=true).")

pkg/controller/controller_test.go

@@ -219,7 +219,7 @@ func testKeyRegister(t *testing.T, ctx context.Context, clientset kubernetes.Int
 	keyLabel := SealedSecretsKeyLabel
 	prefix := "test-keys"
 	testKeySize := 4096
-	keyRegistry, err := initKeyRegistry(ctx, clientset, rand.Reader, ns, prefix, keyLabel, testKeySize)
+	keyRegistry, err := initKeyRegistry(ctx, clientset, rand.Reader, ns, prefix, keyLabel, testKeySize, "CertNotBefore")
 	if err != nil {
 		t.Fatalf("failed to provision key registry: %v", err)
 	}

pkg/controller/main.go

@@ -40,6 +40,7 @@ type Flags struct {
 	ValidFor              time.Duration
 	MyCN                  string
 	KeyRenewPeriod        time.Duration
+	KeyOrderPriority      string
 	AcceptV1Data          bool
 	KeyCutoffTime         string
 	NamespaceAll          bool
@@ -62,7 +63,7 @@ func initKeyPrefix(keyPrefix string) (string, error) {
 	return validateKeyPrefix(keyPrefix)
 }
 
-func initKeyRegistry(ctx context.Context, client kubernetes.Interface, r io.Reader, namespace, prefix, label string, keysize int) (*KeyRegistry, error) {
+func initKeyRegistry(ctx context.Context, client kubernetes.Interface, r io.Reader, namespace, prefix, label string, keysize int, keyOrderPriority string) (*KeyRegistry, error) {
 	slog.Info("Searching for existing private keys")
 	secretList, err := client.CoreV1().Secrets(namespace).List(ctx, metav1.ListOptions{
 		LabelSelector: keySelector.String(),
@@ -88,14 +89,30 @@ func initKeyRegistry(ctx context.Context, client kubernetes.Interface, r io.Read
 		if err != nil {
 			slog.Error("Error reading key", "secret", secret.Name, "error", err)
 		}
-		if err := keyRegistry.registerNewKey(secret.Name, key, certs[0], certs[0].NotBefore); err != nil {
+
+		// Select ordering time based on the keyOrderPriority flag
+		orderingTime := getKeyOrderPriority(keyOrderPriority, certs[0], secret)
+
+		if err := keyRegistry.registerNewKey(secret.Name, key, certs[0], orderingTime); err != nil {
 			return nil, err
 		}
 		slog.Info("registered private key", "secretname", secret.Name)
 	}
 	return keyRegistry, nil
 }
 
+func getKeyOrderPriority(keyOrderPriority string, cert *x509.Certificate, secret v1.Secret) time.Time {
+	switch keyOrderPriority {
+	case "CertNotBefore":
+		return cert.NotBefore
+	case "SecretCreationTimestamp":
+		return secret.GetCreationTimestamp().Time
+	default:
+		slog.Error("Invalid keyOrderPriority. Use CertNotBefore or SecretCreationTimestamp", "keyOrderPriority", keyOrderPriority)
+	}
+	return cert.NotBefore
+}
+
 func myNamespace() string {
 	if ns := os.Getenv("POD_NAMESPACE"); ns != "" {
 		return ns
@@ -169,7 +186,7 @@ func Main(f *Flags, version string) error {
 		return err
 	}
 
-	keyRegistry, err := initKeyRegistry(ctx, clientset, rand.Reader, myNs, prefix, SealedSecretsKeyLabel, f.KeySize)
+	keyRegistry, err := initKeyRegistry(ctx, clientset, rand.Reader, myNs, prefix, SealedSecretsKeyLabel, f.KeySize, f.KeyOrderPriority)
 	if err != nil {
 		return err
 	}

pkg/controller/main_test.go

@@ -49,7 +49,7 @@ func TestInitKeyRegistry(t *testing.T) {
 	client := fake.NewSimpleClientset()
 	client.PrependReactor("create", "secrets", generateNameReactor)
 
-	registry, err := initKeyRegistry(ctx, client, rand, "namespace", "prefix", "label", 1024)
+	registry, err := initKeyRegistry(ctx, client, rand, "namespace", "prefix", "label", 1024, "CertNotBefore")
 	if err != nil {
 		t.Fatalf("initKeyRegistry() returned err: %v", err)
 	}
@@ -69,7 +69,7 @@ func TestInitKeyRegistry(t *testing.T) {
 
 	// Due to limitations of the fake client, we cannot test whether initKeyRegistry is able
 	// to pick up existing keys
-	_, err = initKeyRegistry(ctx, client, rand, "namespace", "prefix", "label", 1024)
+	_, err = initKeyRegistry(ctx, client, rand, "namespace", "prefix", "label", 1024, "CertNotBefore")
 	if err != nil {
 		t.Fatalf("initKeyRegistry() returned err: %v", err)
 	}
@@ -84,7 +84,7 @@ func TestInitKeyRotation(t *testing.T) {
 	client := fake.NewSimpleClientset()
 	client.PrependReactor("create", "secrets", generateNameReactor)
 
-	registry, err := initKeyRegistry(ctx, client, rand, "namespace", "prefix", "label", 1024)
+	registry, err := initKeyRegistry(ctx, client, rand, "namespace", "prefix", "label", 1024, "CertNotBefore")
 	if err != nil {
 		t.Fatalf("initKeyRegistry() returned err: %v", err)
 	}
@@ -125,7 +125,7 @@ func TestInitKeyRotationTick(t *testing.T) {
 	client := fake.NewSimpleClientset()
 	client.PrependReactor("create", "secrets", generateNameReactor)
 
-	registry, err := initKeyRegistry(ctx, client, rand, "namespace", "prefix", "label", 1024)
+	registry, err := initKeyRegistry(ctx, client, rand, "namespace", "prefix", "label", 1024, "CertNotBefore")
 	if err != nil {
 		t.Fatalf("initKeyRegistry() returned err: %v", err)
 	}
@@ -180,7 +180,7 @@ func TestReuseKey(t *testing.T) {
 
 	client.ClearActions()
 
-	registry, err := initKeyRegistry(ctx, client, rand, "namespace", "prefix", SealedSecretsKeyLabel, 1024)
+	registry, err := initKeyRegistry(ctx, client, rand, "namespace", "prefix", SealedSecretsKeyLabel, 1024, "CertNotBefore")
 	if err != nil {
 		t.Fatalf("initKeyRegistry() returned err: %v", err)
 	}
@@ -227,7 +227,7 @@ func TestRenewStaleKey(t *testing.T) {
 		t.Errorf("writeKey() failed with: %v", err)
 	}
 
-	registry, err := initKeyRegistry(ctx, client, rand, "namespace", "prefix", SealedSecretsKeyLabel, 1024)
+	registry, err := initKeyRegistry(ctx, client, rand, "namespace", "prefix", SealedSecretsKeyLabel, 1024, "CertNotBefore")
 	if err != nil {
 		t.Fatalf("initKeyRegistry() returned err: %v", err)
 	}
@@ -284,7 +284,7 @@ func TestKeyCutoff(t *testing.T) {
 		t.Errorf("writeKey() failed with: %v", err)
 	}
 
-	registry, err := initKeyRegistry(ctx, client, rand, "namespace", "prefix", SealedSecretsKeyLabel, 1024)
+	registry, err := initKeyRegistry(ctx, client, rand, "namespace", "prefix", SealedSecretsKeyLabel, 1024, "CertNotBefore")
 	if err != nil {
 		t.Fatalf("initKeyRegistry() returned err: %v", err)
 	}
@@ -351,7 +351,7 @@ func TestLegacySecret(t *testing.T) {
 
 	client.ClearActions()
 
-	registry, err := initKeyRegistry(ctx, client, rand, "namespace", "prefix", SealedSecretsKeyLabel, 1024)
+	registry, err := initKeyRegistry(ctx, client, rand, "namespace", "prefix", SealedSecretsKeyLabel, 1024, "CertNotBefore")
 	if err != nil {
 		t.Fatalf("initKeyRegistry() returned err: %v", err)
 	}