Repository: kopia/kopia
Pull request #3351 (2023-09-24 19:29:19)

refactor(repository): Move encryption into a common package in preparation to handle secrets
As requested in #3345 this is just a code reorg with no functionality change to prepare for reusing the encryption functions for the secret manager.



codecov[bot]: ## [Codecov](https://app.codecov.io/gh/kopia/kopia/pull/3351?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=kopia) Report
Patch coverage: **`66.66%`** and project coverage change: **`-0.05%`** :warning:
> Comparison is base [(`814738c`)](https://app.codecov.io/gh/kopia/kopia/commit/814738c74e7dcf02dbcea676c857e690afd57d7c?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=kopia) 75.88% compared to head [(`50ccabc`)](https://app.codecov.io/gh/kopia/kopia/pull/3351?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=kopia) 75.83%.

<details><summary>Additional details and impacted files</summary>


```diff
@@            Coverage Diff             @@
##           master    #3351      +/-   ##
==========================================
- Coverage   75.88%   75.83%   -0.05%     
==========================================
  Files         462      463       +1     
  Lines       37082    37100      +18     
==========================================
- Hits        28138    28136       -2     
- Misses       7020     7034      +14     
- Partials     1924     1930       +6     
```


| [Files Changed](https://app.codecov.io/gh/kopia/kopia/pull/3351?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=kopia) | Coverage Δ | |
|---|---|---|
| [internal/crypto/aesgcm.go](https://app.codecov.io/gh/kopia/kopia/pull/3351?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=kopia#diff-aW50ZXJuYWwvY3J5cHRvL2Flc2djbS5nbw==) | `62.50% <62.50%> (ø)` | |
| [internal/crypto/key\_derivation\_testing.go](https://app.codecov.io/gh/kopia/kopia/pull/3351?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=kopia#diff-aW50ZXJuYWwvY3J5cHRvL2tleV9kZXJpdmF0aW9uX3Rlc3RpbmcuZ28=) | `58.33% <66.66%> (ø)` | |
| [repo/format/format\_blob.go](https://app.codecov.io/gh/kopia/kopia/pull/3351?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=kopia#diff-cmVwby9mb3JtYXQvZm9ybWF0X2Jsb2IuZ28=) | `75.40% <66.66%> (+3.04%)` | :arrow_up: |
| [internal/crypto/key\_derivation.go](https://app.codecov.io/gh/kopia/kopia/pull/3351?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=kopia#diff-aW50ZXJuYWwvY3J5cHRvL2tleV9kZXJpdmF0aW9uLmdv) | `71.42% <100.00%> (ø)` | |
| [repo/open.go](https://app.codecov.io/gh/kopia/kopia/pull/3351?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=kopia#diff-cmVwby9vcGVuLmdv) | `77.70% <100.00%> (-0.99%)` | :arrow_down: |
| [repo/repository.go](https://app.codecov.io/gh/kopia/kopia/pull/3351?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=kopia#diff-cmVwby9yZXBvc2l0b3J5Lmdv) | `79.34% <100.00%> (ø)` | |

... and [4 files with indirect coverage changes](https://app.codecov.io/gh/kopia/kopia/pull/3351/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=kopia)


</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/kopia/kopia/pull/3351?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=kopia).   
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=kopia).


Patch:
diff --git a/internal/crypto/aesgcm.go b/internal/crypto/aesgcm.go
new file mode 100644
index 00000000..2ca29143
--- /dev/null
+++ b/internal/crypto/aesgcm.go
@@ -0,0 +1,80 @@
+// Package crypto implements common symmetric-encryption and key-derivation functions.
+package crypto
+
+import (
+	"crypto/aes"
+	"crypto/cipher"
+	"crypto/rand"
+	"io"
+
+	"github.com/pkg/errors"
+)
+
+//nolint:gochecknoglobals
+var (
+	purposeAESKey   = []byte("AES")
+	purposeAuthData = []byte("CHECKSUM")
+)
+
+func initCrypto(masterKey, salt []byte) (cipher.AEAD, []byte, error) {
+	aesKey := DeriveKeyFromMasterKey(masterKey, salt, purposeAESKey, 32)     //nolint:gomnd
+	authData := DeriveKeyFromMasterKey(masterKey, salt, purposeAuthData, 32) //nolint:gomnd
+
+	blk, err := aes.NewCipher(aesKey)
+	if err != nil {
+		return nil, nil, errors.Wrap(err, "cannot create cipher")
+	}
+
+	aead, err := cipher.NewGCM(blk)
+	if err != nil {
+		return nil, nil, errors.Wrap(err, "cannot create cipher")
+	}
+
+	return aead, authData, nil
+}
+
+// EncryptAes256Gcm encrypts data with AES 256 GCM.
+func EncryptAes256Gcm(data, masterKey, salt []byte) ([]byte, error) {
+	aead, authData, err := initCrypto(masterKey, salt)
+	if err != nil {
+		return nil, errors.Wrap(err, "unable to initialize crypto")
+	}
+
+	nonceLength := aead.NonceSize()
+	noncePlusContentLength := nonceLength + len(data)
+	cipherText := make([]byte, noncePlusContentLength+aead.Overhead())
+
+	// Store nonce at the beginning of ciphertext.
+	nonce := cipherText[0:nonceLength]
+	if _, err := io.ReadFull(rand.Reader, nonce); err != nil {
+		return nil, errors.Wrap(err, "error reading random bytes for nonce")
+	}
+
+	b := aead.Seal(cipherText[nonceLength:nonceLength], nonce, data, authData)
+	data = nonce[0 : nonceLength+len(b)]
+
+	return data, nil
+}
+
+// DecryptAes256Gcm encrypts data with AES 256 GCM.
+func DecryptAes256Gcm(data, masterKey, salt []byte) ([]byte, error) {
+	aead, authData, err := initCrypto(masterKey, salt)
+	if err != nil {
+		return nil, errors.Wrap(err, "cannot initialize cipher")
+	}
+
+	data = append([]byte(nil), data...)
+	if len(data) < aead.NonceSize() {
+		return nil, errors.Errorf("invalid encrypted payload, too short")
+	}
+
+	nonce := data[0:aead.NonceSize()]
+	payload := data[aead.NonceSize():]
+
+	plainText, err := aead.Open(payload[:0], nonce, payload, authData)
+	if err != nil {
+		return nil, errors.Errorf("unable to decrypt repository blob, invalid credentials?")
+	}
+
+	return plainText, nil
+}
diff --git a/repo/format/crypto_key_derivation.go b/internal/crypto/key_derivation.go
similarity index 68%
rename from repo/format/crypto_key_derivation.go
rename to internal/crypto/key_derivation.go
index 115adb24..4dc02a70 100644
--- a/repo/format/crypto_key_derivation.go
+++ b/internal/crypto/key_derivation.go
@@ -1,4 +1,4 @@
-package format
+package crypto
 
 import (
 	"crypto/sha256"
@@ -8,9 +8,9 @@
 )
 
 // DeriveKeyFromMasterKey computes a key for a specific purpose and length using HKDF based on the master key.
-func DeriveKeyFromMasterKey(masterKey, uniqueID, purpose []byte, length int) []byte {
+func DeriveKeyFromMasterKey(masterKey, salt, purpose []byte, length int) []byte {
 	key := make([]byte, length)
-	k := hkdf.New(sha256.New, masterKey, uniqueID, purpose)
+	k := hkdf.New(sha256.New, masterKey, salt, purpose)
 
 	if _, err := io.ReadFull(k, key); err != nil {
 		panic("unable to derive key from master key, this should never happen")
diff --git a/repo/format/format_blob.go b/repo/format/format_blob.go
index 9ce6ba43..0dcb9435 100644
--- a/repo/format/format_blob.go
+++ b/repo/format/format_blob.go
@@ -3,16 +3,13 @@
 
 import (
 	"context"
-	"crypto/aes"
-	"crypto/cipher"
 	"crypto/hmac"
-	"crypto/rand"
 	"crypto/sha256"
 	"encoding/json"
-	"io"
 
 	"github.com/pkg/errors"
 
+	"github.com/kopia/kopia/internal/crypto"
 	"github.com/kopia/kopia/internal/gather"
 	"github.com/kopia/kopia/repo/blob"
 )
@@ -20,6 +17,9 @@
 // DefaultFormatEncryption is the identifier of the default format blob encryption algorithm.
 const DefaultFormatEncryption = "AES256_GCM"
 
+// DefaultKeyDerivationAlgorithm is the key derivation algorithm for new configurations.
+const DefaultKeyDerivationAlgorithm = crypto.DefaultKeyDerivationAlgorithm
+
 const (
 	aes256GcmEncryption             = "AES256_GCM"
 	lengthOfRecoverBlockLength      = 2 // number of bytes used to store recover block length
@@ -37,9 +37,6 @@
 
 //nolint:gochecknoglobals
 var (
-	purposeAESKey   = []byte("AES")
-	purposeAuthData = []byte("CHECKSUM")
-
 	// formatBlobChecksumSecret is a HMAC secret used for checksumming the format content.
 	// It's not really a secret, but will provide positive identification of blocks that
 	// are repository format blocks.
@@ -73,6 +70,16 @@ func ParseKopiaRepositoryJSON(b []byte) (*KopiaRepositoryJSON, error) {
 	return f, nil
 }
 
+// DeriveFormatEncryptionKeyFromPassword derives encryption key using the provided password and per-repository unique ID.
+func (f *KopiaRepositoryJSON) DeriveFormatEncryptionKeyFromPassword(password string) ([]byte, error) {
+	res, err := crypto.DeriveKeyFromPassword(password, f.UniqueID, f.KeyDerivationAlgorithm)
+	if err != nil {
+		return nil, errors.Wrap(err, "Failed to derive format encryption key")
+	}
+
+	return res, nil
+}
+
 // RecoverFormatBlob attempts to recover format blob replica from the specified file.
 // The format blob can be either the prefix or a suffix of the given file.
 // optionally the length can be provided (if known) to speed up recovery.
@@ -190,65 +197,22 @@ func (f *KopiaRepositoryJSON) WriteKopiaRepositoryBlobWithID(ctx context.Context
 	return nil
 }
 
-func initCrypto(masterKey, repositoryID []byte) (cipher.AEAD, []byte, error) {
-	aesKey := DeriveKeyFromMasterKey(masterKey, repositoryID, purposeAESKey, 32)     //nolint:gomnd
-	authData := DeriveKeyFromMasterKey(masterKey, repositoryID, purposeAuthData, 32) //nolint:gomnd
-
-	blk, err := aes.NewCipher(aesKey)
-	if err != nil {
-		return nil, nil, errors.Wrap(err, "cannot create cipher")
-	}
-
-	aead, err := cipher.NewGCM(blk)
-	if err != nil {
-		return nil, nil, errors.Wrap(err, "cannot create cipher")
-	}
-
-	return aead, authData, nil
-}
-
 func encryptRepositoryBlobBytesAes256Gcm(data, masterKey, repositoryID []byte) ([]byte, error) {
-	aead, authData, err := initCrypto(masterKey, repositoryID)
+	res, err := crypto.EncryptAes256Gcm(data, masterKey, repositoryID)
 	if err != nil {
-		return nil, errors.Wrap(err, "unable to initialize crypto")
+		return nil, errors.Wrap(err, "Failed to encrypt blob")
 	}
 
-	nonceLength := aead.NonceSize()
-	noncePlusContentLength := nonceLength + len(data)
-	cipherText := make([]byte, noncePlusContentLength+aead.Overhead())
-
-	// Store nonce at the beginning of ciphertext.
-	nonce := cipherText[0:nonceLength]
-	if _, err := io.ReadFull(rand.Reader, nonce); err != nil {
-		return nil, errors.Wrap(err, "error reading random bytes for nonce")
-	}
-
-	b := aead.Seal(cipherText[nonceLength:nonceLength], nonce, data, authData)
-	data = nonce[0 : nonceLength+len(b)]
-
-	return data, nil
+	return res, nil
 }
 
 func decryptRepositoryBlobBytesAes256Gcm(data, masterKey, repositoryID []byte) ([]byte, error) {
-	aead, authData, err := initCrypto(masterKey, repositoryID)
-	if err != nil {
-		return nil, errors.Wrap(err, "cannot initialize cipher")
-	}
-
-	data = append([]byte(nil), data...)
-	if len(data) < aead.NonceSize() {
-		return nil, errors.Errorf("invalid encrypted payload, too short")
-	}
-
-	nonce := data[0:aead.NonceSize()]
-	payload := data[aead.NonceSize():]
-
-	plainText, err := aead.Open(payload[:0], nonce, payload, authData)
+	res, err := crypto.DecryptAes256Gcm(data, masterKey, repositoryID)
 	if err != nil {
-		return nil, errors.Errorf("unable to decrypt repository blob, invalid credentials?")
+		return nil, errors.Wrap(err, "Failed to decrypt blob")
 	}
 
-	return plainText, nil
+	return res, nil
 }
 
 func addFormatBlobChecksumAndLength(fb []byte) ([]byte, error) {
diff --git a/repo/open.go b/repo/open.go
index 06b39ad1..4ed52853 100644
--- a/repo/open.go
+++ b/repo/open.go
@@ -14,6 +14,7 @@
 
 	"github.com/kopia/kopia/internal/cache"
 	"github.com/kopia/kopia/internal/cacheprot"
+	"github.com/kopia/kopia/internal/crypto"
 	"github.com/kopia/kopia/internal/feature"
 	"github.com/kopia/kopia/internal/metrics"
 	"github.com/kopia/kopia/internal/retry"
@@ -257,10 +258,10 @@ func openWithConfig(ctx context.Context, st blob.Storage, cliOpts ClientOptions,
 	}
 
 	if fmgr.SupportsPasswordChange() {
-		cacheOpts.HMACSecret = format.DeriveKeyFromMasterKey(fmgr.GetHmacSecret(), fmgr.UniqueID(), localCacheIntegrityPurpose, localCacheIntegrityHMACSecretLength)
+		cacheOpts.HMACSecret = crypto.DeriveKeyFromMasterKey(fmgr.GetHmacSecret(), fmgr.UniqueID(), localCacheIntegrityPurpose, localCacheIntegrityHMACSecretLength)
 	} else {
 		// deriving from ufb.FormatEncryptionKey was actually a bug, that only matters will change when we change the password
-		cacheOpts.HMACSecret = format.DeriveKeyFromMasterKey(fmgr.FormatEncryptionKey(), fmgr.UniqueID(), localCacheIntegrityPurpose, localCacheIntegrityHMACSecretLength)
+		cacheOpts.HMACSecret = crypto.DeriveKeyFromMasterKey(fmgr.FormatEncryptionKey(), fmgr.UniqueID(), localCacheIntegrityPurpose, localCacheIntegrityHMACSecretLength)
 	}
 
 	limits := throttlingLimitsFromConnectionInfo(ctx, st.ConnectionInfo())
diff --git a/repo/repository.go b/repo/repository.go
index 1110625b..e9451eda 100644
--- a/repo/repository.go
+++ b/repo/repository.go
@@ -10,6 +10,7 @@
 	"go.opentelemetry.io/otel"
 
 	"github.com/kopia/kopia/internal/clock"
+	"github.com/kopia/kopia/internal/crypto"
 	"github.com/kopia/kopia/internal/metrics"
 	"github.com/kopia/kopia/repo/blob"
 	"github.com/kopia/kopia/repo/blob/throttling"
@@ -139,13 +140,13 @@ type directRepository struct {
 // DeriveKey derives encryption key of the provided length from the master key.
 func (r *directRepository) DeriveKey(purpose []byte, keyLength int) []byte {
 	if r.cmgr.ContentFormat().SupportsPasswordChange() {
-		return format.DeriveKeyFromMasterKey(r.cmgr.ContentFormat().GetMasterKey(), r.UniqueID(), purpose, keyLength)
+		return crypto.DeriveKeyFromMasterKey(r.cmgr.ContentFormat().GetMasterKey(), r.UniqueID(), purpose, keyLength)
 	}
 
 	// version of kopia <v0.9 had a bug where certain keys were derived directly from
 	// the password and not from the random master key. This made it impossible to change
 	// password.
-	return format.DeriveKeyFromMasterKey(r.fmgr.FormatEncryptionKey(), r.UniqueID(), purpose, keyLength)
+	return crypto.DeriveKeyFromMasterKey(r.fmgr.FormatEncryptionKey(), r.UniqueID(), purpose, keyLength)
 }
 
 // ClientOptions returns client options.


Test patch:
diff --git a/internal/crypto/key_derivation_nontest.go b/internal/crypto/key_derivation_nontest.go
new file mode 100644
index 00000000..7ce781e1
--- /dev/null
+++ b/internal/crypto/key_derivation_nontest.go
@@ -0,0 +1,26 @@
+//go:build !testing
+// +build !testing
+
+package crypto
+
+import (
+	"github.com/pkg/errors"
+	"golang.org/x/crypto/scrypt"
+)
+
+// DefaultKeyDerivationAlgorithm is the key derivation algorithm for new configurations.
+const DefaultKeyDerivationAlgorithm = "scrypt-65536-8-1"
+
+// DeriveKeyFromPassword derives encryption key using the provided password and per-repository unique ID.
+func DeriveKeyFromPassword(password string, salt []byte, algorithm string) ([]byte, error) {
+	const masterKeySize = 32
+
+	switch algorithm {
+	case "scrypt-65536-8-1":
+		//nolint:wrapcheck,gomnd
+		return scrypt.Key([]byte(password), salt, 65536, 8, 1, masterKeySize)
+
+	default:
+		return nil, errors.Errorf("unsupported key algorithm: %v", algorithm)
+	}
+}
diff --git a/repo/format/crypto_key_derivation_testing.go b/internal/crypto/key_derivation_testing.go
similarity index 55%
rename from repo/format/crypto_key_derivation_testing.go
rename to internal/crypto/key_derivation_testing.go
index 47a8b100..04ea1d93 100644
--- a/repo/format/crypto_key_derivation_testing.go
+++ b/internal/crypto/key_derivation_testing.go
@@ -1,7 +1,7 @@
 //go:build testing
 // +build testing
 
-package format
+package crypto
 
 import (
 	"crypto/sha256"
@@ -12,11 +12,11 @@
 // DefaultKeyDerivationAlgorithm is the key derivation algorithm for new configurations.
 const DefaultKeyDerivationAlgorithm = "testing-only-insecure"
 
-// DeriveFormatEncryptionKeyFromPassword derives encryption key using the provided password and per-repository unique ID.
-func (f *KopiaRepositoryJSON) DeriveFormatEncryptionKeyFromPassword(password string) ([]byte, error) {
+// DeriveKeyFromPassword derives encryption key using the provided password and per-repository unique ID.
+func DeriveKeyFromPassword(password string, salt []byte, algorithm string) ([]byte, error) {
 	const masterKeySize = 32
 
-	switch f.KeyDerivationAlgorithm {
+	switch algorithm {
 	case DefaultKeyDerivationAlgorithm:
 		h := sha256.New()
 		if _, err := h.Write([]byte(password)); err != nil {
@@ -26,6 +26,6 @@ func (f *KopiaRepositoryJSON) DeriveFormatEncryptionKeyFromPassword(password str
 		return h.Sum(nil), nil
 
 	default:
-		return nil, errors.Errorf("unsupported key algorithm: %v", f.KeyDerivationAlgorithm)
+		return nil, errors.Errorf("unsupported key algorithm: %v", algorithm)
 	}
 }
diff --git a/repo/format/crypto_key_derivation_nontest.go b/repo/format/crypto_key_derivation_nontest.go
deleted file mode 100644
index 4ddf6bdc..00000000
--- a/repo/format/crypto_key_derivation_nontest.go
+++ /dev/null
@@ -1,26 +0,0 @@
-//go:build !testing
-// +build !testing
-
-package format
-
-import (
-	"github.com/pkg/errors"
-	"golang.org/x/crypto/scrypt"
-)
-
-// DefaultKeyDerivationAlgorithm is the key derivation algorithm for new configurations.
-const DefaultKeyDerivationAlgorithm = "scrypt-65536-8-1"
-
-// DeriveFormatEncryptionKeyFromPassword derives encryption key using the provided password and per-repository unique ID.
-func (f *KopiaRepositoryJSON) DeriveFormatEncryptionKeyFromPassword(password string) ([]byte, error) {
-	const masterKeySize = 32
-
-	switch f.KeyDerivationAlgorithm {
-	case "scrypt-65536-8-1":
-		//nolint:wrapcheck,gomnd
-		return scrypt.Key([]byte(password), f.UniqueID, 65536, 8, 1, masterKeySize)
-
-	default:
-		return nil, errors.Errorf("unsupported key algorithm: %v", f.KeyDerivationAlgorithm)
-	}
-}
diff --git a/repo/repository_test.go b/repo/repository_test.go
index 8fcdb751..60afcfcf 100644
--- a/repo/repository_test.go
+++ b/repo/repository_test.go
@@ -15,6 +15,7 @@
 	"github.com/stretchr/testify/require"
 
 	"github.com/kopia/kopia/internal/cache"
+	"github.com/kopia/kopia/internal/crypto"
 	"github.com/kopia/kopia/internal/epoch"
 	"github.com/kopia/kopia/internal/gather"
 	"github.com/kopia/kopia/internal/metricid"
@@ -866,8 +867,8 @@ func TestDeriveKey(t *testing.T) {
 	formatEncryptionKeyFromPassword, err := j.DeriveFormatEncryptionKeyFromPassword(repotesting.DefaultPasswordForTesting)
 	require.NoError(t, err)
 
-	validV1KeyDerivedFromPassword := format.DeriveKeyFromMasterKey(formatEncryptionKeyFromPassword, uniqueID, testPurpose, testKeyLength)
-	validV2KeyDerivedFromMasterKey := format.DeriveKeyFromMasterKey(masterKey, uniqueID, testPurpose, testKeyLength)
+	validV1KeyDerivedFromPassword := crypto.DeriveKeyFromMasterKey(formatEncryptionKeyFromPassword, uniqueID, testPurpose, testKeyLength)
+	validV2KeyDerivedFromMasterKey := crypto.DeriveKeyFromMasterKey(masterKey, uniqueID, testPurpose, testKeyLength)
 
 	setup := func(v format.Version) repo.DirectRepositoryWriter {
 		_, env := repotesting.NewEnvironment(t, v, repotesting.Options{