Skip to content
This repository has been archived by the owner on Jul 11, 2023. It is now read-only.

Commit

Permalink
feat(certificates) rework vault certificate provider (#4596)
Browse files Browse the repository at this point in the history 
* feat(certificates) rework vault certificate provider
This change pulls out the shared logic between Vault into
the new manager struct. This is done in pieces to avoid a single large PR.
The goal is to reduce all of the existing "providers" to clients that only
deal with certificates (no caching, msg broker, logging, or timing).

Signed-off-by: Sarah Christoff <[email protected]>

* fix errors

Signed-off-by: Sarah Christoff <[email protected]>

* fix goimports

Signed-off-by: Sarah Christoff <[email protected]>

* add assert

* move assert, remove return

Signed-off-by: Sarah Christoff <[email protected]>

* remove whitelines

Signed-off-by: Sarah Christoff <[email protected]>
  • Loading branch information
@schristoff
schristoff authored Mar 30, 2022
1 parent 8bae12f commit d1100a8
Show file tree
Hide file tree
Showing 11 changed files with 1,201 additions and 506 deletions.
198 changes: 145 additions & 53 deletions go.mod
View file

Large diffs are not rendered by default.

899 changes: 854 additions & 45 deletions go.sum
View file

Large diffs are not rendered by default.

23 changes: 16 additions & 7 deletions pkg/certificate/providers/config.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -291,24 +291,33 @@ func GetCertFromKubernetes(ns string, secretName string, kubeClient kubernetes.I
// getHashiVaultOSMCertificateManager returns a certificate manager instance with Hashi Vault as the certificate provider
func (c *Config) getHashiVaultOSMCertificateManager(options VaultOptions) (certificate.Manager, debugger.CertificateManagerDebugger, error) {
if _, ok := map[string]interface{}{"http": nil, "https": nil}[options.VaultProtocol]; !ok {
return nil, nil, errors.Errorf("Value %s is not a valid Hashi Vault protocol", options.VaultProtocol)
return nil, nil, fmt.Errorf("value %s is not a valid Hashi Vault protocol", options.VaultProtocol)
}

// A Vault address would have the following shape: "http://vault.default.svc.cluster.local:8200"
vaultAddr := fmt.Sprintf("%s://%s:%d", options.VaultProtocol, options.VaultHost, options.VaultPort)
vaultCertManager, err := vault.NewCertManager(
vaultClient, err := vault.New(
vaultAddr,
options.VaultToken,
options.VaultRole,
c.cfg,
c.cfg.GetServiceCertValidityPeriod(),
c.msgBroker,
)
if err != nil {
return nil, nil, errors.Errorf("Error instantiating Hashicorp Vault as a Certificate Manager: %+v", err)
return nil, nil, fmt.Errorf("error instantiating Hashicorp Vault as a Certificate Manager: %w", err)
}

vaultCert, err := vaultClient.GetRootCertificate()
if err != nil {
return nil, nil, fmt.Errorf("error getting Vault Root Certificate, got: %w", err)
}

return vaultCertManager, vaultCertManager, nil
certManager, err := certificate.NewManager(vaultCert, vaultClient, c.cfg.GetServiceCertValidityPeriod(), c.msgBroker)
if err != nil {
return nil, nil, fmt.Errorf("error instantiating osm certificate.Manager for Vault cert-manager : %w", err)
}

rotor.New(certManager).Start(checkCertificateExpirationInterval)

return certManager, certManager, nil
}

// getCertManagerOSMCertificateManager returns a certificate manager instance with cert-manager as the certificate provider
Expand Down
176 changes: 28 additions & 148 deletions pkg/certificate/providers/vault/certificate_manager.go
View file
Original file line number Diff line number Diff line change
@@ -1,21 +1,16 @@
package vault

import (
"fmt"
"time"

"github.com/hashicorp/vault/api"
"github.com/pkg/errors"

"github.com/openservicemesh/osm/pkg/announcements"
"github.com/openservicemesh/osm/pkg/certificate"
"github.com/openservicemesh/osm/pkg/certificate/pem"
"github.com/openservicemesh/osm/pkg/certificate/rotor"
"github.com/openservicemesh/osm/pkg/configurator"
"github.com/openservicemesh/osm/pkg/constants"
"github.com/openservicemesh/osm/pkg/errcode"
"github.com/openservicemesh/osm/pkg/k8s/events"
"github.com/openservicemesh/osm/pkg/logger"
"github.com/openservicemesh/osm/pkg/messaging"
)

var log = logger.New("vault")
Expand All @@ -30,180 +25,65 @@ const (
commonNameField = "common_name"
ttlField = "ttl"

checkCertificateExpirationInterval = 5 * time.Second
decade = 8765 * time.Hour
decade = 8765 * time.Hour
)

// NewCertManager implements certificate.Manager and wraps a Hashi Vault with methods to allow easy certificate issuance.
func NewCertManager(
vaultAddr,
token string,
role string,
cfg configurator.Configurator,
serviceCertValidityDuration time.Duration,
msgBroker *messaging.Broker) (*CertManager, error) {
// New constructs a new certificate client using Vault's cert-manager
func New(vaultAddr, token, role string) (*CertManager, error) {
if vaultAddr == "" {
return nil, fmt.Errorf("vault address must not be empty")
}
if token == "" {
return nil, fmt.Errorf("vault token must not be empty")
}
if role == "" {
return nil, fmt.Errorf("vault role must not be empty")
}
c := &CertManager{
role: vaultRole(role),
cfg: cfg,
serviceCertValidityDuration: serviceCertValidityDuration,
msgBroker: msgBroker,
role: role,
}
config := api.DefaultConfig()
config.Address = vaultAddr

var err error
if c.client, err = api.NewClient(config); err != nil {
return nil, errors.Errorf("Error creating Vault CertManager without TLS at %s", vaultAddr)
return nil, fmt.Errorf("error creating Vault CertManager without TLS at %s, got err: %w", vaultAddr, err)
}

log.Info().Msgf("Created Vault CertManager, with role=%q at %v", role, vaultAddr)

c.client.SetToken(token)

issuingCA, serialNumber, err := c.getIssuingCA(c.issue)
if err != nil {
return nil, err
}

c.ca = &certificate.Certificate{
CommonName: constants.CertificationAuthorityCommonName,
SerialNumber: serialNumber,
Expiration: time.Now().Add(decade),
CertChain: issuingCA,
IssuingCA: issuingCA,
}

// Instantiating a new certificate rotation mechanism will start a goroutine for certificate rotation.
rotor.New(c).Start(checkCertificateExpirationInterval)

return c, nil
}

func (cm *CertManager) getIssuingCA(issue func(certificate.CommonName, time.Duration) (*certificate.Certificate, error)) ([]byte, certificate.SerialNumber, error) {
// Create a temp certificate to determine the public part of the issuing CA
cert, err := issue("localhost", decade)
if err != nil {
return nil, "", err
}

issuingCA := cert.GetIssuingCA()

// We are not going to need this certificate - remove it
cm.ReleaseCertificate(cert.GetCommonName())

return issuingCA, cert.GetSerialNumber(), err
}

func (cm *CertManager) issue(cn certificate.CommonName, validityPeriod time.Duration) (*certificate.Certificate, error) {
secret, err := cm.client.Logical().Write(getIssueURL(cm.role).String(), getIssuanceData(cn, validityPeriod))
// IssueCertificate requests a new signed certificate from the configured Vault issuer.
func (cm *CertManager) IssueCertificate(cn certificate.CommonName, validityPeriod time.Duration) (*certificate.Certificate, error) {
secret, err := cm.client.Logical().Write(getIssueURL(cm.role), getIssuanceData(cn, validityPeriod))
if err != nil {
// TODO(#3962): metric might not be scraped before process restart resulting from this error
log.Error().Err(err).Str(errcode.Kind, errcode.GetErrCodeWithMetric(errcode.ErrIssuingCert)).
Msgf("Error issuing new certificate for CN=%s", cn)
return nil, err
}

return newCert(cn, secret, time.Now().Add(validityPeriod)), nil
}

func (cm *CertManager) deleteFromCache(cn certificate.CommonName) {
cm.cache.Delete(cn)
}

func (cm *CertManager) getFromCache(cn certificate.CommonName) *certificate.Certificate {
if certificateInterface, exists := cm.cache.Load(cn); exists {
cert := certificateInterface.(*certificate.Certificate)
log.Trace().Msgf("Certificate found in cache SerialNumber=%s", cert.GetSerialNumber())
if cert.ShouldRotate() {
log.Trace().Msgf("Certificate found in cache but has expired SerialNumber=%s", cert.GetSerialNumber())
return nil
}
return cert
}
return nil
}

// IssueCertificate issues a certificate by leveraging the Hashi Vault CertManager.
func (cm *CertManager) IssueCertificate(cn certificate.CommonName, validityPeriod time.Duration) (*certificate.Certificate, error) {
start := time.Now()

if cert := cm.getFromCache(cn); cert != nil {
return cert, nil
}

cert, err := cm.issue(cn, validityPeriod)
if err != nil {
return cert, err
}

cm.cache.Store(cn, cert)

log.Trace().Msgf("Issued new certificate with SerialNumber=%s took %+v", cert.GetSerialNumber(), time.Since(start))

return cert, nil
}

// ReleaseCertificate is called when a cert will no longer be needed and should be removed from the system.
func (cm *CertManager) ReleaseCertificate(cn certificate.CommonName) {
// TODO(draychev): implement Hashicorp Vault delete-cert API here: https://github.com/openservicemesh/osm/issues/2068
cm.deleteFromCache(cn)
}

// ListCertificates lists all certificates issued
func (cm *CertManager) ListCertificates() ([]*certificate.Certificate, error) {
var certs []*certificate.Certificate
cm.cache.Range(func(cnInterface interface{}, certInterface interface{}) bool {
certs = append(certs, certInterface.(*certificate.Certificate))
return true // continue the iteration
})
return certs, nil
}

// GetCertificate returns a certificate given its Common Name (CN)
func (cm *CertManager) GetCertificate(cn certificate.CommonName) (*certificate.Certificate, error) {
if cert := cm.getFromCache(cn); cert != nil {
return cert, nil
}
return nil, errCertNotFound
}

// GetRootCertificate returns the root certificate.
func (cm *CertManager) GetRootCertificate() (*certificate.Certificate, error) {
return cm.ca, nil
}

// RotateCertificate implements certificate.Manager and rotates an existing certificate.
func (cm *CertManager) RotateCertificate(cn certificate.CommonName) (*certificate.Certificate, error) {
start := time.Now()

oldCert, ok := cm.cache.Load(cn)
if !ok {
return nil, errors.Errorf("Old certificate does not exist for CN=%s", cn)
}

// We want the validity duration of the CertManager to remain static during the lifetime
// of the CertManager. This tests to see if this value is set, and if it isn't then it
// should make the infrequent call to configuration to get this value and cache it for
// future certificate operations.
if cm.serviceCertValidityDuration == 0 {
cm.serviceCertValidityDuration = cm.cfg.GetServiceCertValidityPeriod()
}
newCert, err := cm.issue(cn, cm.serviceCertValidityDuration)
// Create a temp certificate to determine the public part of the issuing CA
cert, err := cm.IssueCertificate("localhost", decade)
if err != nil {
return nil, err
}

cm.cache.Store(cn, newCert)

cm.msgBroker.GetCertPubSub().Pub(events.PubSubMessage{
Kind: announcements.CertificateRotated,
NewObj: newCert,
OldObj: oldCert.(*certificate.Certificate),
}, announcements.CertificateRotated.String())

log.Debug().Msgf("Rotated certificate (old SerialNumber=%s) with new SerialNumber=%s took %+v", oldCert.(*certificate.Certificate).GetSerialNumber(), newCert.SerialNumber, time.Since(start))

return newCert, nil
//TODO(2068): implement a delete cert
return &certificate.Certificate{
CommonName: constants.CertificationAuthorityCommonName,
SerialNumber: cert.GetSerialNumber(),
Expiration: time.Now().Add(decade),
CertChain: cert.CertChain,
IssuingCA: cert.IssuingCA,
}, err
}

func newCert(cn certificate.CommonName, secret *api.Secret, expiration time.Time) *certificate.Certificate {
Expand Down
Loading

0 comments on commit d1100a8

Please sign in to comment.