From 674206320c4280f8a11b76872138285a2c179902 Mon Sep 17 00:00:00 2001
From: Herman Slatman <hermanslatman@hotmail.com>
Date: Tue, 11 Oct 2022 14:12:06 +0200
Subject: [PATCH] Write updated CA configuration after migrating provisioners

---
 authority/authority.go     | 31 ++++++++++++++++++++++---------
 authority/config/config.go | 25 +++++++++++++++++++++++++
 2 files changed, 47 insertions(+), 9 deletions(-)

diff --git a/authority/authority.go b/authority/authority.go
index 188633c3..e3bc3473 100644
--- a/authority/authority.go
+++ b/authority/authority.go
@@ -605,8 +605,8 @@ func (a *Authority) init() error {
 		if len(provs) == 0 && !strings.EqualFold(a.config.AuthorityConfig.DeploymentType, "linked") {
 			var firstJWKProvisioner *linkedca.Provisioner
 			if len(a.config.AuthorityConfig.Provisioners) > 0 {
-				a.initLogf("Starting migration of provisioners")
 				// Existing provisioners detected; try migrating them to DB storage
+				a.initLogf("Starting migration of provisioners")
 				for _, p := range a.config.AuthorityConfig.Provisioners {
 					lp, err := ProvisionerToLinkedca(p)
 					if err != nil {
@@ -627,15 +627,28 @@ func (a *Authority) init() error {
 					}
 				}
 
-				// TODO(hs): try to update ca.json to remove migrated provisioners from the
-				// file? This may not always be possible though, so we shouldn't fail hard on
-				// every error. The next time the CA runs, it won't have perform the migration,
-				// because there'll be at least a JWK provisioner.
+				// TODO(hs): test if this works with LinkedCA too. Also could be useful
+				// for printing out where the configuration is read from in case of LinkedCA.
+				c := a.config
+				if c.WasLoadedFromFile() {
+					// TODO(hs): check if prerequisites for writing files look OK (user/group, permission bits, etc) as
+					// extra safety check before trying to write at all?
 
-				// 1. check if prerequisites for writing files look OK (user/group, permission bits, etc)
-				// 2. update the configuration to write (internal representation; do a deep copy first?)
-				// 3. try writing the new ca.json
-				// 4. on failure, perform rollback of the write (restore original in internal representation)
+					// Remove the existing provisioners from the authority configuration
+					// and commit it to the existing configuration file. NOTE: committing
+					// the configuration at this point also writes other properties that
+					// have been initialized with default values, such as the `backdate` and
+					// `template` settings in the `authority`.
+					oldProvisioners := c.AuthorityConfig.Provisioners
+					c.AuthorityConfig.Provisioners = []provisioner.Interface{}
+					if err := c.Commit(); err != nil {
+						// Restore the provisioners in in-memory representation for consistency
+						// when writing the updated configuration fails. This is considered a soft
+						// error, so execution can continue.
+						c.AuthorityConfig.Provisioners = oldProvisioners
+						a.initLogf("Failed removing provisioners from configuration: %v", err)
+					}
+				}
 
 				a.initLogf("Finished migrating provisioners")
 			}
diff --git a/authority/config/config.go b/authority/config/config.go
index c5e74b39..79228e98 100644
--- a/authority/config/config.go
+++ b/authority/config/config.go
@@ -73,6 +73,9 @@ type Config struct {
 	Templates        *templates.Templates `json:"templates,omitempty"`
 	CommonName       string               `json:"commonName,omitempty"`
 	SkipValidation   bool                 `json:"-"`
+
+	// Keeps record of the filename the Config is read from
+	loadedFromFilename string
 }
 
 // ASN1DN contains ASN1.DN attributes that are used in Subject and Issuer
@@ -163,6 +166,10 @@ func LoadConfiguration(filename string) (*Config, error) {
 		return nil, errors.Wrapf(err, "error parsing %s", filename)
 	}
 
+	// store filename that was read to populate Config
+	c.loadedFromFilename = filename
+
+	// initialize the Config
 	c.Init()
 
 	return &c, nil
@@ -199,6 +206,24 @@ func (c *Config) Save(filename string) error {
 	return errors.Wrapf(enc.Encode(c), "error writing %s", filename)
 }
 
+// Commit saves the current configuration to the same
+// file it was initially loaded from.
+//
+// TODO(hs): rename Save() to WriteTo() and replace this
+// with Save()? Or is Commit clear enough.
+func (c *Config) Commit() error {
+	if !c.WasLoadedFromFile() {
+		return errors.New("cannot commit configuration if not loaded from file")
+	}
+	return c.Save(c.loadedFromFilename)
+}
+
+// WasLoadedFromFile returns whether or not the Config was
+// read from a file.
+func (c *Config) WasLoadedFromFile() bool {
+	return c.loadedFromFilename != ""
+}
+
 // Validate validates the configuration.
 func (c *Config) Validate() error {
 	switch {