Merge pull request #1906 from wallyworld/fix-certs-on-boot

Fix server cert on machine agent startup

Fixes: https://bugs.launchpad.net/bugs/1434680

This fix contains the 1.22 fix done by Menno plus new work required for 1.23.
The 1.22 fix ensures that when the state server certificate is updated, its DNSNames contains "local" and "juju-apiserver" and "juju-mongodb".

This 1.23 fix extends that by upgrading the state server cert as soon as the machine agent starts. This is because of an unrelated piece of work which updates the mongo upstart conf and caused mongo to restart. So this fix needs to occur right at the start of the machine agent startup, before EnsureServer() is called.

(Review request: http://reviews.vapour.ws/r/1229/)

cert/cert.go

@@ -117,6 +117,13 @@ func NewCA(envName string, expiry time.Time) (certPEM, keyPEM string, err error)
 	return string(certPEMData), string(keyPEMData), nil
 }
 
+// NewServer generates a certificate/key pair suitable for use by a server, with an
+// expiry time of 10 years.
+func NewDefaultServer(caCertPEM, caKeyPEM string, hostnames []string) (certPEM, keyPEM string, err error) {
+	expiry := time.Now().UTC().AddDate(10, 0, 0)
+	return newLeaf(caCertPEM, caKeyPEM, expiry, hostnames, []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth})
+}
+
 // NewServer generates a certificate/key pair suitable for use by a server.
 func NewServer(caCertPEM, caKeyPEM string, expiry time.Time, hostnames []string) (certPEM, keyPEM string, err error) {
 	return newLeaf(caCertPEM, caKeyPEM, expiry, hostnames, []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth})

cert/cert_test.go

