Skip to content

Commit

Permalink
Remove the requirement of an identity certificate
Browse files Browse the repository at this point in the history
This commit removes the requirement of having an identity certificate
per issuer. This certificate was used to make the request "GET /roots"
that might require an mTLS certificate in some deployments. The change
removes these requests and uses the defined bundle in the issuer instead.

Fixes #111
  • Loading branch information
maraino committed Aug 29, 2023
1 parent 83cf219 commit 1b64cd3
Show file tree
Hide file tree
Showing 7 changed files with 18 additions and 67 deletions.
3 changes: 1 addition & 2 deletions api/v1beta1/stepclusterissuer_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,7 @@ type StepClusterIssuerSpec struct {
// CABundle is a base64 encoded TLS certificate used to verify connections
// to the step certificates server. If not set the system root certificates
// are used to validate the TLS connection.
// +optional
CABundle []byte `json:"caBundle,omitempty"`
CABundle []byte `json:"caBundle"`
}

// StepClusterIssuerStatus defines the observed state of StepClusterIssuer
Expand Down
3 changes: 1 addition & 2 deletions api/v1beta1/stepissuer_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,7 @@ type StepIssuerSpec struct {
// CABundle is a base64 encoded TLS certificate used to verify connections
// to the step certificates server. If not set the system root certificates
// are used to validate the TLS connection.
// +optional
CABundle []byte `json:"caBundle,omitempty"`
CABundle []byte `json:"caBundle"`
}

// StepIssuerStatus defines the observed state of StepIssuer
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ spec:
description: URL is the base URL for the step certificates instance.
type: string
required:
- caBundle
- provisioner
- url
type: object
Expand Down
1 change: 1 addition & 0 deletions config/crd/bases/certmanager.step.sm_stepissuers.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ spec:
description: URL is the base URL for the step certificates instance.
type: string
required:
- caBundle
- provisioner
- url
type: object
Expand Down
3 changes: 2 additions & 1 deletion controllers/stepclusterissuer_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,6 @@ func (r *StepClusterIssuerReconciler) Reconcile(ctx context.Context, req ctrl.Re
}

// Initialize and store the provisioner
//nolint:contextcheck // legacy
p, err := provisioners.NewFromStepClusterIssuer(iss, password)
if err != nil {
log.Error(err, "failed to initialize provisioner")
Expand All @@ -111,6 +110,8 @@ func validateStepClusterIssuerSpec(s api.StepClusterIssuerSpec) error {
switch {
case s.URL == "":
return fmt.Errorf("spec.url cannot be empty")
case len(s.CABundle) == 0:
return fmt.Errorf("spec.caBundle cannot be empty")
case s.Provisioner.Name == "":
return fmt.Errorf("spec.provisioner.name cannot be empty")
case s.Provisioner.KeyID == "":
Expand Down
3 changes: 2 additions & 1 deletion controllers/stepissuer_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,6 @@ func (r *StepIssuerReconciler) Reconcile(ctx context.Context, req ctrl.Request)
}

// Initialize and store the provisioner
//nolint:contextcheck // legacy
p, err := provisioners.NewFromStepIssuer(iss, password)
if err != nil {
log.Error(err, "failed to initialize provisioner")
Expand All @@ -111,6 +110,8 @@ func validateStepIssuerSpec(s api.StepIssuerSpec) error {
switch {
case s.URL == "":
return fmt.Errorf("spec.url cannot be empty")
case len(s.CABundle) == 0:
return fmt.Errorf("spec.caBundle cannot be empty")
case s.Provisioner.Name == "":
return fmt.Errorf("spec.provisioner.name cannot be empty")
case s.Provisioner.KeyID == "":
Expand Down
71 changes: 10 additions & 61 deletions provisioners/step.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,62 +21,47 @@ var collection = new(sync.Map)
// requests using step certificates.
type Step struct {
name string
caBundle []byte
provisioner *ca.Provisioner
}

// NewFromStepIssuer returns a new Step provisioner, configured with the information in the
// given issuer.
func NewFromStepIssuer(iss *api.StepIssuer, password []byte) (*Step, error) {
var options []ca.ClientOption
if len(iss.Spec.CABundle) > 0 {
options = append(options, ca.WithCABundle(iss.Spec.CABundle))
options := []ca.ClientOption{
ca.WithCABundle(iss.Spec.CABundle),
}

provisioner, err := ca.NewProvisioner(iss.Spec.Provisioner.Name, iss.Spec.Provisioner.KeyID, iss.Spec.URL, password, options...)
if err != nil {
return nil, err
}

p := &Step{
name: iss.Name + "." + iss.Namespace,
caBundle: iss.Spec.CABundle,
provisioner: provisioner,
}

// Request identity certificate if required.
if version, err := provisioner.Version(); err == nil {
if version.RequireClientAuthentication {
if err := p.createIdentityCertificate(); err != nil {
return nil, err
}
}
}

return p, nil
}

func NewFromStepClusterIssuer(iss *api.StepClusterIssuer, password []byte) (*Step, error) {
var options []ca.ClientOption
if len(iss.Spec.CABundle) > 0 {
options = append(options, ca.WithCABundle(iss.Spec.CABundle))
options := []ca.ClientOption{
ca.WithCABundle(iss.Spec.CABundle),
}

provisioner, err := ca.NewProvisioner(iss.Spec.Provisioner.Name, iss.Spec.Provisioner.KeyID, iss.Spec.URL, password, options...)
if err != nil {
return nil, err
}

p := &Step{
name: iss.Name + "." + iss.Namespace,
caBundle: iss.Spec.CABundle,
provisioner: provisioner,
}

// Request identity certificate if required.
if version, err := provisioner.Version(); err == nil {
if version.RequireClientAuthentication {
if err := p.createIdentityCertificate(); err != nil {
return nil, err
}
}
}

return p, nil
}

Expand All @@ -95,45 +80,9 @@ func Store(namespacedName types.NamespacedName, provisioner *Step) {
collection.Store(namespacedName, provisioner)
}

func (s *Step) createIdentityCertificate() error {
csr, pk, err := ca.CreateCertificateRequest(s.name)
if err != nil {
return err
}
token, err := s.provisioner.Token(s.name)
if err != nil {
return err
}
resp, err := s.provisioner.Sign(&capi.SignRequest{
CsrPEM: *csr,
OTT: token,
})
if err != nil {
return err
}
tr, err := s.provisioner.Client.Transport(context.Background(), resp, pk)
if err != nil {
return err
}
s.provisioner.Client.SetTransport(tr)
return nil
}

// Sign sends the certificate requests to the Step CA and returns the signed
// certificate.
func (s *Step) Sign(_ context.Context, cr *certmanager.CertificateRequest) ([]byte, []byte, error) {
// Get root certificate(s)
roots, err := s.provisioner.Roots()
if err != nil {
return nil, nil, err
}

// Encode root certificates
caPem, err := encodeX509(roots.Certificates...)
if err != nil {
return nil, nil, err
}

// decode and check certificate request
csr, err := decodeCSR(cr.Spec.Request)
if err != nil {
Expand Down Expand Up @@ -180,7 +129,7 @@ func (s *Step) Sign(_ context.Context, cr *certmanager.CertificateRequest) ([]by
if err != nil {
return nil, nil, err
}
return chainPem, caPem, nil
return chainPem, s.caBundle, nil
}

// decodeCSR decodes a certificate request in PEM format and returns the
Expand Down

0 comments on commit 1b64cd3

Please sign in to comment.