Skip to content

Commit

Permalink
feat(kuma-cp): added validation for backend name
Browse files Browse the repository at this point in the history
Backend name for builtin CA should be lowercase and must be valid dns name. Because we are creating k8s secret from this name, which has strict naming rules.

Fix #5012

Signed-off-by: Marcin Skalski <marcin.skalski@konghq.com>
  • Loading branch information
Automaat committed Sep 29, 2022
1 parent b6fdbda commit 10641ec
Show file tree
Hide file tree
Showing 2 changed files with 75 additions and 4 deletions.
21 changes: 17 additions & 4 deletions pkg/plugins/ca/builtin/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"fmt"

"github.com/asaskevich/govalidator"
"github.com/pkg/errors"

mesh_proto "github.com/kumahq/kuma/api/mesh/v1alpha1"
Expand All @@ -23,6 +24,8 @@ import (

var log = core.Log.WithName("ca").WithName("builtin")

const MaxBackendNameLength = 255

type builtinCaManager struct {
secretManager manager.ResourceManager
}
Expand Down Expand Up @@ -60,9 +63,15 @@ func (b *builtinCaManager) ValidateBackend(ctx context.Context, mesh string, bac
cfg := &config.BuiltinCertificateAuthorityConfig{}
if err := util_proto.ToTyped(backend.Conf, cfg); err != nil {
verr.AddViolation("", "could not convert backend config: "+err.Error())
return verr.OrNil()
}
return nil
backendNameWithPrefix := createSecretName(mesh, backend.Name, "cert")
if len(backendNameWithPrefix) > MaxBackendNameLength {
verr.AddViolationAt(core_validators.RootedAt("mtls").Field("backends").Field("name"), fmt.Sprintf("Backend name is too long. Max length: %d", MaxBackendNameLength))
}
if !govalidator.IsDNSName(backendNameWithPrefix) || !govalidator.IsLowerCase(backendNameWithPrefix) {
verr.AddViolationAt(core_validators.RootedAt("mtls").Field("backends").Field("name"), fmt.Sprintf("%q name must be valid dns name", backend.Name))
}
return verr.OrNil()
}

func (b *builtinCaManager) UsedSecrets(mesh string, backend *mesh_proto.CertificateAuthorityBackend) ([]string, error) {
Expand Down Expand Up @@ -114,17 +123,21 @@ func (b *builtinCaManager) create(ctx context.Context, mesh string, backend *mes
func certSecretResKey(mesh string, backendName string) core_model.ResourceKey {
return core_model.ResourceKey{
Mesh: mesh,
Name: fmt.Sprintf("%s.ca-builtin-cert-%s", mesh, backendName), // we add mesh as a prefix to have uniqueness of Secret names on K8S
Name: createSecretName(mesh, backendName, "cert"),
}
}

func keySecretResKey(mesh string, backendName string) core_model.ResourceKey {
return core_model.ResourceKey{
Mesh: mesh,
Name: fmt.Sprintf("%s.ca-builtin-key-%s", mesh, backendName), // we add mesh as a prefix to have uniqueness of Secret names on K8S
Name: createSecretName(mesh, backendName, "key"),
}
}

func createSecretName(mesh string, backendName string, secretType string) string {
return fmt.Sprintf("%s.ca-builtin-%s-%s", mesh, secretType, backendName) // we add mesh as a prefix to have uniqueness of Secret names on K8S
}

func (b *builtinCaManager) GetRootCert(ctx context.Context, mesh string, backend *mesh_proto.CertificateAuthorityBackend) ([]core_ca.Cert, error) {
ca, err := b.getCa(ctx, mesh, backend.Name)
if err != nil {
Expand Down
58 changes: 58 additions & 0 deletions pkg/plugins/ca/builtin/manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"encoding/pem"
"time"

"github.com/ghodss/yaml"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"

Expand Down Expand Up @@ -147,6 +148,63 @@ var _ = Describe("Builtin CA Manager", func() {
})
})

Context("ValidateBackend", func() {
It("should validate backend", func() {
// given
mesh := "default"
backend := &mesh_proto.CertificateAuthorityBackend{
Name: "builtin-1",
Type: "builtin",
}

// when
err := caManager.ValidateBackend(context.Background(), mesh, backend)

// then
Expect(err).ToNot(HaveOccurred())
})

It("should fail validation when name too long", func() {
// given
mesh := "default"
backend := &mesh_proto.CertificateAuthorityBackend{
Name: "backend-long.backend-long.backend-long.backend-long.backend-long.backend-long.backend-long.backend-long.backend-long.backend-long.backend-long.backend-long.backend-long.backend-long.backend-long.backend-long.backend-long.backend-long.backend-long",
Type: "builtin",
}

// when
verr := caManager.ValidateBackend(context.Background(), mesh, backend)

// then
actual, err := yaml.Marshal(verr)
Expect(err).ToNot(HaveOccurred())
Expect(actual).To(MatchYAML(`
violations:
- field: mtls.backends.name
message: 'Backend name is too long. Max length: 255'`))
})

It("should fail validation when backend name contains uppercase letters", func() {
// given
mesh := "default"
backend := &mesh_proto.CertificateAuthorityBackend{
Name: "builtinCA",
Type: "builtin",
}

// when
verr := caManager.ValidateBackend(context.Background(), mesh, backend)

// then
actual, err := yaml.Marshal(verr)
Expect(err).ToNot(HaveOccurred())
Expect(actual).To(MatchYAML(`
violations:
- field: mtls.backends.name
message: '"builtinCA" name must be valid dns name'`))
})
})

Context("GetRootCert", func() {
It("should retrieve created certs", func() {
// given
Expand Down

0 comments on commit 10641ec

Please sign in to comment.