diff --git a/pkg/ccl/backupccl/backup_job.go b/pkg/ccl/backupccl/backup_job.go index 67f9ff1baf91..8750b064e891 100644 --- a/pkg/ccl/backupccl/backup_job.go +++ b/pkg/ccl/backupccl/backup_job.go @@ -800,15 +800,25 @@ func getBackupDetailAndManifest( mem := execCfg.RootMemoryMonitor.MakeBoundAccount() defer mem.Close(ctx) - prevBackups, encryptionOptions, memSize, err := backupinfo.FetchPreviousBackups(ctx, &mem, user, - makeCloudStorage, backupDestination.PrevBackupURIs, *initialDetails.EncryptionOptions, &kmsEnv) + var prevBackups []backuppb.BackupManifest + var baseEncryptionOptions *jobspb.BackupEncryptionOptions + if len(backupDestination.PrevBackupURIs) != 0 { + var err error + baseEncryptionOptions, err = backupencryption.GetEncryptionFromBase(ctx, user, makeCloudStorage, + backupDestination.PrevBackupURIs[0], *initialDetails.EncryptionOptions, &kmsEnv) + if err != nil { + return jobspb.BackupDetails{}, backuppb.BackupManifest{}, err + } - if err != nil { - return jobspb.BackupDetails{}, backuppb.BackupManifest{}, err + var memSize int64 + prevBackups, memSize, err = backupinfo.GetBackupManifests(ctx, &mem, user, + makeCloudStorage, backupDestination.PrevBackupURIs, baseEncryptionOptions, &kmsEnv) + + if err != nil { + return jobspb.BackupDetails{}, backuppb.BackupManifest{}, err + } + defer mem.Shrink(ctx, memSize) } - defer func() { - mem.Shrink(ctx, memSize) - }() if len(prevBackups) > 0 { baseManifest := prevBackups[0] @@ -881,7 +891,7 @@ func getBackupDetailAndManifest( backupDestination.ChosenSubdir, backupDestination.URIsByLocalityKV, prevBackups, - encryptionOptions, + baseEncryptionOptions, &kmsEnv) if err != nil { return jobspb.BackupDetails{}, backuppb.BackupManifest{}, err diff --git a/pkg/ccl/backupccl/backup_planning.go b/pkg/ccl/backupccl/backup_planning.go index 6c0e1bba34c4..7a7ac6a7b16a 100644 --- a/pkg/ccl/backupccl/backup_planning.go +++ b/pkg/ccl/backupccl/backup_planning.go @@ -659,7 +659,7 @@ func backupPlanHook( if err := requireEnterprise(p.ExecCfg(), "encryption"); err != nil { return err } - encryptionParams.RawPassphrae = pw + encryptionParams.RawPassphrase = pw case jobspb.EncryptionMode_KMS: encryptionParams.RawKmsUris = kms if err := requireEnterprise(p.ExecCfg(), "encryption"); err != nil { diff --git a/pkg/ccl/backupccl/backupdest/backup_destination.go b/pkg/ccl/backupccl/backupdest/backup_destination.go index 7e4b43c70d4a..2dc6538e8813 100644 --- a/pkg/ccl/backupccl/backupdest/backup_destination.go +++ b/pkg/ccl/backupccl/backupdest/backup_destination.go @@ -97,6 +97,8 @@ type ResolvedDestination struct { URIsByLocalityKV map[string]string // PrevBackupURIs is the list of full paths for previous backups in the chain. + // This includes the base backup at index 0, and any subsequent incremental + // backups. This field will not be populated when running a full backup. PrevBackupURIs []string } @@ -535,14 +537,14 @@ func ResolveBackupManifests( } ownedMemSize += memSize - var prev []string + var incrementalBackups []string if len(incStores) > 0 { - prev, err = FindPriorBackups(ctx, incStores[0], includeManifest) + incrementalBackups, err = FindPriorBackups(ctx, incStores[0], includeManifest) if err != nil { return nil, nil, nil, 0, err } } - numLayers := len(prev) + 1 + numLayers := len(incrementalBackups) + 1 defaultURIs = make([]string, numLayers) mainBackupManifests = make([]backuppb.BackupManifest, numLayers) @@ -571,14 +573,14 @@ func ResolveBackupManifests( } } - // For each backup layer we construct the default URI. We don't load the - // manifests in this loop since we want to do that concurrently. - for i := range prev { - // prev[i] is the path to the manifest file itself for layer i -- the + // For each incremental backup layer we construct the default URI. We don't + // load the manifests in this loop since we want to do that concurrently. + for i := range incrementalBackups { + // incrementalBackups[i] is the path to the manifest file itself for layer i -- the // dirname piece of that path is the subdirectory in each of the // partitions in which we'll also expect to find a partition manifest. // Recall full inc URI is // - incSubDir := path.Dir(prev[i]) + incSubDir := path.Dir(incrementalBackups[i]) u := *baseURIs[0] // NB: makes a copy to avoid mutating the baseURI. u.Path = backuputils.JoinURLPath(u.Path, incSubDir) defaultURIs[i+1] = u.String() @@ -586,28 +588,22 @@ func ResolveBackupManifests( // Load the default backup manifests for each backup layer, this is done // concurrently. - enc := jobspb.BackupEncryptionOptions{ - Mode: jobspb.EncryptionMode_None, - } - if encryption != nil { - enc = *encryption - } - defaultManifestsForEachLayer, _, memSize, err := backupinfo.FetchPreviousBackups(ctx, mem, user, - mkStore, defaultURIs[1:], enc, kmsEnv) + defaultManifestsForEachLayer, memSize, err := backupinfo.GetBackupManifests(ctx, mem, user, + mkStore, defaultURIs, encryption, kmsEnv) if err != nil { return nil, nil, nil, 0, err } ownedMemSize += memSize - // Iterate over the layers one last time to memoize the loaded manifests and - // read the locality info. + // Iterate over the incremental backups one last time to memoize the loaded + // manifests and read the locality info. // // TODO(adityamaru): Parallelize the loading of the locality descriptors. - for i := range prev { + for i := range incrementalBackups { // The manifest for incremental layer i slots in at i+1 since the full // backup manifest occupies index 0 in `mainBackupManifests`. - mainBackupManifests[i+1] = defaultManifestsForEachLayer[i] - incSubDir := path.Dir(prev[i]) + mainBackupManifests[i+1] = defaultManifestsForEachLayer[i+1] + incSubDir := path.Dir(incrementalBackups[i]) partitionURIs := make([]string, numPartitions) for j := range baseURIs { u := *baseURIs[j] // NB: makes a copy to avoid mutating the baseURI. @@ -616,7 +612,7 @@ func ResolveBackupManifests( } localityInfo[i+1], err = backupinfo.GetLocalityInfo(ctx, incStores, partitionURIs, - defaultManifestsForEachLayer[i], encryption, kmsEnv, incSubDir) + defaultManifestsForEachLayer[i+1], encryption, kmsEnv, incSubDir) if err != nil { return nil, nil, nil, 0, err } diff --git a/pkg/ccl/backupccl/backupencryption/encryption.go b/pkg/ccl/backupccl/backupencryption/encryption.go index a63484eb8398..dc0851aeec62 100644 --- a/pkg/ccl/backupccl/backupencryption/encryption.go +++ b/pkg/ccl/backupccl/backupencryption/encryption.go @@ -237,7 +237,7 @@ func MakeNewEncryptionOptions( encryptionInfo = &jobspb.EncryptionInfo{Salt: salt} encryptionOptions = &jobspb.BackupEncryptionOptions{ Mode: jobspb.EncryptionMode_Passphrase, - Key: storageccl.GenerateKey([]byte(encryptionParams.RawPassphrae), salt), + Key: storageccl.GenerateKey([]byte(encryptionParams.RawPassphrase), salt), } case jobspb.EncryptionMode_KMS: // Generate a 32 byte/256-bit crypto-random number which will serve as @@ -354,7 +354,7 @@ func WriteNewEncryptionInfoToBackup( return cloud.WriteFile(ctx, dest, newEncryptionInfoFile, bytes.NewReader(buf)) } -// GetEncryptionFromBase retrieves the encryption options of a base backup. It +// GetEncryptionFromBase retrieves the encryption options of the base backup. It // is expected that incremental backups use the same encryption options as the // base backups. func GetEncryptionFromBase( @@ -381,7 +381,7 @@ func GetEncryptionFromBase( case jobspb.EncryptionMode_Passphrase: encryptionOptions = &jobspb.BackupEncryptionOptions{ Mode: jobspb.EncryptionMode_Passphrase, - Key: storageccl.GenerateKey([]byte(encryptionParams.RawPassphrae), opts[0].Salt), + Key: storageccl.GenerateKey([]byte(encryptionParams.RawPassphrase), opts[0].Salt), } case jobspb.EncryptionMode_KMS: var defaultKMSInfo *jobspb.BackupEncryptionOptions_KMSInfo diff --git a/pkg/ccl/backupccl/backupinfo/manifest_handling.go b/pkg/ccl/backupccl/backupinfo/manifest_handling.go index 4e01d1976f1e..6d874ca83839 100644 --- a/pkg/ccl/backupccl/backupinfo/manifest_handling.go +++ b/pkg/ccl/backupccl/backupinfo/manifest_handling.go @@ -1298,43 +1298,11 @@ func NewTimestampedCheckpointFileName() string { return fmt.Sprintf("%s-%s", BackupManifestCheckpointName, hex.EncodeToString(buffer)) } -// FetchPreviousBackups takes a list of URIs of previous backups and returns -// their manifest as well as the encryption options of the first backup in the -// chain. -func FetchPreviousBackups( - ctx context.Context, - mem *mon.BoundAccount, - user username.SQLUsername, - makeCloudStorage cloud.ExternalStorageFromURIFactory, - prevBackupURIs []string, - encryptionParams jobspb.BackupEncryptionOptions, - kmsEnv cloud.KMSEnv, -) ([]backuppb.BackupManifest, *jobspb.BackupEncryptionOptions, int64, error) { - ctx, sp := tracing.ChildSpan(ctx, "backupinfo.FetchPreviousBackups") - defer sp.Finish() - - if len(prevBackupURIs) == 0 { - return nil, nil, 0, nil - } - - baseBackup := prevBackupURIs[0] - encryptionOptions, err := backupencryption.GetEncryptionFromBase(ctx, user, makeCloudStorage, baseBackup, - encryptionParams, kmsEnv) - if err != nil { - return nil, nil, 0, err - } - prevBackups, size, err := getBackupManifests(ctx, mem, user, makeCloudStorage, prevBackupURIs, - encryptionOptions, kmsEnv) - if err != nil { - return nil, nil, 0, err - } - - return prevBackups, encryptionOptions, size, nil -} - -// getBackupManifests fetches the backup manifest from a list of backup URIs. -// The manifests are loaded from External Storage in parallel. -func getBackupManifests( +// GetBackupManifests fetches the backup manifest from a list of backup URIs. +// The caller is expected to pass in the fully hydrated encryptionParams +// required to read the manifests. The manifests are loaded from External +// Storage in parallel. +func GetBackupManifests( ctx context.Context, mem *mon.BoundAccount, user username.SQLUsername, @@ -1343,6 +1311,9 @@ func getBackupManifests( encryption *jobspb.BackupEncryptionOptions, kmsEnv cloud.KMSEnv, ) ([]backuppb.BackupManifest, int64, error) { + ctx, sp := tracing.ChildSpan(ctx, "backupinfo.GetBackupManifests") + defer sp.Finish() + manifests := make([]backuppb.BackupManifest, len(backupURIs)) if len(backupURIs) == 0 { return manifests, 0, nil diff --git a/pkg/ccl/backupccl/testdata/backup-restore/encrypted-backups b/pkg/ccl/backupccl/testdata/backup-restore/encrypted-backups new file mode 100644 index 000000000000..edc6889c55bf --- /dev/null +++ b/pkg/ccl/backupccl/testdata/backup-restore/encrypted-backups @@ -0,0 +1,190 @@ +new-server name=s1 +---- + +exec-sql +CREATE DATABASE d; +USE d; +CREATE TABLE foo (i INT PRIMARY KEY, s STRING); +CREATE TABLE baz (i INT PRIMARY KEY, s STRING); +INSERT INTO baz VALUES (1, 'x'),(2,'y'),(3,'z'); +---- + +exec-sql +BACKUP INTO 'nodelocal://1/full' WITH encryption_passphrase='123'; +---- + +exec-sql +BACKUP INTO 'nodelocal://1/full2' WITH encryption_passphrase='456', incremental_location='nodelocal://1/inc'; +---- + +exec-sql +BACKUP INTO 'nodelocal://1/full3' WITH kms='testkms:///cmk?AUTH=implicit'; +---- + +# No passphrase results in an error. +exec-sql expect-error-regex=(file appears encrypted) +SELECT object_name, object_type, backup_type FROM [SHOW BACKUP FROM LATEST IN 'nodelocal://1/full'] +---- +regex matches error + +query-sql +SELECT object_name, object_type, backup_type FROM [SHOW BACKUP FROM LATEST IN 'nodelocal://1/full' +WITH encryption_passphrase='123'] WHERE database_name <> 'system' ORDER BY (backup_type, object_name) +---- +bank table full +baz table full +foo table full +public schema full +public schema full +public schema full +public schema full + +exec-sql expect-error-regex=(file appears encrypted) +SELECT object_name, object_type, backup_type FROM [SHOW BACKUP FROM LATEST IN 'nodelocal://1/full2' +WITH incremental_location='nodelocal://1/inc'] WHERE database_name <> 'system' ORDER BY (backup_type, object_name); +---- +regex matches error + +query-sql +SELECT object_name, object_type, backup_type FROM [SHOW BACKUP FROM LATEST IN 'nodelocal://1/full2' +WITH encryption_passphrase='456', incremental_location='nodelocal://1/inc'] WHERE database_name <> 'system' +ORDER BY (backup_type, object_name); +---- +bank table full +baz table full +foo table full +public schema full +public schema full +public schema full +public schema full + + +# Providing a passphrase when the backup is encrypted by KMS results in an error. +exec-sql expect-error-regex=(failed to decrypt) +SELECT object_name, object_type, backup_type FROM [SHOW BACKUP FROM LATEST IN 'nodelocal://1/full3' +WITH encryption_passphrase='123'] WHERE database_name <> 'system' ORDER BY (backup_type, object_name) +---- +regex matches error + +query-sql +SELECT object_name, object_type, backup_type FROM [SHOW BACKUP FROM LATEST IN 'nodelocal://1/full3' +WITH kms='testkms:///cmk?AUTH=implicit'] WHERE database_name <> 'system' ORDER BY (backup_type, object_name) +---- +bank table full +baz table full +foo table full +public schema full +public schema full +public schema full +public schema full + + +exec-sql +INSERT INTO baz VALUES(4, 'a'), (5, 'b'), (6, 'c'); +---- + +# Add an incremental layer to each backup. + +exec-sql +BACKUP INTO LATEST IN 'nodelocal://1/full' WITH encryption_passphrase='123'; +---- + +exec-sql +BACKUP INTO LATEST IN 'nodelocal://1/full2' WITH encryption_passphrase='456', incremental_location='nodelocal://1/inc'; +---- + +exec-sql +BACKUP INTO LATEST IN 'nodelocal://1/full3' WITH kms='testkms:///cmk?AUTH=implicit'; +---- + +query-sql +SELECT object_name, object_type, backup_type FROM [SHOW BACKUP FROM LATEST IN 'nodelocal://1/full' +WITH encryption_passphrase='123'] WHERE database_name <> 'system' ORDER BY (backup_type, object_name) +---- +bank table full +baz table full +foo table full +public schema full +public schema full +public schema full +public schema full +bank table incremental +baz table incremental +foo table incremental +public schema incremental +public schema incremental +public schema incremental +public schema incremental + +query-sql +SELECT object_name, object_type, backup_type FROM [SHOW BACKUP FROM LATEST IN 'nodelocal://1/full2' +WITH encryption_passphrase='456', incremental_location='nodelocal://1/inc'] WHERE database_name <> 'system' +ORDER BY (backup_type, object_name) +---- +bank table full +baz table full +foo table full +public schema full +public schema full +public schema full +public schema full +bank table incremental +baz table incremental +foo table incremental +public schema incremental +public schema incremental +public schema incremental +public schema incremental + +query-sql +SELECT object_name, object_type, backup_type FROM [SHOW BACKUP FROM LATEST IN 'nodelocal://1/full3' +WITH kms='testkms:///cmk?AUTH=implicit'] WHERE database_name <> 'system' +ORDER BY (backup_type, object_name) +---- +bank table full +baz table full +foo table full +public schema full +public schema full +public schema full +public schema full +bank table incremental +baz table incremental +foo table incremental +public schema incremental +public schema incremental +public schema incremental +public schema incremental + +exec-sql +RESTORE DATABASE d FROM LATEST IN 'nodelocal://1/full' WITH new_db_name='d2', encryption_passphrase='123'; +---- + +exec-sql +RESTORE DATABASE d FROM LATEST IN 'nodelocal://1/full2' WITH new_db_name='d3', encryption_passphrase='456', incremental_location='nodelocal://1/inc'; +---- + +exec-sql +RESTORE DATABASE d FROM LATEST IN 'nodelocal://1/full3' WITH new_db_name='d4', kms='testkms:///cmk?AUTH=implicit'; +---- + +query-sql +USE d2; +SELECT * FROM [SHOW TABLES] ORDER BY table_name; +---- +public baz table root +public foo table root + +query-sql +USE d3; +SELECT * FROM [SHOW TABLES] ORDER BY table_name; +---- +public baz table root +public foo table root + +query-sql +USE d4; +SELECT * FROM [SHOW TABLES] ORDER BY table_name; +---- +public baz table root +public foo table root diff --git a/pkg/jobs/jobspb/jobs.proto b/pkg/jobs/jobspb/jobs.proto index 61283d46aff9..cba1fe344dc9 100644 --- a/pkg/jobs/jobspb/jobs.proto +++ b/pkg/jobs/jobspb/jobs.proto @@ -51,7 +51,7 @@ message BackupEncryptionOptions { // encryption or decryption when mode == KMS. KMSInfo kms_info = 3 [(gogoproto.customname) = "KMSInfo"]; - string raw_passphrae = 4; + string raw_passphrase = 4; repeated string raw_kms_uris = 5; }