diff --git a/pkg/ctlog/config.go b/pkg/ctlog/config.go index 920cf5213..2d9d63305 100644 --- a/pkg/ctlog/config.go +++ b/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)) diff --git a/pkg/ctlog/config_test.go b/pkg/ctlog/config_test.go index e689fc793..0c761662e 100644 --- a/pkg/ctlog/config_test.go +++ b/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)) +}