Skip to content

Commit

Permalink
security: Add support for OCSP
Browse files Browse the repository at this point in the history
This adds support for certificate revocation for organizations who run
an OCSP server for their CA.

Closes cockroachdb#29641

Release note (security update): Certificate revocation is now
supported via OSCP and the setting `security.ocsp.mode`.
  • Loading branch information
bdarnell committed Aug 21, 2020
1 parent a752bad commit 228bf4a
Show file tree
Hide file tree
Showing 18 changed files with 325 additions and 52 deletions.
2 changes: 1 addition & 1 deletion pkg/cli/cert.go
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,7 @@ List certificates and keys found in the certificate directory.

// runListCerts loads and lists all certs.
func runListCerts(cmd *cobra.Command, args []string) error {
cm, err := security.NewCertificateManager(baseCfg.SSLCertsDir)
cm, err := security.NewCertificateManager(baseCfg.SSLCertsDir, security.CommandTLSSettings{})
if err != nil {
return errors.Wrap(err, "cannot load certificates")
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/cli/client_url.go
Original file line number Diff line number Diff line change
Expand Up @@ -352,7 +352,7 @@ func (cliCtx *cliContext) makeClientConnURL() (url.URL, error) {
if userName == "" {
userName = security.RootUser
}
sCtx := rpc.MakeSecurityContext(cliCtx.Config, roachpb.SystemTenantID)
sCtx := rpc.MakeSecurityContext(cliCtx.Config, security.CommandTLSSettings{}, roachpb.SystemTenantID)
if err := sCtx.LoadSecurityOptions(
opts, userName,
); err != nil {
Expand Down
4 changes: 2 additions & 2 deletions pkg/cli/start.go
Original file line number Diff line number Diff line change
Expand Up @@ -485,7 +485,7 @@ func runStart(cmd *cobra.Command, args []string, disableReplication bool) error
// (Re-)compute the client connection URL. We cannot do this
// earlier (e.g. above, in the runStart function) because
// at this time the address and port have not been resolved yet.
sCtx := rpc.MakeSecurityContext(serverCfg.Config, roachpb.SystemTenantID)
sCtx := rpc.MakeSecurityContext(serverCfg.Config, security.ClusterTLSSettings(serverCfg.Settings), roachpb.SystemTenantID)
pgURL, err := sCtx.PGURL(url.User(security.RootUser))
if err != nil {
log.Errorf(ctx, "failed computing the URL: %v", err)
Expand Down Expand Up @@ -658,7 +658,7 @@ If problems persist, please see %s.`
// (Re-)compute the client connection URL. We cannot do this
// earlier (e.g. above, in the runStart function) because
// at this time the address and port have not been resolved yet.
sCtx := rpc.MakeSecurityContext(serverCfg.Config, roachpb.SystemTenantID)
sCtx := rpc.MakeSecurityContext(serverCfg.Config, security.ClusterTLSSettings(serverCfg.Settings), roachpb.SystemTenantID)
pgURL, err := sCtx.PGURL(url.User(security.RootUser))
if err != nil {
log.Errorf(ctx, "failed computing the URL: %v", err)
Expand Down
3 changes: 2 additions & 1 deletion pkg/rpc/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (
circuit "github.com/cockroachdb/circuitbreaker"
"github.com/cockroachdb/cockroach/pkg/base"
"github.com/cockroachdb/cockroach/pkg/roachpb"
"github.com/cockroachdb/cockroach/pkg/security"
"github.com/cockroachdb/cockroach/pkg/settings/cluster"
"github.com/cockroachdb/cockroach/pkg/util/contextutil"
"github.com/cockroachdb/cockroach/pkg/util/envutil"
Expand Down Expand Up @@ -431,7 +432,7 @@ func NewContext(opts ContextOptions) *Context {

ctx := &Context{
ContextOptions: opts,
SecurityContext: MakeSecurityContext(opts.Config, opts.TenantID),
SecurityContext: MakeSecurityContext(opts.Config, security.ClusterTLSSettings(opts.Settings), opts.TenantID),
breakerClock: breakerClock{
clock: opts.Clock,
},
Expand Down
8 changes: 6 additions & 2 deletions pkg/rpc/tls.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ func wrapError(err error) error {
// the certificate manager.
type SecurityContext struct {
security.CertsLocator
security.TLSSettings
config *base.Config
tenID roachpb.TenantID
lazy struct {
Expand All @@ -68,9 +69,12 @@ type SecurityContext struct {
// MakeSecurityContext makes a SecurityContext.
//
// TODO(tbg): don't take a whole Config. This can be trimmed down significantly.
func MakeSecurityContext(cfg *base.Config, tenID roachpb.TenantID) SecurityContext {
func MakeSecurityContext(
cfg *base.Config, tlsSettings security.TLSSettings, tenID roachpb.TenantID,
) SecurityContext {
return SecurityContext{
CertsLocator: security.MakeCertsLocator(cfg.SSLCertsDir),
TLSSettings: tlsSettings,
config: cfg,
tenID: tenID,
}
Expand All @@ -86,7 +90,7 @@ func (ctx *SecurityContext) GetCertificateManager() (*security.CertificateManage
opts = append(opts, security.ForTenant(ctx.tenID.ToUint64()))
}
ctx.lazy.certificateManager.cm, ctx.lazy.certificateManager.err =
security.NewCertificateManager(ctx.config.SSLCertsDir, opts...)
security.NewCertificateManager(ctx.config.SSLCertsDir, ctx, opts...)

if ctx.lazy.certificateManager.err == nil && !ctx.config.Insecure {
infos, err := ctx.lazy.certificateManager.cm.ListCertificates()
Expand Down
27 changes: 21 additions & 6 deletions pkg/security/certificate_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,8 @@ type CertificateManager struct {
tenantIdentifier uint64
CertsLocator

tlsSettings TLSSettings

// The metrics struct is initialized at init time and metrics do their
// own locking.
certMetrics CertificateMetrics
Expand Down Expand Up @@ -152,7 +154,9 @@ type CertificateMetrics struct {
TenantClientExpiration *metric.Gauge
}

func makeCertificateManager(certsDir string, opts ...Option) *CertificateManager {
func makeCertificateManager(
certsDir string, tlsSettings TLSSettings, opts ...Option,
) *CertificateManager {
var o cmOptions
for _, fn := range opts {
fn(&o)
Expand All @@ -161,6 +165,7 @@ func makeCertificateManager(certsDir string, opts ...Option) *CertificateManager
return &CertificateManager{
CertsLocator: MakeCertsLocator(certsDir),
tenantIdentifier: o.tenantIdentifier,
tlsSettings: tlsSettings,
certMetrics: CertificateMetrics{
CAExpiration: metric.NewGauge(metaCAExpiration),
ClientCAExpiration: metric.NewGauge(metaClientCAExpiration),
Expand Down Expand Up @@ -193,17 +198,21 @@ func ForTenant(tenantIdentifier uint64) Option {
}

// NewCertificateManager creates a new certificate manager.
func NewCertificateManager(certsDir string, opts ...Option) (*CertificateManager, error) {
cm := makeCertificateManager(certsDir, opts...)
func NewCertificateManager(
certsDir string, tlsSettings TLSSettings, opts ...Option,
) (*CertificateManager, error) {
cm := makeCertificateManager(certsDir, tlsSettings, opts...)
return cm, cm.LoadCertificates()
}

// NewCertificateManagerFirstRun creates a new certificate manager.
// The certsDir is created if it does not exist.
// This should only be called when generating certificates, the server has
// no business creating the certs directory.
func NewCertificateManagerFirstRun(certsDir string, opts ...Option) (*CertificateManager, error) {
cm := makeCertificateManager(certsDir, opts...)
func NewCertificateManagerFirstRun(
certsDir string, tlsSettings TLSSettings, opts ...Option,
) (*CertificateManager, error) {
cm := makeCertificateManager(certsDir, tlsSettings, opts...)
if err := NewCertificateLoader(cm.certsDir).MaybeCreateCertsDir(); err != nil {
return nil, err
}
Expand Down Expand Up @@ -616,6 +625,7 @@ func (cm *CertificateManager) getEmbeddedServerTLSConfig(
}

cfg, err := newServerTLSConfig(
cm.tlsSettings,
nodeCert.FileContents,
nodeCert.KeyFileContents,
ca.FileContents,
Expand Down Expand Up @@ -680,6 +690,7 @@ func (cm *CertificateManager) getEmbeddedTenantServerTLSConfig(
}

cfg, err := newServerTLSConfig(
cm.tlsSettings,
serverCert.FileContents,
serverCert.KeyFileContents,
serverCA.FileContents,
Expand Down Expand Up @@ -723,6 +734,7 @@ func (cm *CertificateManager) getEmbeddedUIServerTLSConfig(
}

cfg, err := newUIServerTLSConfig(
cm.tlsSettings,
uiCert.FileContents,
uiCert.KeyFileContents)
if err != nil {
Expand Down Expand Up @@ -865,6 +877,7 @@ func (cm *CertificateManager) GetTenantClientTLSConfig() (*tls.Config, error) {
}

cfg, err := newClientTLSConfig(
cm.tlsSettings,
tenantClientCert.FileContents,
tenantClientCert.KeyFileContents,
ca.FileContents)
Expand Down Expand Up @@ -896,6 +909,7 @@ func (cm *CertificateManager) GetClientTLSConfig(user string) (*tls.Config, erro
}

cfg, err := newClientTLSConfig(
cm.tlsSettings,
clientCert.FileContents,
clientCert.KeyFileContents,
ca.FileContents)
Expand All @@ -918,6 +932,7 @@ func (cm *CertificateManager) GetClientTLSConfig(user string) (*tls.Config, erro
}

cfg, err := newClientTLSConfig(
cm.tlsSettings,
clientCert.FileContents,
clientCert.KeyFileContents,
ca.FileContents)
Expand All @@ -942,7 +957,7 @@ func (cm *CertificateManager) GetUIClientTLSConfig() (*tls.Config, error) {
return nil, err
}

cfg, err := newUIClientTLSConfig(uiCA.FileContents)
cfg, err := newUIClientTLSConfig(cm.tlsSettings, uiCA.FileContents)
if err != nil {
return nil, err
}
Expand Down
6 changes: 3 additions & 3 deletions pkg/security/certificate_manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ import (

func TestManagerWithEmbedded(t *testing.T) {
defer leaktest.AfterTest(t)()
cm, err := security.NewCertificateManager("test_certs")
cm, err := security.NewCertificateManager("test_certs", security.CommandTLSSettings{})
if err != nil {
t.Error(err)
}
Expand Down Expand Up @@ -104,11 +104,11 @@ func TestManagerWithPrincipalMap(t *testing.T) {
require.NoError(t, security.SetCertPrincipalMap(strings.Split(s, ",")))
}
newCertificateManager := func() error {
_, err := security.NewCertificateManager(certsDir)
_, err := security.NewCertificateManager(certsDir, security.CommandTLSSettings{})
return err
}
loadUserCert := func(user string) error {
cm, err := security.NewCertificateManager(certsDir)
cm, err := security.NewCertificateManager(certsDir, security.CommandTLSSettings{})
if err != nil {
return err
}
Expand Down
12 changes: 6 additions & 6 deletions pkg/security/certs.go
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ func createCACertAndKey(
caKeyPath = os.ExpandEnv(caKeyPath)

// Create a certificate manager with "create dir if not exist".
cm, err := NewCertificateManagerFirstRun(certsDir)
cm, err := NewCertificateManagerFirstRun(certsDir, CommandTLSSettings{})
if err != nil {
return err
}
Expand Down Expand Up @@ -269,7 +269,7 @@ func CreateNodePair(
caKeyPath = os.ExpandEnv(caKeyPath)

// Create a certificate manager with "create dir if not exist".
cm, err := NewCertificateManagerFirstRun(certsDir)
cm, err := NewCertificateManagerFirstRun(certsDir, CommandTLSSettings{})
if err != nil {
return err
}
Expand Down Expand Up @@ -328,7 +328,7 @@ func CreateUIPair(
caKeyPath = os.ExpandEnv(caKeyPath)

// Create a certificate manager with "create dir if not exist".
cm, err := NewCertificateManagerFirstRun(certsDir)
cm, err := NewCertificateManagerFirstRun(certsDir, CommandTLSSettings{})
if err != nil {
return err
}
Expand Down Expand Up @@ -390,7 +390,7 @@ func CreateClientPair(
caKeyPath = os.ExpandEnv(caKeyPath)

// Create a certificate manager with "create dir if not exist".
cm, err := NewCertificateManagerFirstRun(certsDir)
cm, err := NewCertificateManagerFirstRun(certsDir, CommandTLSSettings{})
if err != nil {
return err
}
Expand Down Expand Up @@ -471,7 +471,7 @@ func CreateTenantClientPair(
caKeyPath = os.ExpandEnv(caKeyPath)

// Create a certificate manager with "create dir if not exist".
cm, err := NewCertificateManagerFirstRun(certsDir)
cm, err := NewCertificateManagerFirstRun(certsDir, CommandTLSSettings{})
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -504,7 +504,7 @@ func CreateTenantClientPair(

// WriteTenantClientPair writes a TenantClientPair into certsDir.
func WriteTenantClientPair(certsDir string, cp *TenantClientPair, overwrite bool) error {
cm, err := NewCertificateManagerFirstRun(certsDir)
cm, err := NewCertificateManagerFirstRun(certsDir, CommandTLSSettings{})
if err != nil {
return err
}
Expand Down
6 changes: 3 additions & 3 deletions pkg/security/certs_rotation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ func TestRotateCerts(t *testing.T) {
// Test client with the same certs.
clientContext := testutils.NewNodeTestBaseContext()
clientContext.SSLCertsDir = certsDir
firstSCtx := rpc.MakeSecurityContext(clientContext, roachpb.SystemTenantID)
firstSCtx := rpc.MakeSecurityContext(clientContext, security.CommandTLSSettings{}, roachpb.SystemTenantID)
firstClient, err := firstSCtx.GetHTTPClient()
if err != nil {
t.Fatalf("could not create http client: %v", err)
Expand Down Expand Up @@ -132,7 +132,7 @@ func TestRotateCerts(t *testing.T) {
clientContext = testutils.NewNodeTestBaseContext()
clientContext.SSLCertsDir = certsDir

secondSCtx := rpc.MakeSecurityContext(clientContext, roachpb.SystemTenantID)
secondSCtx := rpc.MakeSecurityContext(clientContext, security.CommandTLSSettings{}, roachpb.SystemTenantID)
secondClient, err := secondSCtx.GetHTTPClient()
if err != nil {
t.Fatalf("could not create http client: %v", err)
Expand Down Expand Up @@ -201,7 +201,7 @@ func TestRotateCerts(t *testing.T) {
// This is HTTP and succeeds because we do not ask for or verify client certificates.
clientContext = testutils.NewNodeTestBaseContext()
clientContext.SSLCertsDir = certsDir
thirdSCtx := rpc.MakeSecurityContext(clientContext, roachpb.SystemTenantID)
thirdSCtx := rpc.MakeSecurityContext(clientContext, security.CommandTLSSettings{}, roachpb.SystemTenantID)
thirdClient, err := thirdSCtx.GetHTTPClient()
if err != nil {
t.Fatalf("could not create http client: %v", err)
Expand Down
4 changes: 2 additions & 2 deletions pkg/security/certs_tenant_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -100,14 +100,14 @@ func testTenantCertificatesInner(t *testing.T, embedded bool) {
// the server CA and server node certs, and it will validate incoming
// connections based on the tenant CA.

cm, err := security.NewCertificateManager(certsDir)
cm, err := security.NewCertificateManager(certsDir, security.CommandTLSSettings{})
require.NoError(t, err)
serverTLSConfig, err := cm.GetTenantServerTLSConfig()
require.NoError(t, err)

// Make a new CertificateManager for the tenant. We could've used this one
// for the server as well, but this way it's closer to reality.
cm, err = security.NewCertificateManager(certsDir, security.ForTenant(tenant))
cm, err = security.NewCertificateManager(certsDir, security.CommandTLSSettings{}, security.ForTenant(tenant))
require.NoError(t, err)

// The client in turn trusts the server CA and presents its tenant certs to the
Expand Down
16 changes: 9 additions & 7 deletions pkg/security/certs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ func TestGenerateCACert(t *testing.T) {
certsDir, cleanup := tempDir(t)
defer cleanup()

cm, err := security.NewCertificateManager(certsDir)
cm, err := security.NewCertificateManager(certsDir, security.CommandTLSSettings{})
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
Expand Down Expand Up @@ -339,6 +339,7 @@ func TestUseCerts(t *testing.T) {

// Load TLS Configs. This is what TestServer and HTTPClient do internally.
if _, err := security.LoadServerTLSConfig(
security.CommandTLSSettings{},
filepath.Join(certsDir, security.EmbeddedCACert),
filepath.Join(certsDir, security.EmbeddedCACert),
filepath.Join(certsDir, security.EmbeddedNodeCert),
Expand All @@ -347,6 +348,7 @@ func TestUseCerts(t *testing.T) {
t.Fatalf("Expected success, got %v", err)
}
if _, err := security.LoadClientTLSConfig(
security.CommandTLSSettings{},
filepath.Join(certsDir, security.EmbeddedCACert),
filepath.Join(certsDir, security.EmbeddedNodeCert),
filepath.Join(certsDir, security.EmbeddedNodeKey),
Expand All @@ -369,7 +371,7 @@ func TestUseCerts(t *testing.T) {
// Insecure mode.
clientContext := testutils.NewNodeTestBaseContext()
clientContext.Insecure = true
sCtx := rpc.MakeSecurityContext(clientContext, roachpb.SystemTenantID)
sCtx := rpc.MakeSecurityContext(clientContext, security.CommandTLSSettings{}, roachpb.SystemTenantID)
httpClient, err := sCtx.GetHTTPClient()
if err != nil {
t.Fatal(err)
Expand All @@ -389,7 +391,7 @@ func TestUseCerts(t *testing.T) {
clientContext = testutils.NewNodeTestBaseContext()
clientContext.SSLCertsDir = certsDir
{
secondSCtx := rpc.MakeSecurityContext(clientContext, roachpb.SystemTenantID)
secondSCtx := rpc.MakeSecurityContext(clientContext, security.CommandTLSSettings{}, roachpb.SystemTenantID)
httpClient, err = secondSCtx.GetHTTPClient()
}
if err != nil {
Expand Down Expand Up @@ -459,7 +461,7 @@ func TestUseSplitCACerts(t *testing.T) {
// Insecure mode.
clientContext := testutils.NewNodeTestBaseContext()
clientContext.Insecure = true
sCtx := rpc.MakeSecurityContext(clientContext, roachpb.SystemTenantID)
sCtx := rpc.MakeSecurityContext(clientContext, security.CommandTLSSettings{}, roachpb.SystemTenantID)
httpClient, err := sCtx.GetHTTPClient()
if err != nil {
t.Fatal(err)
Expand All @@ -479,7 +481,7 @@ func TestUseSplitCACerts(t *testing.T) {
clientContext = testutils.NewNodeTestBaseContext()
clientContext.SSLCertsDir = certsDir
{
secondSCtx := rpc.MakeSecurityContext(clientContext, roachpb.SystemTenantID)
secondSCtx := rpc.MakeSecurityContext(clientContext, security.CommandTLSSettings{}, roachpb.SystemTenantID)
httpClient, err = secondSCtx.GetHTTPClient()
}
if err != nil {
Expand Down Expand Up @@ -583,7 +585,7 @@ func TestUseWrongSplitCACerts(t *testing.T) {
// Insecure mode.
clientContext := testutils.NewNodeTestBaseContext()
clientContext.Insecure = true
sCtx := rpc.MakeSecurityContext(clientContext, roachpb.SystemTenantID)
sCtx := rpc.MakeSecurityContext(clientContext, security.CommandTLSSettings{}, roachpb.SystemTenantID)
httpClient, err := sCtx.GetHTTPClient()
if err != nil {
t.Fatal(err)
Expand All @@ -603,7 +605,7 @@ func TestUseWrongSplitCACerts(t *testing.T) {
clientContext = testutils.NewNodeTestBaseContext()
clientContext.SSLCertsDir = certsDir
{
secondCtx := rpc.MakeSecurityContext(clientContext, roachpb.SystemTenantID)
secondCtx := rpc.MakeSecurityContext(clientContext, security.CommandTLSSettings{}, roachpb.SystemTenantID)
httpClient, err = secondCtx.GetHTTPClient()
}
if err != nil {
Expand Down
Loading

0 comments on commit 228bf4a

Please sign in to comment.