Fix issue #1201, dedupe fulcio certs. (#539)

Signed-off-by: Ville Aikas <[email protected]>

Author: vaikas

pkg/ctlog/config.go

@@ -218,16 +218,28 @@ func Unmarshal(ctx context.Context, in map[string][]byte) (*Config, error) {
 	if err != nil {
 		return nil, fmt.Errorf("decrypting existing private key: %w", err)
 	}
-	// If there's legacy rootCA entry, check it first.
+	// Make sure to dedupe along the way just to make sure we do not have
+	// duplicate entries.
+	uniqueFulcioCerts := map[string][]byte{}
+
+	// If there's legacy rootCA entry, check it first. This will get converted
+	// to fulcio-0 when marshaling, but we just want to make sure it's there
+	// when we're converting from ConfigMap based configuration into secret
+	// based one.
 	if legacyRoot, ok := in[LegacyRootCAKey]; ok && len(legacyRoot) > 0 {
-		ret.FulcioCerts = append(ret.FulcioCerts, legacyRoot)
+		uniqueFulcioCerts[string(legacyRoot)] = legacyRoot
 	}
-	// Then loop through Fulcio roots
+
 	for k, v := range in {
 		if strings.HasPrefix(k, "fulcio-") {
-			ret.FulcioCerts = append(ret.FulcioCerts, v)
+			uniqueFulcioCerts[string(v)] = v
 		}
 	}
+
+	// Then loop through Fulcio roots that have been deduped above
+	for _, v := range uniqueFulcioCerts {
+		ret.FulcioCerts = append(ret.FulcioCerts, v)
+	}
 	return &ret, nil
 }
 
@@ -244,7 +256,7 @@ func (c *Config) MarshalConfig(ctx context.Context) (map[string][]byte, error) {
 	// of files containing them for the RootsPemFile. Names don't matter
 	// so we just call them fulcio-%
 	// What matters however is to ensure that the filenames match the keys
-	// in the configmap / secret that we construc so they get properly mounted.
+	// in the configmap / secret that we construct so they get properly mounted.
 	rootPems := make([]string, 0, len(c.FulcioCerts))
 	for i := range c.FulcioCerts {
 		rootPems = append(rootPems, fmt.Sprintf("%sfulcio-%d", rootsPemFileDir, i))

pkg/ctlog/config_test.go

@@ -361,3 +361,39 @@ func TestDecrypteExistingPrivateKey(t *testing.T) {
 		t.Fatalf("got back a nil public key")
 	}
 }
+
+func TestDedupeUnmarshaling(t *testing.T) {
+	for k, v := range testConfigs {
+		t.Logf("testing with: %s", k)
+		cm, err := createBaseConfig(t, v)
+		if err != nil {
+			t.Fatalf("Failed to create base config: %v", err)
+		}
+		// Override the legacy rootca entry with our own for ease of testing. It
+		// doesn't really matter what it is for this test.
+		cm["fulcio-0"] = []byte("this is a test cert")
+		cm["fulcio-1"] = []byte("this is a test cert")
+		cm["fulcio-99"] = []byte("this is a different test cert")
+		config, err := Unmarshal(context.Background(), cm)
+		if err != nil {
+			t.Fatalf("failed to Unmarshal: %v", err)
+		}
+		// We should have original root cert, 0&1 deduped into one and fulcio-99
+		if len(config.FulcioCerts) != 3 {
+			t.Errorf("wanted 3 fulcio certs, got: %d", len(config.FulcioCerts))
+		}
+		checkContains(t, config.FulcioCerts, []byte("this is a test cert"))
+		checkContains(t, config.FulcioCerts, []byte("this is a different test cert"))
+		checkContains(t, config.FulcioCerts, cm["rootca"])
+	}
+}
+
+func checkContains(t *testing.T, fulcioCerts [][]byte, cert []byte) {
+	t.Helper()
+	for i := range fulcioCerts {
+		if bytes.Equal(fulcioCerts[i], cert) {
+			return
+		}
+	}
+	t.Errorf("did not find %s in fulcioCerts", string(cert))
+}