[backport v1.8] Force state change if encryption used fallback (#2236)

Signed-off-by: Christian Mesh <christianmesh1@gmail.com>

CHANGELOG.md

@@ -2,6 +2,7 @@
 
 BUG FIXES:
 * Error messages related to validation of sensitive input variables will no longer disclose the sensitive value in the UI. ([#2219](https://github.com/opentofu/opentofu/pull/2219))
+* Changes to encryption configuration now auto-apply the migration ([#2232](https://github.com/opentofu/opentofu/pull/2232))
 
 
 ## 1.8.6

internal/encryption/base.go

@@ -7,6 +7,7 @@ package encryption
 
 import (
 	"encoding/json"
+	"errors"
 	"fmt"
 
 	"github.com/opentofu/opentofu/internal/configs"
@@ -119,11 +120,23 @@ func (s *baseEncryption) encrypt(data []byte, enhance func(basedata) interface{}
 	return jsond, nil
 }
 
+//nolint:revive // this name is fine
+type EncryptionStatus int
+
+const (
+	StatusUnknown   EncryptionStatus = 0
+	StatusSatisfied EncryptionStatus = 1
+	StatusMigration EncryptionStatus = 2
+)
+
 // TODO Find a way to make these errors actionable / clear
-func (s *baseEncryption) decrypt(data []byte, validator func([]byte) error) ([]byte, error) {
+//
+//nolint:cyclop // backport
+func (s *baseEncryption) decrypt(data []byte, validator func([]byte) error) ([]byte, EncryptionStatus, error) {
 	es := basedata{}
 	err := json.Unmarshal(data, &es)
 
+	//nolint:nestif // bugger off
 	if len(es.Version) == 0 || err != nil {
 		// Not a valid payload, might be already decrypted
 		verr := validator(data)
@@ -132,24 +145,34 @@ func (s *baseEncryption) decrypt(data []byte, validator func([]byte) error) ([]b
 
 			// Return the outer json error if we have one
 			if err != nil {
-				return nil, fmt.Errorf("invalid data format for decryption: %w, %w", err, verr)
+				return nil, StatusUnknown, fmt.Errorf("invalid data format for decryption: %w, %w", err, verr)
 			}
 
 			// Must have been invalid json payload
-			return nil, fmt.Errorf("unable to determine data structure during decryption: %w", verr)
+			return nil, StatusUnknown, fmt.Errorf("unable to determine data structure during decryption: %w", verr)
 		}
 
 		// Yep, it's already decrypted
+		unencryptedSupported := false
 		for _, method := range s.encMethods {
 			if unencrypted.Is(method) {
-				return data, nil
+				unencryptedSupported = true
+				break
 			}
 		}
-		return nil, fmt.Errorf("encountered unencrypted payload without unencrypted method configured")
+		if !unencryptedSupported {
+			return nil, StatusUnknown, fmt.Errorf("encountered unencrypted payload without unencrypted method configured")
+		}
+		if unencrypted.Is(s.encMethods[0]) {
+			// Decrypted and no pending migration
+			return data, StatusSatisfied, nil
+		}
+		// Decrypted and pending migration
+		return data, StatusMigration, nil
 	}
 
 	if es.Version != encryptionVersion {
-		return nil, fmt.Errorf("invalid encrypted payload version: %s != %s", es.Version, encryptionVersion)
+		return nil, StatusUnknown, fmt.Errorf("invalid encrypted payload version: %s != %s", es.Version, encryptionVersion)
 	}
 
 	// TODO Discuss if we should potentially cache this based on a json-encoded version of es.Meta and reduce overhead dramatically
@@ -157,11 +180,11 @@ func (s *baseEncryption) decrypt(data []byte, validator func([]byte) error) ([]b
 	if diags.HasErrors() {
 		// This cast to error here is safe as we know that at least one error exists
 		// This is also quite unlikely to happen as the constructor already has checked this code path
-		return nil, diags
+		return nil, StatusUnknown, diags
 	}
 
 	errs := make([]error, 0)
-	for _, method := range methods {
+	for i, method := range methods {
 		if unencrypted.Is(method) {
 			// Not applicable
 			continue
@@ -169,7 +192,12 @@ func (s *baseEncryption) decrypt(data []byte, validator func([]byte) error) ([]b
 		uncd, err := method.Decrypt(es.Data)
 		if err == nil {
 			// Success
-			return uncd, nil
+			if i == 0 {
+				// Decrypted with first method (encryption method)
+				return uncd, StatusSatisfied, nil
+			}
+			// Used a fallback
+			return uncd, StatusMigration, nil
 		}
 		// Record the failure
 		errs = append(errs, fmt.Errorf("attempted decryption failed for %s: %w", s.name, err))
@@ -182,5 +210,5 @@ func (s *baseEncryption) decrypt(data []byte, validator func([]byte) error) ([]b
 		errMessage += err.Error() + sep
 		sep = "\n"
 	}
-	return nil, fmt.Errorf(errMessage)
+	return nil, StatusUnknown, errors.New(errMessage)
 }

internal/encryption/example_test.go

@@ -80,11 +80,15 @@ func Example() {
 	}
 
 	// Decrypt
-	decryptedState, err := sfe.DecryptState(encrypted)
+	decryptedState, status, err := sfe.DecryptState(encrypted)
 	if err != nil {
 		panic(err)
 	}
 
+	if status != encryption.StatusSatisfied {
+		panic(status)
+	}
+
 	fmt.Printf("%s\n", decryptedState)
 	// Output: {"serial": 42, "lineage": "magic"}
 }

internal/encryption/plan.go

@@ -60,13 +60,14 @@ func (p planEncryption) EncryptPlan(data []byte) ([]byte, error) {
 }
 
 func (p planEncryption) DecryptPlan(data []byte) ([]byte, error) {
-	return p.base.decrypt(data, func(data []byte) error {
+	data, _, err := p.base.decrypt(data, func(data []byte) error {
 		// Check magic bytes
 		if len(data) < 2 || string(data[:2]) != "PK" {
 			return fmt.Errorf("Invalid plan file %v", string(data[:2]))
 		}
 		return nil
 	})
+	return data, err
 }
 
 func PlanEncryptionDisabled() PlanEncryption {

internal/encryption/state.go

@@ -32,7 +32,7 @@ type StateEncryption interface {
 	// function. Do not attempt to determine if the state file is encrypted as this function will take care of any
 	// and all encryption-related matters. After the function returns, use the returned byte array as a normal state
 	// file.
-	DecryptState([]byte) ([]byte, error)
+	DecryptState([]byte) ([]byte, EncryptionStatus, error)
 
 	// EncryptState encrypts a state file and returns the encrypted form.
 	//
@@ -87,8 +87,8 @@ func (s *stateEncryption) EncryptState(plainState []byte) ([]byte, error) {
 	})
 }
 
-func (s *stateEncryption) DecryptState(encryptedState []byte) ([]byte, error) {
-	decryptedState, err := s.base.decrypt(encryptedState, func(data []byte) error {
+func (s *stateEncryption) DecryptState(encryptedState []byte) ([]byte, EncryptionStatus, error) {
+	decryptedState, status, err := s.base.decrypt(encryptedState, func(data []byte) error {
 		tmp := struct {
 			FormatVersion string `json:"terraform_version"`
 		}{}
@@ -105,32 +105,32 @@ func (s *stateEncryption) DecryptState(encryptedState []byte) ([]byte, error) {
 	})
 
 	if err != nil {
-		return nil, err
+		return nil, status, err
 	}
 
 	// Make sure that the state passthrough fields match
 	var encrypted statedata
 	err = json.Unmarshal(encryptedState, &encrypted)
 	if err != nil {
-		return nil, err
+		return nil, status, err
 	}
 	var state statedata
 	err = json.Unmarshal(decryptedState, &state)
 	if err != nil {
-		return nil, err
+		return nil, status, err
 	}
 
 	// TODO make encrypted.Serial non-optional.  This is only for supporting alpha1 states!
 	if encrypted.Serial != nil && state.Serial != nil && *state.Serial != *encrypted.Serial {
-		return nil, fmt.Errorf("invalid state metadata, serial field mismatch %v vs %v", *encrypted.Serial, *state.Serial)
+		return nil, status, fmt.Errorf("invalid state metadata, serial field mismatch %v vs %v", *encrypted.Serial, *state.Serial)
 	}
 
 	// TODO make encrypted.Lineage non-optional.  This is only for supporting alpha1 states!
 	if encrypted.Lineage != "" && state.Lineage != encrypted.Lineage {
-		return nil, fmt.Errorf("invalid state metadata, linage field mismatch %v vs %v", encrypted.Lineage, state.Lineage)
+		return nil, status, fmt.Errorf("invalid state metadata, linage field mismatch %v vs %v", encrypted.Lineage, state.Lineage)
 	}
 
-	return decryptedState, nil
+	return decryptedState, status, nil
 }
 
 func StateEncryptionDisabled() StateEncryption {
@@ -142,6 +142,6 @@ type stateDisabled struct{}
 func (s *stateDisabled) EncryptState(plainState []byte) ([]byte, error) {
 	return plainState, nil
 }
-func (s *stateDisabled) DecryptState(encryptedState []byte) ([]byte, error) {
-	return encryptedState, nil
+func (s *stateDisabled) DecryptState(encryptedState []byte) ([]byte, EncryptionStatus, error) {
+	return encryptedState, StatusSatisfied, nil
 }

internal/plans/planfile/planfile_test.go

@@ -46,12 +46,14 @@ func TestRoundtrip(t *testing.T) {
 		Serial:           2,
 		Lineage:          "abc123",
 		State:            states.NewState(),
+		EncryptionStatus: encryption.StatusSatisfied,
 	}
 	prevStateFileIn := &statefile.File{
 		TerraformVersion: tfversion.SemVer,
 		Serial:           1,
 		Lineage:          "abc123",
 		State:            states.NewState(),
+		EncryptionStatus: encryption.StatusSatisfied,
 	}
 
 	// Minimal plan too, since the serialization of the tfplan portion of the

internal/states/remote/state.go

@@ -41,6 +41,7 @@ type State struct {
 	// state has changed from an existing state we read in.
 	lineage, readLineage string
 	serial, readSerial   uint64
+	readEncryption       encryption.EncryptionStatus
 	mu                   sync.Mutex
 	state, readState     *states.State
 	disableLocks         bool
@@ -176,6 +177,7 @@ func (s *State) refreshState() error {
 	// track changes as lineage, serial and/or state are mutated
 	s.readLineage = stateFile.Lineage
 	s.readSerial = stateFile.Serial
+	s.readEncryption = stateFile.EncryptionStatus
 	s.readState = s.state.DeepCopy()
 	return nil
 }
@@ -192,7 +194,7 @@ func (s *State) PersistState(schemas *tofu.Schemas) error {
 		lineageUnchanged := s.readLineage != "" && s.lineage == s.readLineage
 		serialUnchanged := s.readSerial != 0 && s.serial == s.readSerial
 		stateUnchanged := statefile.StatesMarshalEqual(s.state, s.readState)
-		if stateUnchanged && lineageUnchanged && serialUnchanged {
+		if stateUnchanged && lineageUnchanged && serialUnchanged && s.readEncryption != encryption.StatusMigration {
 			// If the state, lineage or serial haven't changed at all then we have nothing to do.
 			return nil
 		}
@@ -238,6 +240,7 @@ func (s *State) PersistState(schemas *tofu.Schemas) error {
 	// operation would correctly detect no changes to the lineage, serial or state.
 	s.readState = s.state.DeepCopy()
 	s.readLineage = s.lineage
+	s.readEncryption = encryption.StatusSatisfied
 	s.readSerial = s.serial
 	return nil
 }

internal/states/statefile/file.go

@@ -8,6 +8,7 @@ package statefile
 import (
 	version "github.com/hashicorp/go-version"
 
+	"github.com/opentofu/opentofu/internal/encryption"
 	"github.com/opentofu/opentofu/internal/states"
 	tfversion "github.com/opentofu/opentofu/version"
 )
@@ -34,6 +35,8 @@ type File struct {
 
 	// State is the actual state represented by this file.
 	State *states.State
+
+	EncryptionStatus encryption.EncryptionStatus
 }
 
 func New(state *states.State, lineage string, serial uint64) *File {
@@ -49,6 +52,7 @@ func New(state *states.State, lineage string, serial uint64) *File {
 		State:            state,
 		Lineage:          lineage,
 		Serial:           serial,
+		EncryptionStatus: encryption.StatusUnknown,
 	}
 }
 
@@ -63,5 +67,6 @@ func (f *File) DeepCopy() *File {
 		Serial:           f.Serial,
 		Lineage:          f.Lineage,
 		State:            f.State.DeepCopy(),
+		EncryptionStatus: f.EncryptionStatus,
 	}
 }

internal/states/statefile/read.go

@@ -76,7 +76,7 @@ func Read(r io.Reader, enc encryption.StateEncryption) (*File, error) {
 		return nil, ErrNoState
 	}
 
-	decrypted, err := enc.DecryptState(src)
+	decrypted, status, err := enc.DecryptState(src)
 	if err != nil {
 		return nil, err
 	}
@@ -91,6 +91,8 @@ func Read(r io.Reader, enc encryption.StateEncryption) (*File, error) {
 		panic("readState returned nil state with no errors")
 	}
 
+	state.EncryptionStatus = status
+
 	return state, diags.Err()
 }
 

internal/states/statefile/roundtrip_test.go

@@ -97,6 +97,10 @@ func TestRoundtripEncryption(t *testing.T) {
 	if err != nil {
 		t.Fatalf("unexpected error: %s", err)
 	}
+	// Check status
+	if originalState.EncryptionStatus != encryption.StatusMigration {
+		t.Fatal("wrong status")
+	}
 
 	// Write encrypted
 	var encrypted bytes.Buffer
@@ -117,6 +121,13 @@ func TestRoundtripEncryption(t *testing.T) {
 	if err != nil {
 		t.Fatalf("unexpected error: %s", err)
 	}
+	// Check status
+	if newState.EncryptionStatus != encryption.StatusSatisfied {
+		t.Fatal("wrong status")
+	}
+
+	// Overwrite status for deep comparison
+	originalState.EncryptionStatus = newState.EncryptionStatus
 
 	// Compare before/after encryption workflow
 	problems := deep.Equal(newState, originalState)