@@ -83,10 +83,27 @@ func (certSuite) TestNewServer(c *gc.C) {
 	caCert, _, err := cert.ParseCertAndKey(caCertPEM, caKeyPEM)
 	c.Assert(err, jc.ErrorIsNil)
 
-	var noHostnames []string
-	srvCertPEM, srvKeyPEM, err := cert.NewServer(caCertPEM, caKeyPEM, expiry, noHostnames)
+	srvCertPEM, srvKeyPEM, err := cert.NewServer(caCertPEM, caKeyPEM, expiry, nil)
+	c.Assert(err, jc.ErrorIsNil)
+	checkCertificate(c, caCert, srvCertPEM, srvKeyPEM, now, expiry)
+}
+
+func (certSuite) TestNewDefaultServer(c *gc.C) {
+	now := time.Now()
+	expiry := roundTime(now.AddDate(1, 0, 0))
+	caCertPEM, caKeyPEM, err := cert.NewCA("foo", expiry)
 	c.Assert(err, jc.ErrorIsNil)
 
+	caCert, _, err := cert.ParseCertAndKey(caCertPEM, caKeyPEM)
+	c.Assert(err, jc.ErrorIsNil)
+
+	srvCertPEM, srvKeyPEM, err := cert.NewDefaultServer(caCertPEM, caKeyPEM, nil)
+	c.Assert(err, jc.ErrorIsNil)
+	srvCertExpiry := roundTime(now.AddDate(10, 0, 0))
+	checkCertificate(c, caCert, srvCertPEM, srvKeyPEM, now, srvCertExpiry)
+}
+
+func checkCertificate(c *gc.C, caCert *x509.Certificate, srvCertPEM, srvKeyPEM string, now, expiry time.Time) {
 	srvCert, srvKey, err := cert.ParseCertAndKey(srvCertPEM, srvKeyPEM)
 	c.Assert(err, jc.ErrorIsNil)
 	c.Assert(srvCert.Subject.CommonName, gc.Equals, "*")

cmd/jujud/agent/machine.go

@@ -19,6 +19,7 @@ import (
 	"github.com/juju/names"
 	"github.com/juju/utils"
 	"github.com/juju/utils/featureflag"
+	"github.com/juju/utils/set"
 	"github.com/juju/utils/symlink"
 	"github.com/juju/utils/voyeur"
 	"gopkg.in/juju/charm.v4"
@@ -34,6 +35,7 @@ import (
 	"github.com/juju/juju/api/metricsmanager"
 	"github.com/juju/juju/apiserver"
 	"github.com/juju/juju/apiserver/params"
+	"github.com/juju/juju/cert"
 	"github.com/juju/juju/cmd/jujud/reboot"
 	cmdutil "github.com/juju/juju/cmd/jujud/util"
 	"github.com/juju/juju/container"
@@ -325,20 +327,70 @@ func (a *MachineAgent) Dying() <-chan struct{} {
 	return a.tomb.Dying()
 }
 
+// upgradeCertificateDNSNames ensure that the state server certificate
+// recorded in the agent config and also mongo server.pem contains the
+// DNSNames entires required by Juju/
+func (a *MachineAgent) upgradeCertificateDNSNames() error {
+	agentConfig := a.CurrentConfig()
+	si, ok := agentConfig.StateServingInfo()
+	if !ok || si.CAPrivateKey == "" {
+		// No certificate information exists yet, nothing to do.
+		return nil
+	}
+	// Parse the current certificate to get the current dns names.
+	serverCert, err := cert.ParseCert(si.Cert)
+	if err != nil {
+		return err
+	}
+	update := false
+	dnsNames := set.NewStrings(serverCert.DNSNames...)
+	requiredDNSNames := []string{"local", "juju-apiserver", "juju-mongodb"}
+	for _, dnsName := range requiredDNSNames {
+		if dnsNames.Contains(dnsName) {
+			continue
+		}
+		dnsNames.Add(dnsName)
+		update = true
+	}
+	if !update {
+		return nil
+	}
+	// Write a new certificate to the mongp pem and agent config files.
+	si.Cert, si.PrivateKey, err = cert.NewDefaultServer(agentConfig.CACert(), si.CAPrivateKey, dnsNames.Values())
+	if err != nil {
+		return err
+	}
+	if err := mongo.UpdateSSLKey(agentConfig.DataDir(), si.Cert, si.PrivateKey); err != nil {
+		return err
+	}
+	return a.AgentConfigWriter.ChangeConfig(func(config agent.ConfigSetter) error {
+		config.SetStateServingInfo(si)
+		return nil
+	})
+}
+
 // Run runs a machine agent.
 func (a *MachineAgent) Run(*cmd.Context) error {
 
 	defer a.tomb.Done()
 	if err := a.ReadConfig(a.Tag().String()); err != nil {
 		return fmt.Errorf("cannot read agent configuration: %v", err)
 	}
-	agentConfig := a.CurrentConfig()
 
 	logger.Infof("machine agent %v start (%s [%s])", a.Tag(), version.Current, runtime.Compiler)
 	if flags := featureflag.String(); flags != "" {
 		logger.Warningf("developer feature flags enabled: %s", flags)
 	}
 
+	// Before doing anything else, we need to make sure the certificate generated for
+	// use by mongo to validate state server connections is correct. This needs to be done
+	// before any possible restart of the mongo service.
+	// See bug http://pad.lv/1434680
+	if err := a.upgradeCertificateDNSNames(); err != nil {
+		return errors.Annotate(err, "error upgrading server certificate")
+	}
+	agentConfig := a.CurrentConfig()
+
 	if err := a.upgradeWorkerContext.InitializeUsingAgent(a); err != nil {
 		return errors.Annotate(err, "error during upgradeWorkerContext initialisation")
 	}

cmd/jujud/agent/machine_test.go

@@ -1389,6 +1389,51 @@ func (s *MachineSuite) TestCertificateUpdateWorkerUpdatesCertificate(c *gc.C) {
 	}
 }
 
+func (s *MachineSuite) TestCertificateDNSUpdated(c *gc.C) {
+	// Disable the certificate work so it doesn't update the certificate.
+	newUpdater := func(certupdater.AddressWatcher, certupdater.StateServingInfoGetter, certupdater.EnvironConfigGetter,
+		certupdater.StateServingInfoSetter, chan params.StateServingInfo,
+	) worker.Worker {
+		return worker.NewNoOpWorker()
+	}
+	s.PatchValue(&newCertificateUpdater, newUpdater)
+
+	// Set up the machine agent.
+	m, _, _ := s.primeAgent(c, version.Current, state.JobManageEnviron)
+	a := s.newAgent(c, m)
+
+	// Set up check that certificate has been updated when the agent starts.
+	updated := make(chan struct{})
+	expectedDnsNames := set.NewStrings("local", "juju-apiserver", "juju-mongodb")
+	go func() {
+		for {
+			stateInfo, _ := a.CurrentConfig().StateServingInfo()
+			srvCert, err := cert.ParseCert(stateInfo.Cert)
+			c.Assert(err, jc.ErrorIsNil)
+			certDnsNames := set.NewStrings(srvCert.DNSNames...)
+			if !expectedDnsNames.Difference(certDnsNames).IsEmpty() {
+				continue
+			}
+			pemContent, err := ioutil.ReadFile(filepath.Join(s.DataDir(), "server.pem"))
+			c.Assert(err, jc.ErrorIsNil)
+			if string(pemContent) == stateInfo.Cert+"\n"+stateInfo.PrivateKey {
+				close(updated)
+				break
+			}
+			time.Sleep(10 * time.Millisecond)
+		}
+	}()
+
+	go func() { c.Check(a.Run(nil), jc.ErrorIsNil) }()
+	defer func() { c.Check(a.Stop(), jc.ErrorIsNil) }()
+	// Wait for certificate to be updated.
+	select {
+	case <-updated:
+	case <-time.After(coretesting.LongWait):
+		c.Fatalf("timeout while waiting for certificate to be updated")
+	}
+}
+
 func (s *MachineSuite) TestMachineAgentNetworkerMode(c *gc.C) {
 	tests := []struct {
 		about          string

environs/config/config.go

@@ -1430,7 +1430,7 @@ func (cfg *Config) GenerateStateServerCertAndKey(hostAddresses []string) (string
 	if !hasCAKey {
 		return "", "", fmt.Errorf("environment configuration has no ca-private-key")
 	}
-	return cert.NewServer(caCert, caKey, time.Now().UTC().AddDate(10, 0, 0), hostAddresses)
+	return cert.NewDefaultServer(caCert, caKey, hostAddresses)
 }
 
 // SpecializeCharmRepo customizes a repository for a given configuration.

mongo/mongo.go

@@ -220,10 +220,8 @@ func EnsureServer(args EnsureServerParams) error {
 		}
 	}
 
-	certKey := args.Cert + "\n" + args.PrivateKey
-	err = utils.AtomicWriteFile(sslKeyPath(args.DataDir), []byte(certKey), 0600)
-	if err != nil {
-		return fmt.Errorf("cannot write SSL key: %v", err)
+	if err := UpdateSSLKey(args.DataDir, args.Cert, args.PrivateKey); err != nil {
+		return err
 	}
 
 	err = utils.AtomicWriteFile(sharedSecretPath(args.DataDir), []byte(args.SharedSecret), 0600)
@@ -260,6 +258,13 @@ func EnsureServer(args EnsureServerParams) error {
 	return nil
 }
 
+// UpdateSSLKey writes a new SSL key used by mongo to validate connections from Juju state server(s)
+func UpdateSSLKey(dataDir, cert, privateKey string) error {
+	certKey := cert + "\n" + privateKey
+	err := utils.AtomicWriteFile(sslKeyPath(dataDir), []byte(certKey), 0600)
+	return errors.Annotate(err, "cannot write SSL key")
+}
+
 func makeJournalDirs(dataDir string) error {
 	journalDir := path.Join(dataDir, "journal")
 	if err := os.MkdirAll(journalDir, 0700); err != nil {

worker/certupdater/certupdater.go

@@ -100,10 +100,12 @@ func (c *CertificateUpdater) Handle() error {
 		return errors.Annotate(err, "cannot add CA private key to environment config")
 	}
 
-	// For backwards compatibility, we must include "juju-apiserver" as a
-	// hostname as that is what clients specify as the hostname for verification.
-	// We also explicitly include localhost.
-	serverAddrs := []string{"localhost", "juju-apiserver"}
+	// For backwards compatibility, we must include "juju-apiserver"
+	// and "juju-mongodb" as hostnames as that is what clients specify
+	// as the hostname for verification (this certicate is used both
+	// for serving MongoDB and API server connections).  We also
+	// explicitly include localhost.
+	serverAddrs := []string{"localhost", "juju-apiserver", "juju-mongodb"}
 	for _, addr := range addresses {
 		if addr.Value == "localhost" {
 			continue

worker/certupdater/certupdater_test.go

@@ -134,9 +134,12 @@ func (s *CertUpdaterSuite) TestAddressChange(c *gc.C) {
 		c.Fatalf("timed out waiting for certificate to be updated")
 	}
 
-	// The server certificates must report "juju-apiserver" as a DNS name
-	// for backwards-compatibility with API clients.
-	c.Assert(srvCert.DNSNames, gc.DeepEquals, []string{"localhost", "juju-apiserver"})
+	// The server certificates must report "juju-apiserver" as a DNS
+	// name for backwards-compatibility with API clients. They must
+	// also report "juju-mongodb" because these certicates are also
+	// used for serving MongoDB connections.
+	c.Assert(srvCert.DNSNames, gc.DeepEquals,
+		[]string{"localhost", "juju-apiserver", "juju-mongodb"})
 }
 
 type mockStateServingGetterNoCAKey struct{}