From 52dcd20877d5097cce7377cd6a87ab8e2d4dea8b Mon Sep 17 00:00:00 2001 From: Viktor Torstensson Date: Fri, 10 Oct 2025 22:39:31 +0200 Subject: [PATCH 1/3] firewalldb: use alias->session map throughout mig In the upcoming commits, we will update the kv stores and the privacy mapper migration to not migrate entries if their linked session has been deleted. As those checks will need to query the SQL db to see if the session still exists, we move the session alias to session map to not only be used in the actions migration, but throughout the migration when ever we need to query a session by its alias. This is done to avoid multiple queries to the SQL db for the same session alias, to improve the performance of the migration. --- firewalldb/sql_migration.go | 80 ++++++++++++++++++++----------------- 1 file changed, 43 insertions(+), 37 deletions(-) diff --git a/firewalldb/sql_migration.go b/firewalldb/sql_migration.go index b6db608a9..9abec823d 100644 --- a/firewalldb/sql_migration.go +++ b/firewalldb/sql_migration.go @@ -93,18 +93,29 @@ func MigrateFirewallDBToSQL(ctx context.Context, kvStore *bbolt.DB, log.Infof("Starting migration of the rules DB to SQL") - err := migrateKVStoresDBToSQL(ctx, kvStore, sqlTx) + sessions, err := sessionDB.ListSessions(ctx) + if err != nil { + return fmt.Errorf("listing sessions failed: %w", err) + } + + sessionMap, err := mapSessions(sessions) + if err != nil { + return fmt.Errorf("mapping sessions failed: %w", err) + } + + err = migrateKVStoresDBToSQL(ctx, kvStore, sqlTx, sessionMap) if err != nil { return err } - err = migratePrivacyMapperDBToSQL(ctx, kvStore, sqlTx) + err = migratePrivacyMapperDBToSQL(ctx, kvStore, sqlTx, sessionMap) if err != nil { return err } err = migrateActionsToSQL( ctx, kvStore, sqlTx, sessionDB, accountDB, macRootKeyIDs, + sessionMap, ) if err != nil { return err @@ -119,7 +130,7 @@ func MigrateFirewallDBToSQL(ctx context.Context, kvStore *bbolt.DB, // database to the SQL database. The function also asserts that the // migrated values match the original values in the KV store. func migrateKVStoresDBToSQL(ctx context.Context, kvStore *bbolt.DB, - sqlTx SQLQueries) error { + sqlTx SQLQueries, sessMap map[[4]byte]sqlc.Session) error { log.Infof("Starting migration of the KV stores to SQL") @@ -139,7 +150,7 @@ func migrateKVStoresDBToSQL(ctx context.Context, kvStore *bbolt.DB, // 2) Insert all collected key-value pairs into the SQL database. for _, entry := range pairs { - insertedPair, err := insertPair(ctx, sqlTx, entry) + insertedPair, err := insertPair(ctx, sqlTx, sessMap, entry) if err != nil { return fmt.Errorf("inserting kv pair %v failed: %w", entry.key, err) @@ -382,7 +393,7 @@ func collectKVPairs(bkt *bbolt.Bucket, errorOnBuckets, perm bool, // insertPair inserts a single key-value pair into the SQL database. func insertPair(ctx context.Context, tx SQLQueries, - entry *kvEntry) (*sqlKvEntry, error) { + sessMap map[[4]byte]sqlc.Session, entry *kvEntry) (*sqlKvEntry, error) { ruleID, err := tx.GetOrInsertRuleID(ctx, entry.ruleName) if err != nil { @@ -397,15 +408,16 @@ func insertPair(ctx context.Context, tx SQLQueries, } entry.groupAlias.WhenSome(func(alias []byte) { - var groupID int64 - groupID, err = tx.GetSessionIDByAlias(ctx, alias) - if err != nil { - err = fmt.Errorf("getting group id by alias %x "+ - "failed: %w", alias, err) - return + var groupAlias [4]byte + copy(groupAlias[:], alias) + + sess, ok := sessMap[groupAlias] + if !ok { + err = fmt.Errorf("session group %x not found in map", + alias) } - p.GroupID = sqldb.SQLInt64(groupID) + p.GroupID = sess.GroupID }) if err != nil { return nil, err @@ -515,12 +527,12 @@ func verifyBktKeys(bkt *bbolt.Bucket, errorOnKeyValues bool, // from the KV database to the SQL database. The function also asserts that the // migrated values match the original values in the privacy mapper store. func migratePrivacyMapperDBToSQL(ctx context.Context, kvStore *bbolt.DB, - sqlTx SQLQueries) error { + sqlTx SQLQueries, sessMap map[[4]byte]sqlc.Session) error { log.Infof("Starting migration of the privacy mapper store to SQL") // 1) Collect all privacy pairs from the KV store. - privPairs, err := collectPrivacyPairs(ctx, kvStore, sqlTx) + privPairs, err := collectPrivacyPairs(kvStore, sessMap) if err != nil { return fmt.Errorf("error migrating privacy mapper store: %w", err) @@ -549,8 +561,8 @@ func migratePrivacyMapperDBToSQL(ctx context.Context, kvStore *bbolt.DB, } // collectPrivacyPairs collects all privacy pairs from the KV store. -func collectPrivacyPairs(ctx context.Context, kvStore *bbolt.DB, - sqlTx SQLQueries) (privacyPairs, error) { +func collectPrivacyPairs(kvStore *bbolt.DB, + sessMap map[[4]byte]sqlc.Session) (privacyPairs, error) { groupPairs := make(privacyPairs) @@ -576,24 +588,28 @@ func collectPrivacyPairs(ctx context.Context, kvStore *bbolt.DB, "%s not found", groupId) } - groupSqlId, err := sqlTx.GetSessionIDByAlias( - ctx, groupId, - ) - if errors.Is(err, sql.ErrNoRows) { + var groupAlias [4]byte + copy(groupAlias[:], groupId) + + sess, ok := sessMap[groupAlias] + if !ok { return fmt.Errorf("session with group id %x "+ "not found in sql db", groupId) - } else if err != nil { - return err + } + + if !sess.GroupID.Valid { + return fmt.Errorf("session group id for "+ + "session %d is not set ", sess.ID) } groupRealToPseudoPairs, err := collectGroupPairs(gBkt) if err != nil { return fmt.Errorf("processing group bkt "+ "for group id %s (sqlID %d) failed: %w", - groupId, groupSqlId, err) + groupId, sess.GroupID.Int64, err) } - groupPairs[groupSqlId] = groupRealToPseudoPairs + groupPairs[sess.GroupID.Int64] = groupRealToPseudoPairs return nil }) @@ -794,7 +810,8 @@ func validateGroupPairsMigration(ctx context.Context, sqlTx SQLQueries, // values match the original values in the actions store. func migrateActionsToSQL(ctx context.Context, kvStore *bbolt.DB, sqlTx SQLQueries, sessionDB session.SQLQueries, - accountsDB accounts.SQLQueries, macRootKeyIDs [][]byte) error { + accountsDB accounts.SQLQueries, macRootKeyIDs [][]byte, + sessMap map[[4]byte]sqlc.Session) error { log.Infof("Starting migration of the actions store to SQL") @@ -811,16 +828,6 @@ func migrateActionsToSQL(ctx context.Context, kvStore *bbolt.DB, return fmt.Errorf("mapping accounts failed: %w", err) } - sessions, err := sessionDB.ListSessions(ctx) - if err != nil { - return fmt.Errorf("listing sessions failed: %w", err) - } - - sessionMap, err := mapSessions(sessions) - if err != nil { - return fmt.Errorf("mapping sessions failed: %w", err) - } - // Next, as the kvdb actions only have their last 4 bytes set for the // MacaroonRootKeyID field, we'll do a best effort attempt fetch the // full root key ID (all 8 bytes) from lnd when migrating each action. @@ -927,8 +934,7 @@ func migrateActionsToSQL(ctx context.Context, kvStore *bbolt.DB, // migrated. err = migrateActionToSQL( ctx, sqlTx, sessionDB, accountsDB, - acctsMap, sessionMap, action, - macRootKeyID, + acctsMap, sessMap, action, macRootKeyID, ) if err != nil { return fmt.Errorf("migrating action "+ From 4591818bd716fcd2ba7e7d47e34967e470bdd7af Mon Sep 17 00:00:00 2001 From: Viktor Torstensson Date: Fri, 10 Oct 2025 22:38:32 +0200 Subject: [PATCH 2/3] firewalldb: handle deleted sessions in kv stores mig If the user has deleted their session.db file, but kept their rules.db file, there can exist kv entry values that point to a now deleted session ID. Such kv entries should be ignored during the migration, as they are cannot be used anymore. This commit updates the migration to handle this case. --- firewalldb/sql_migration.go | 34 +++++++-- firewalldb/sql_migration_test.go | 126 +++++++++++++++++++++++++++++++ 2 files changed, 155 insertions(+), 5 deletions(-) diff --git a/firewalldb/sql_migration.go b/firewalldb/sql_migration.go index 9abec823d..826ff69c9 100644 --- a/firewalldb/sql_migration.go +++ b/firewalldb/sql_migration.go @@ -139,7 +139,7 @@ func migrateKVStoresDBToSQL(ctx context.Context, kvStore *bbolt.DB, // 1) Collect all key-value pairs from the KV store. err := kvStore.View(func(tx *bbolt.Tx) error { var err error - pairs, err = collectAllPairs(tx) + pairs, err = collectAllPairs(sessMap, tx) return err }) if err != nil { @@ -198,7 +198,9 @@ func migrateKVStoresDBToSQL(ctx context.Context, kvStore *bbolt.DB, // designed to iterate over all buckets and values that exist in the KV store. // That ensures that we find all stores and values that exist in the KV store, // and can be sure that the kv store actually follows the expected structure. -func collectAllPairs(tx *bbolt.Tx) ([]*kvEntry, error) { +func collectAllPairs(sessMap map[[4]byte]sqlc.Session, + tx *bbolt.Tx) ([]*kvEntry, error) { + var entries []*kvEntry for _, perm := range []bool{true, false} { mainBucket, err := getMainBucket(tx, false, perm) @@ -228,7 +230,7 @@ func collectAllPairs(tx *bbolt.Tx) ([]*kvEntry, error) { } pairs, err := collectRulePairs( - ruleBucket, perm, string(rule), + sessMap, ruleBucket, perm, string(rule), ) if err != nil { return err @@ -248,8 +250,8 @@ func collectAllPairs(tx *bbolt.Tx) ([]*kvEntry, error) { // collectRulePairs processes a single rule bucket, which should contain the // global and session-kv-store key buckets. -func collectRulePairs(bkt *bbolt.Bucket, perm bool, rule string) ([]*kvEntry, - error) { +func collectRulePairs(sessMap map[[4]byte]sqlc.Session, bkt *bbolt.Bucket, + perm bool, rule string) ([]*kvEntry, error) { var params []*kvEntry @@ -281,6 +283,25 @@ func collectRulePairs(bkt *bbolt.Bucket, perm bool, rule string) ([]*kvEntry, "under %s bucket", sessKVStoreBucketKey) } + var alias [4]byte + copy(alias[:], groupAlias) + if _, ok := sessMap[alias]; !ok { + // If we can't find the session group in the + // SQL db, that indicates that the session was + // never migrated from KVDB. This likely means + // that the user deleted their session.db file, + // but kept the rules.db file. As the KV entries + // are useless when the session no longer + // exists, we can just skip the migration of the + // KV entries for this group. + log.Warnf("Skipping migration of KV store "+ + "entries for session group %x, as the "+ + "session group was not found", + groupAlias) + + return nil + } + groupBucket := sessBkt.Bucket(groupAlias) if groupBucket == nil { return fmt.Errorf("group bucket for group "+ @@ -413,6 +434,9 @@ func insertPair(ctx context.Context, tx SQLQueries, sess, ok := sessMap[groupAlias] if !ok { + // This should be unreachable, as we check for the + // existence of the session group when collecting + // the kv pairs. err = fmt.Errorf("session group %x not found in map", alias) } diff --git a/firewalldb/sql_migration_test.go b/firewalldb/sql_migration_test.go index 8e95097a4..77a9e7ec7 100644 --- a/firewalldb/sql_migration_test.go +++ b/firewalldb/sql_migration_test.go @@ -418,10 +418,26 @@ func TestFirewallDBMigration(t *testing.T) { name: "session specific kv entries", populateDB: sessionSpecificEntries, }, + { + name: "session specific kv entries deleted session", + populateDB: sessionSpecificEntriesDeletedSession, + }, + { + name: "session specific kv entries deleted and existing sessions", + populateDB: sessionSpecificEntriesDeletedAndExistingSessions, + }, { name: "feature specific kv entries", populateDB: featureSpecificEntries, }, + { + name: "feature specific kv entries deleted session", + populateDB: featureSpecificEntriesDeletedSession, + }, + { + name: "feature specific kv entries deleted and existing sessions", + populateDB: featureSpecificEntriesDeletedAndExistingSessions, + }, { name: "all kv entry combinations", populateDB: allEntryCombinations, @@ -585,6 +601,61 @@ func sessionSpecificEntries(t *testing.T, ctx context.Context, boltDB *BoltDB, ) } +// sessionSpecificEntriesDeletedSession populates the kv store with one session +// specific entry for the local temp store, and one session specific entry for +// the perm local store. Once populated, the session that the entries are linked +// to is deleted. When migrating, we therefore expect that the kv entries linked +// to the deleted session are not migrated. +func sessionSpecificEntriesDeletedSession(t *testing.T, ctx context.Context, + boltDB *BoltDB, sessionStore session.Store, _ accounts.Store, + _ *rootKeyMockStore) *expectedResult { + + groupAlias := getNewSessionAlias(t, ctx, sessionStore) + + _ = insertTempAndPermEntry( + t, ctx, boltDB, testRuleName, groupAlias, fn.None[string](), + testEntryKey, testEntryValue, + ) + + err := sessionStore.DeleteReservedSessions(ctx) + require.NoError(t, err) + + return &expectedResult{ + // Since the session the kx entries were linked to has been + // deleted, we expect no kv entries to be migrated. + kvEntries: []*kvEntry{}, + privPairs: make(privacyPairs), + actions: []*Action{}, + } +} + +// sessionSpecificEntriesDeletedAndExistingSessions populates the kv store with +// two sessions and their corresponding kv entries. +// One of the sessions is deleted prior to the migration though, and therefore +// the migration should only migrate the kv entries linked to the still existing +// session. +func sessionSpecificEntriesDeletedAndExistingSessions(t *testing.T, + ctx context.Context, boltDB *BoltDB, sessionStore session.Store, + _ accounts.Store, _ *rootKeyMockStore) *expectedResult { + + groupAlias1 := getNewSessionAlias(t, ctx, sessionStore) + + _ = insertTempAndPermEntry( + t, ctx, boltDB, testRuleName, groupAlias1, fn.None[string](), + testEntryKey, testEntryValue, + ) + + err := sessionStore.DeleteReservedSessions(ctx) + require.NoError(t, err) + + groupAlias2 := getNewSessionAlias(t, ctx, sessionStore) + + return insertTempAndPermEntry( + t, ctx, boltDB, testRuleName2, groupAlias2, fn.None[string](), + testEntryKey2, testEntryValue, + ) +} + // featureSpecificEntries populates the kv store with one feature specific // entry for the local temp store, and one feature specific entry for the perm // local store. @@ -600,6 +671,61 @@ func featureSpecificEntries(t *testing.T, ctx context.Context, boltDB *BoltDB, ) } +// featureSpecificEntriesDeletedSession populates the kv store with one feature +// specific entry for the local temp store, and one feature specific entry for +// the perm local store. Once populated, the session that the entries are linked +// to is deleted. When migrating, we therefore expect that the kv entries linked +// to the deleted session are not migrated. +func featureSpecificEntriesDeletedSession(t *testing.T, ctx context.Context, + boltDB *BoltDB, sessionStore session.Store, _ accounts.Store, + _ *rootKeyMockStore) *expectedResult { + + groupAlias := getNewSessionAlias(t, ctx, sessionStore) + + _ = insertTempAndPermEntry( + t, ctx, boltDB, testRuleName, groupAlias, + fn.Some(testFeatureName), testEntryKey, testEntryValue, + ) + + err := sessionStore.DeleteReservedSessions(ctx) + require.NoError(t, err) + + return &expectedResult{ + // Since the session the kv entries were linked to has been + // deleted, we expect no kv entries to be migrated. + kvEntries: []*kvEntry{}, + privPairs: make(privacyPairs), + actions: []*Action{}, + } +} + +// featureSpecificEntriesDeletedAndExistingSessions populates the kv store with +// two sessions and their corresponding feature specific kv entries. +// One of the sessions is deleted prior to the migration though, and therefore +// the migration should only migrate the kv entries linked to the still existing +// session. +func featureSpecificEntriesDeletedAndExistingSessions(t *testing.T, + ctx context.Context, boltDB *BoltDB, sessionStore session.Store, + _ accounts.Store, _ *rootKeyMockStore) *expectedResult { + + groupAlias1 := getNewSessionAlias(t, ctx, sessionStore) + + _ = insertTempAndPermEntry( + t, ctx, boltDB, testRuleName, groupAlias1, + fn.Some(testFeatureName), testEntryKey, testEntryValue, + ) + + err := sessionStore.DeleteReservedSessions(ctx) + require.NoError(t, err) + + groupAlias2 := getNewSessionAlias(t, ctx, sessionStore) + + return insertTempAndPermEntry( + t, ctx, boltDB, testRuleName2, groupAlias2, + fn.Some(testFeatureName2), testEntryKey2, testEntryValue, + ) +} + // allEntryCombinations adds all types of different entries at all possible // levels of the kvstores, including multple entries with the same // ruleName, groupAlias and featureName. The test aims to cover all possible From 11cb8080bd0d9fd945764948070f72af84b26f1d Mon Sep 17 00:00:00 2001 From: Viktor Torstensson Date: Fri, 10 Oct 2025 21:21:51 +0200 Subject: [PATCH 3/3] firewalldb: handle deleted sessions in priv pair mig If the user has deleted their session.db file, but kept their rules.db file, there can exist privacy mapper pairs that point to a now deleted session ID. Such pairs should be ignored during the migration, as they are cannot be used anymore. This commit updates the migration to handle this case. --- firewalldb/sql_migration.go | 15 +++++++-- firewalldb/sql_migration_test.go | 55 ++++++++++++++++++++++++++++++++ 2 files changed, 68 insertions(+), 2 deletions(-) diff --git a/firewalldb/sql_migration.go b/firewalldb/sql_migration.go index 826ff69c9..1664ea073 100644 --- a/firewalldb/sql_migration.go +++ b/firewalldb/sql_migration.go @@ -617,8 +617,19 @@ func collectPrivacyPairs(kvStore *bbolt.DB, sess, ok := sessMap[groupAlias] if !ok { - return fmt.Errorf("session with group id %x "+ - "not found in sql db", groupId) + // If we can't find the session group in the SQL + // db, that indicates that the session was never + // migrated from KVDB. This likely means that + // the user deleted their session.db file, but + // kept the rules.db file. As the privacy pairs + // are useless when the session no longer + // exists, we can just skip the migration of the + // privacy pairs for this group. + log.Warnf("Skipping migration of privacy "+ + "pairs for session group %x, as the "+ + "session group was not found", groupId) + + return nil } if !sess.GroupID.Valid { diff --git a/firewalldb/sql_migration_test.go b/firewalldb/sql_migration_test.go index 77a9e7ec7..314dcc55a 100644 --- a/firewalldb/sql_migration_test.go +++ b/firewalldb/sql_migration_test.go @@ -458,6 +458,14 @@ func TestFirewallDBMigration(t *testing.T) { name: "multiple sessions and privacy pairs", populateDB: multipleSessionsAndPrivacyPairs, }, + { + name: "deleted session with privacy pair", + populateDB: deletedSessionWithPrivPair, + }, + { + name: "deleted and existing sessions with privacy pairs", + populateDB: deletedAndExistingSessionsWithPrivPairs, + }, { name: "random privacy pairs", populateDB: randomPrivacyPairs, @@ -1059,6 +1067,53 @@ func oneSessionAndPrivPair(t *testing.T, ctx context.Context, return createPrivacyPairs(t, ctx, boltDB, sessionStore, 1, 1) } +// deletedSessionWithPrivPair inserts 1 session with a linked 1 privacy pair +// into the boltDB, and then deletes the session from the sessions store, to +// simulate the case where a session has been deleted, but the privacy pairs +// still exist. This can happen if the user deletes their session db but not +// their firewall db. +func deletedSessionWithPrivPair(t *testing.T, ctx context.Context, + boltDB *BoltDB, sessionStore session.Store, _ accounts.Store, + _ *rootKeyMockStore) *expectedResult { + + _ = createPrivacyPairs(t, ctx, boltDB, sessionStore, 1, 1) + + // Now we delete the session that the privacy pair was linked to. + err := sessionStore.DeleteReservedSessions(ctx) + require.NoError(t, err) + + return &expectedResult{ + kvEntries: []*kvEntry{}, + // Since the session the privacy pair was linked to has been + // deleted, we expect no privacy pairs to be migrated. + privPairs: make(privacyPairs), + actions: []*Action{}, + } +} + +// deletedAndExistingSessionsWithPrivPairs generates 2 different privacy pairs, +// each linked to a different sessions. However, one of the sessions is deleted +// prior to the migration, to test that only one of the privacy pairs should be +// migrated, while the other one should be ignored since its session has been +// deleted. +func deletedAndExistingSessionsWithPrivPairs(t *testing.T, ctx context.Context, + boltDB *BoltDB, sessionStore session.Store, _ accounts.Store, + _ *rootKeyMockStore) *expectedResult { + + // First generate one privacy pair linked to a session that will be + // deleted. + _ = createPrivacyPairs(t, ctx, boltDB, sessionStore, 1, 1) + + // Delete the linked session. + err := sessionStore.DeleteReservedSessions(ctx) + require.NoError(t, err) + + // Now generate another privacy pair linked to a session that won't be + // deleted prior to the migration. Therefore, this privacy pair should + // be migrated. + return createPrivacyPairs(t, ctx, boltDB, sessionStore, 1, 1) +} + // oneSessionsMultiplePrivPairs inserts 1 session with 10 privacy pairs into the // boltDB. func oneSessionsMultiplePrivPairs(t *testing.T, ctx context.Context,