-
Notifications
You must be signed in to change notification settings - Fork 198
treat string partition ranges as sensitive data #4501
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
+421
−22
Merged
Changes from 3 commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -6,6 +6,8 @@ import ( | |||||
| "errors" | ||||||
| "fmt" | ||||||
| "log/slog" | ||||||
| "maps" | ||||||
| "slices" | ||||||
| "strconv" | ||||||
| "time" | ||||||
|
|
||||||
|
|
@@ -14,6 +16,7 @@ import ( | |||||
| "github.com/jackc/pgx/v5/pgtype" | ||||||
| "go.temporal.io/sdk/log" | ||||||
| "google.golang.org/protobuf/encoding/protojson" | ||||||
| "google.golang.org/protobuf/proto" | ||||||
|
|
||||||
| "github.com/PeerDB-io/peerdb/flow/alerting" | ||||||
| "github.com/PeerDB-io/peerdb/flow/connectors/utils/monitoring" | ||||||
|
|
@@ -24,8 +27,9 @@ import ( | |||||
| ) | ||||||
|
|
||||||
| const ( | ||||||
| lastSyncStateTableName = "metadata_last_sync_state" | ||||||
| qrepTableName = "metadata_qrep_partitions" | ||||||
| lastSyncStateTableName = "metadata_last_sync_state" | ||||||
| qrepTableName = "metadata_qrep_partitions" | ||||||
| qrepPartitionRangesTableName = "metadata_qrep_offloaded_partition_ranges" | ||||||
| ) | ||||||
|
|
||||||
| type PostgresMetadata struct { | ||||||
|
|
@@ -260,6 +264,15 @@ func (p *PostgresMetadata) FinishQRepPartition( | |||||
| jobName string, | ||||||
| startTime time.Time, | ||||||
| ) error { | ||||||
| if partition.RangeOffloaded { | ||||||
| // The range was offloaded but has since been rehydrated, so drop it before persisting | ||||||
| // to metadata_qrep_partitions. | ||||||
| // Clone rather than mutate in place because this partition is shared with the pull | ||||||
| // goroutine, cloning keeps us from relying on implicit ordering to avoid a write race. | ||||||
| // TODO: drop the sync_partition column entirely; nothing reads it back. | ||||||
| partition = proto.Clone(partition).(*protos.QRepPartition) | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
| partition.Range = nil | ||||||
| } | ||||||
| pbytes, err := protojson.Marshal(partition) | ||||||
| if err != nil { | ||||||
| return fmt.Errorf("failed to marshal partition to json: %w", err) | ||||||
|
|
@@ -286,6 +299,133 @@ func (p *PostgresMetadata) IsQRepPartitionSynced(ctx context.Context, req *proto | |||||
| return exists, nil | ||||||
| } | ||||||
|
|
||||||
| // OffloadPartitionRanges encrypts partition ranges and persists them to the catalog, | ||||||
| // then clears each partition's range in-place and sets RangeOffloaded so the range can | ||||||
| // be rehydrated later. Used in conjunction with RestoreOffloadedPartitionRanges. | ||||||
| func OffloadPartitionRanges( | ||||||
| ctx context.Context, | ||||||
| pool shared.CatalogPool, | ||||||
| parentMirrorName string, | ||||||
| runUUID string, | ||||||
| partitions []*protos.QRepPartition, | ||||||
| ) error { | ||||||
| key, err := internal.PeerDBCurrentEncKey(ctx) | ||||||
| if err != nil { | ||||||
| return fmt.Errorf("failed to load current encryption key: %w", err) | ||||||
| } | ||||||
|
|
||||||
| tx, err := pool.Begin(ctx) | ||||||
| if err != nil { | ||||||
| return fmt.Errorf("failed to begin transaction: %w", err) | ||||||
| } | ||||||
| defer shared.RollbackTx(tx, internal.LoggerFromCtx(ctx)) | ||||||
|
|
||||||
| // delete any orphaned rows from previously failed attempt | ||||||
| if _, err := tx.Exec(ctx, | ||||||
| `DELETE FROM `+qrepPartitionRangesTableName+` WHERE run_uuid=$1`, runUUID, | ||||||
| ); err != nil { | ||||||
| return fmt.Errorf("failed to clear existing partition ranges: %w", err) | ||||||
| } | ||||||
|
|
||||||
| insertRows := make([][]any, 0, len(partitions)) | ||||||
| for _, partition := range partitions { | ||||||
| if partition.Range == nil { | ||||||
| continue | ||||||
| } | ||||||
| payload, err := proto.Marshal(partition.Range) | ||||||
| if err != nil { | ||||||
| return fmt.Errorf("failed to marshal partition range: %w", err) | ||||||
| } | ||||||
| encrypted, err := key.Encrypt(payload) | ||||||
| if err != nil { | ||||||
| return fmt.Errorf("failed to encrypt partition range: %w", err) | ||||||
| } | ||||||
| insertRows = append(insertRows, []any{parentMirrorName, runUUID, partition.PartitionId, key.ID, encrypted}) | ||||||
|
|
||||||
| partition.Range = nil | ||||||
| partition.RangeOffloaded = true | ||||||
| } | ||||||
|
|
||||||
| if _, err := tx.CopyFrom(ctx, | ||||||
| pgx.Identifier{qrepPartitionRangesTableName}, | ||||||
| []string{"parent_mirror_name", "run_uuid", "partition_uuid", "enc_key_id", "range_payload"}, | ||||||
| pgx.CopyFromRows(insertRows), | ||||||
| ); err != nil { | ||||||
| return fmt.Errorf("failed to persist encrypted partition ranges: %w", err) | ||||||
| } | ||||||
|
|
||||||
| if err := tx.Commit(ctx); err != nil { | ||||||
| return fmt.Errorf("failed to commit partition ranges: %w", err) | ||||||
| } | ||||||
| return nil | ||||||
| } | ||||||
|
|
||||||
| // RestoreOffloadedPartitionRanges hydrates offloaded partition ranges in-place with the decrypted | ||||||
| // values fetched from the catalog. Used in conjunction with OffloadPartitionRanges. | ||||||
| func RestoreOffloadedPartitionRanges( | ||||||
| ctx context.Context, | ||||||
| pool shared.CatalogPool, | ||||||
| runUUID string, | ||||||
| partitions []*protos.QRepPartition, | ||||||
| ) error { | ||||||
| partitionByIDs := make(map[string]*protos.QRepPartition, len(partitions)) | ||||||
| for _, partition := range partitions { | ||||||
| if !partition.RangeOffloaded { | ||||||
| continue | ||||||
| } | ||||||
| partitionByIDs[partition.PartitionId] = partition | ||||||
| } | ||||||
| if len(partitionByIDs) == 0 { | ||||||
| return nil | ||||||
| } | ||||||
|
|
||||||
| rows, err := pool.Query(ctx, | ||||||
| `SELECT partition_uuid,enc_key_id,range_payload FROM `+qrepPartitionRangesTableName+` `+ | ||||||
| `WHERE run_uuid=$1 AND partition_uuid=ANY($2)`, | ||||||
| runUUID, slices.Collect(maps.Keys(partitionByIDs)), | ||||||
| ) | ||||||
| if err != nil { | ||||||
| return fmt.Errorf("failed to query partition ranges: %w", err) | ||||||
| } | ||||||
| results, err := pgx.CollectRows(rows, pgx.RowToStructByPos[struct { | ||||||
| PartitionID string | ||||||
| EncKeyID string | ||||||
| Payload []byte | ||||||
| }]) | ||||||
| if err != nil { | ||||||
| return fmt.Errorf("failed to read encrypted partition ranges: %w", err) | ||||||
| } | ||||||
|
|
||||||
| for _, res := range results { | ||||||
| partition, ok := partitionByIDs[res.PartitionID] | ||||||
| if !ok { | ||||||
| return fmt.Errorf("received unexpected partition range for partition %s", res.PartitionID) | ||||||
| } | ||||||
|
|
||||||
| payload, err := internal.Decrypt(ctx, res.EncKeyID, res.Payload) | ||||||
| if err != nil { | ||||||
| return fmt.Errorf("failed to decrypt partition range for %s: %w", res.PartitionID, err) | ||||||
| } | ||||||
|
|
||||||
| var partitionRange protos.PartitionRange | ||||||
| if err := proto.Unmarshal(payload, &partitionRange); err != nil { | ||||||
| return fmt.Errorf("failed to unmarshal partition range for %s: %w", res.PartitionID, err) | ||||||
| } | ||||||
| partition.Range = &partitionRange | ||||||
| delete(partitionByIDs, res.PartitionID) | ||||||
| } | ||||||
|
|
||||||
| if len(partitionByIDs) > 0 { | ||||||
| missing := make([]string, 0, len(partitionByIDs)) | ||||||
| for id := range partitionByIDs { | ||||||
| missing = append(missing, id) | ||||||
| } | ||||||
| return fmt.Errorf("missing encrypted partition ranges for partitions: %v", missing) | ||||||
| } | ||||||
|
|
||||||
| return nil | ||||||
| } | ||||||
|
|
||||||
| func (p *PostgresMetadata) SyncFlowCleanup(ctx context.Context, jobName string) error { | ||||||
| tx, err := p.pool.Begin(ctx) | ||||||
| if err != nil { | ||||||
|
|
@@ -313,5 +453,9 @@ func SyncFlowCleanupInTx(ctx context.Context, tx pgx.Tx, jobName string) error { | |||||
| return err | ||||||
| } | ||||||
|
|
||||||
| if _, err := tx.Exec(ctx, `DELETE FROM `+qrepPartitionRangesTableName+` WHERE parent_mirror_name = $1`, jobName); err != nil { | ||||||
| return err | ||||||
| } | ||||||
|
|
||||||
| return nil | ||||||
| } | ||||||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,109 @@ | ||
| package connmetadata | ||
|
|
||
| import ( | ||
| "testing" | ||
| "time" | ||
|
|
||
| "github.com/google/uuid" | ||
| "github.com/jackc/pgx/v5" | ||
| "github.com/stretchr/testify/require" | ||
| "google.golang.org/protobuf/proto" | ||
| "google.golang.org/protobuf/types/known/timestamppb" | ||
|
|
||
| "github.com/PeerDB-io/peerdb/flow/generated/protos" | ||
| "github.com/PeerDB-io/peerdb/flow/internal" | ||
| ) | ||
|
|
||
| func TestOffloadRestoreSensitivePartitionRanges(t *testing.T) { | ||
| const encKeyID = "test_enc_key" | ||
| t.Setenv("PEERDB_CURRENT_ENC_KEY_ID", encKeyID) | ||
| t.Setenv("PEERDB_ENC_KEYS", `[{"id":"`+encKeyID+`","value":"cGVlcmRiX2NpX3Rlc3RfZW5jX2tleV8zMl9ieXRlcyE="}]`) | ||
|
ilidemi marked this conversation as resolved.
|
||
|
|
||
| ctx := t.Context() | ||
| pool, err := internal.GetCatalogConnectionPoolFromEnv(ctx) | ||
| require.NoError(t, err) | ||
|
|
||
| for _, tc := range []struct { | ||
| name string | ||
| ranges []*protos.PartitionRange | ||
| }{ | ||
| { | ||
| name: "int", | ||
| ranges: []*protos.PartitionRange{ | ||
| {Range: &protos.PartitionRange_IntRange{IntRange: &protos.IntPartitionRange{Start: 1, End: 100}}}, | ||
| {Range: &protos.PartitionRange_IntRange{IntRange: &protos.IntPartitionRange{Start: 101, End: 200}}}, | ||
| }, | ||
| }, | ||
| { | ||
| name: "timestamp", | ||
| ranges: []*protos.PartitionRange{ | ||
| {Range: &protos.PartitionRange_TimestampRange{TimestampRange: &protos.TimestampPartitionRange{ | ||
| Start: timestamppb.New(time.Date(2026, 1, 1, 0, 0, 0, 0, time.UTC)), | ||
| End: timestamppb.New(time.Date(2026, 6, 30, 23, 59, 59, 0, time.UTC)), | ||
| }}}, | ||
| {Range: &protos.PartitionRange_TimestampRange{TimestampRange: &protos.TimestampPartitionRange{ | ||
| Start: timestamppb.New(time.Date(2026, 7, 1, 0, 0, 0, 0, time.UTC)), | ||
| End: timestamppb.New(time.Date(2026, 12, 31, 23, 59, 59, 0, time.UTC)), | ||
| }}}, | ||
| }, | ||
| }, | ||
| { | ||
| name: "string_uuid", | ||
| ranges: []*protos.PartitionRange{ | ||
| {Range: &protos.PartitionRange_StringRange{ | ||
| StringRange: &protos.StringPartitionRange{Start: uuid.NewString(), End: uuid.NewString()}, | ||
| }}, | ||
| {Range: &protos.PartitionRange_StringRange{ | ||
| StringRange: &protos.StringPartitionRange{Start: uuid.NewString(), End: uuid.NewString(), EndInclusive: true}, | ||
| }}, | ||
| }, | ||
| }, | ||
| } { | ||
| t.Run(tc.name, func(t *testing.T) { | ||
| parentMirrorName := "test_offload_restore_partition_ranges_" + tc.name | ||
| runUUID := uuid.NewString() | ||
| t.Cleanup(func() { | ||
| _, _ = pool.Exec(ctx, `DELETE FROM `+qrepPartitionRangesTableName+` WHERE parent_mirror_name=$1`, parentMirrorName) | ||
| }) | ||
|
|
||
| partitions := make([]*protos.QRepPartition, len(tc.ranges)) | ||
| expectedRanges := make([]*protos.PartitionRange, len(tc.ranges)) | ||
| for i, r := range tc.ranges { | ||
| partitions[i] = &protos.QRepPartition{PartitionId: uuid.NewString(), Range: r} | ||
| expectedRanges[i] = proto.Clone(r).(*protos.PartitionRange) | ||
| } | ||
|
|
||
| require.NoError(t, OffloadPartitionRanges(ctx, pool, parentMirrorName, runUUID, partitions)) | ||
| for _, p := range partitions { | ||
| require.Nil(t, p.Range) | ||
| require.True(t, p.RangeOffloaded) | ||
| } | ||
|
|
||
| rows, err := pool.Query(ctx, | ||
| `SELECT enc_key_id, range_payload FROM `+qrepPartitionRangesTableName+` WHERE parent_mirror_name=$1`, | ||
| parentMirrorName) | ||
| require.NoError(t, err) | ||
| results, err := pgx.CollectRows(rows, pgx.RowToStructByPos[struct { | ||
| KeyID string | ||
| Payload []byte | ||
| }]) | ||
| require.NoError(t, err) | ||
| require.Len(t, results, len(partitions)) | ||
| for _, res := range results { | ||
| require.Equal(t, encKeyID, res.KeyID) | ||
| require.NotEmpty(t, res.Payload) | ||
| for _, r := range expectedRanges { | ||
| unencrypted, err := proto.Marshal(r) | ||
| require.NoError(t, err) | ||
| require.NotContains(t, string(res.Payload), string(unencrypted)) | ||
| } | ||
| } | ||
|
|
||
| require.NoError(t, RestoreOffloadedPartitionRanges(ctx, pool, runUUID, partitions)) | ||
| for i, p := range partitions { | ||
| require.Truef(t, proto.Equal(expectedRanges[i], p.Range), | ||
| "partition %d range mismatch after restore: want %v, got %v", i, expectedRanges[i], p.Range) | ||
| } | ||
| }) | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this was missed earlier but we also serialize the entire QRepPartition in metadata_qrep_partitions.
so here we sanitize this as well.