Merge pull request #4382 from oasisprotocol/tjanez/sentry-client-cert…

…-save

go/common/identity: Save re-generated sentry client and node's persistent TLS certificate

.changelog/4382.bugfix.1.md

@@ -0,0 +1,8 @@
+go/common/identity: Save re-generated sentry client TLS certificate
+
+Sentry client TLS certificate is always re-generated from the private key when
+the Oasis Node starts.
+
+Previously, the re-generated sentry client TLS certificate was not saved to
+disk, which caused confusion since the on-disk certificate file (i.e.
+`sentry_client_tls_identity_cert.pem`) had incorrect/outdated expiry date.

.changelog/4382.bugfix.2.md

@@ -0,0 +1,8 @@
+go/common/identity: Save re-generated node's persistent TLS certificate
+
+If a node's TLS certificate is persistent, it is always re-generated from the
+private key when the Oasis Node starts.
+
+Previously, the re-generated node's persistent TLS certificate was not saved
+to disk, which caused confusion since the on-disk certificate file (i.e.
+`tls_identity_cert.pem`) had incorrect/outdated expiry date.

go/common/identity/identity.go

@@ -269,86 +269,96 @@ func doLoadOrGenerate(dataDir string, signerFactory signature.SignerFactory, sho
 	var (
 		nextCert   *tls.Certificate
 		nextSigner signature.Signer
-		dnr        bool
+		dontRotate bool
 	)
 
-	// First, check if we can load the TLS certificate from disk.
+	// Load and re-generate node's persistent TLS certificate (if it exists).
+	// NOTE: This will reuse the node's persistent TLS private key (if it
+	// exists) and re-generate the TLS certificate with a validity of 1 year.
+	// NOTE: The node needs to be restarted at least once a year so the TLS
+	// certificate doesn't expire.
 	tlsCertPath, tlsKeyPath := TLSCertPaths(dataDir)
 	cert, err := tlsCert.LoadFromKey(tlsKeyPath, CommonName)
-	if err == nil {
-		// Load successful, ensure that we won't ever rotate the certificates.
-		dnr = true
-	} else {
+	if err == nil || persistTLS {
 		if persistTLS {
-			// Freshly generate TLS certificates.
+			// Loading node's persistent TLS private key failed, generate a new
+			// private key and the corresponding TLS certificate.
 			cert, err = tlsCert.Generate(CommonName)
 			if err != nil {
 				return nil, err
 			}
+		}
+		// Save re-generated TLS certificate (and private key) to disk.
+		err = tlsCert.Save(tlsCertPath, tlsKeyPath, cert)
+		if err != nil {
+			return nil, err
+		}
 
-			// Save generated TLS certificate to disk.
-			err = tlsCert.Save(tlsCertPath, tlsKeyPath, cert)
-			if err != nil {
-				return nil, err
-			}
-
-			// Disable TLS rotation if we're persisting TLS certificates.
-			dnr = true
-		} else {
-			// Current key; try loading, else generate, then save.
-			keyPath := ephemeralKeyPath(dataDir, tlsEphemeralGenCurrent)
-			cert, err = tlsCert.LoadFromKey(keyPath, CommonName)
-			if err != nil {
-				if !os.IsNotExist(err) {
-					return nil, fmt.Errorf("identity: unable to read ephemeral key from file: %w", err)
-				}
-				cert, err = tlsCert.Generate(CommonName)
-				if err != nil {
-					return nil, err
-				}
+		// Disable TLS rotation since we've either successfully loaded an
+		// existing node's persistent TLS certificate or persisting TLS
+		// certificates has been requested.
+		dontRotate = true
+	} else {
+		// Use ephemeral TLS keys.
+		// Current key; try loading, else generate, then save.
+		keyPath := ephemeralKeyPath(dataDir, tlsEphemeralGenCurrent)
+		cert, err = tlsCert.LoadFromKey(keyPath, CommonName)
+		if err != nil {
+			if !os.IsNotExist(err) {
+				return nil, fmt.Errorf("identity: unable to read ephemeral key from file: %w", err)
 			}
-			err = tlsCert.SaveKey(keyPath, cert)
+			cert, err = tlsCert.Generate(CommonName)
 			if err != nil {
 				return nil, err
 			}
+		}
+		err = tlsCert.SaveKey(keyPath, cert)
+		if err != nil {
+			return nil, err
+		}
 
-			// Next key, to be used in the next rotation; load or generate.
-			nextKeyPath := ephemeralKeyPath(dataDir, tlsEphemeralGenNext)
-			nextCert, err = tlsCert.LoadFromKey(nextKeyPath, CommonName)
-			if err != nil {
-				if !os.IsNotExist(err) {
-					return nil, fmt.Errorf("identity: unable to read next ephemeral key from file: %w", err)
-				}
-				nextCert, err = tlsCert.Generate(CommonName)
-				if err != nil {
-					return nil, err
-				}
+		// Next key, to be used in the next rotation; load or generate.
+		nextKeyPath := ephemeralKeyPath(dataDir, tlsEphemeralGenNext)
+		nextCert, err = tlsCert.LoadFromKey(nextKeyPath, CommonName)
+		if err != nil {
+			if !os.IsNotExist(err) {
+				return nil, fmt.Errorf("identity: unable to read next ephemeral key from file: %w", err)
 			}
-			err = tlsCert.SaveKey(nextKeyPath, nextCert)
+			nextCert, err = tlsCert.Generate(CommonName)
 			if err != nil {
 				return nil, err
 			}
-			nextSigner = memory.NewFromRuntime(nextCert.PrivateKey.(ed25519.PrivateKey))
 		}
+		err = tlsCert.SaveKey(nextKeyPath, nextCert)
+		if err != nil {
+			return nil, err
+		}
+		nextSigner = memory.NewFromRuntime(nextCert.PrivateKey.(ed25519.PrivateKey))
 	}
 
-	// Load or generate the sentry client certificate for this node.
+	// Load and re-generate the sentry client TLS certificate for this node (if
+	// it exists).
+	// NOTE: This will reuse the sentry client's private key (if it exists)
+	// and re-generate the TLS certificate with a validity of 1 year.
+	// NOTE: The node needs to be restarted at least once a year so the TLS
+	// certificate doesn't expire.
 	tlsSentryClientCertPath, tlsSentryClientKeyPath := TLSSentryClientCertPaths(dataDir)
 	sentryClientCert, err := tlsCert.LoadFromKey(tlsSentryClientKeyPath, CommonName)
 	if err != nil {
 		if !os.IsNotExist(err) {
 			return nil, fmt.Errorf("identity: unable to read sentry client key from file: %w", err)
 		}
-		// Load failed, generate fresh sentry client cert.
+		// Loading sentry client's private key failed, generate a new
+		// private key and the corresponding TLS certificate.
 		sentryClientCert, err = tlsCert.Generate(CommonName)
 		if err != nil {
 			return nil, err
 		}
-		// And save it to disk.
-		err = tlsCert.Save(tlsSentryClientCertPath, tlsSentryClientKeyPath, sentryClientCert)
-		if err != nil {
-			return nil, err
-		}
+	}
+	// Save the re-generated TLS certificate (and private key) to disk.
+	err = tlsCert.Save(tlsSentryClientCertPath, tlsSentryClientKeyPath, sentryClientCert)
+	if err != nil {
+		return nil, err
 	}
 
 	return &Identity{
@@ -360,7 +370,7 @@ func doLoadOrGenerate(dataDir string, signerFactory signature.SignerFactory, sho
 		tlsCertificate:             cert,
 		nextTLSSigner:              nextSigner,
 		nextTLSCertificate:         nextCert,
-		DoNotRotateTLS:             dnr,
+		DoNotRotateTLS:             dontRotate,
 		TLSSentryClientCertificate: sentryClientCert,
 		tlsRotationNotifier:        pubsub.NewBroker(false),
 		dataDir:                    dataDir,

go/common/identity/identity_test.go

@@ -20,10 +20,17 @@ func TestLoadOrGenerate(t *testing.T) {
 	factory, err := fileSigner.NewFactory(dataDir, RequiredSignerRoles...)
 	require.NoError(t, err, "NewFactory")
 
+	tlsCertPath, _ := TLSCertPaths(dataDir)
+	tlsSentryClientCertPath, _ := TLSSentryClientCertPaths(dataDir)
+
 	// Generate a new identity.
 	identity, err := LoadOrGenerate(dataDir, factory, true)
 	require.NoError(t, err, "LoadOrGenerate")
 	require.EqualValues(t, []signature.PublicKey{identity.GetTLSSigner().Public()}, identity.GetTLSPubKeys())
+	tlsCertFile1, err := ioutil.ReadFile(tlsCertPath)
+	require.NoError(t, err, "read TLS cert")
+	tlsSentryClientCertFile1, err := ioutil.ReadFile(tlsSentryClientCertPath)
+	require.NoError(t, err, "read sentry client TLS cert")
 
 	// Sleep to make sure that any regenerated TLS certificates will have different expiration.
 	time.Sleep(2 * time.Second)
@@ -40,6 +47,12 @@ func TestLoadOrGenerate(t *testing.T) {
 	require.EqualValues(t, identity.GetTLSPubKeys(), identity2.GetTLSPubKeys())
 	require.NotEqual(t, identity.TLSSentryClientCertificate, identity2.TLSSentryClientCertificate)
 	require.EqualValues(t, identity.TLSSentryClientCertificate.PrivateKey, identity2.TLSSentryClientCertificate.PrivateKey)
+	tlsCertFile2, err := ioutil.ReadFile(tlsCertPath)
+	require.NoError(t, err, "read TLS cert (2)")
+	require.NotEqualValues(t, tlsCertFile1, tlsCertFile2)
+	tlsSentryClientCertFile2, err := ioutil.ReadFile(tlsSentryClientCertPath)
+	require.NoError(t, err, "read sentry client TLS cert (2)")
+	require.NotEqualValues(t, tlsSentryClientCertFile1, tlsSentryClientCertFile2)
 
 	dataDir2, err := ioutil.TempDir("", "oasis-identity-test2_")
 	require.NoError(t, err, "create data dir (2)")