diff --git a/firewalldb/sql_migration.go b/firewalldb/sql_migration.go index b6db608a9..1664ea073 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") @@ -128,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 { @@ -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) @@ -187,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) @@ -217,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 @@ -237,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 @@ -270,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 "+ @@ -382,7 +414,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 +429,19 @@ 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 { + // 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) } - p.GroupID = sqldb.SQLInt64(groupID) + p.GroupID = sess.GroupID }) if err != nil { return nil, err @@ -515,12 +551,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 +585,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 +612,39 @@ func collectPrivacyPairs(ctx context.Context, kvStore *bbolt.DB, "%s not found", groupId) } - groupSqlId, err := sqlTx.GetSessionIDByAlias( - ctx, groupId, - ) - if errors.Is(err, sql.ErrNoRows) { - return fmt.Errorf("session with group id %x "+ - "not found in sql db", groupId) - } else if err != nil { - return err + var groupAlias [4]byte + copy(groupAlias[:], groupId) + + sess, ok := sessMap[groupAlias] + if !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 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 { + 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 +845,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 +863,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 +969,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 "+ diff --git a/firewalldb/sql_migration_test.go b/firewalldb/sql_migration_test.go index 8e95097a4..314dcc55a 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, @@ -442,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, @@ -585,6 +609,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 +679,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 @@ -933,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,