diff --git a/cmd/icingadb-migrate/convert.go b/cmd/icingadb-migrate/convert.go index 0a875e5c1..faa167139 100644 --- a/cmd/icingadb-migrate/convert.go +++ b/cmd/icingadb-migrate/convert.go @@ -11,6 +11,7 @@ import ( "github.com/icinga/icingadb/pkg/icingadb/v1/history" "github.com/jmoiron/sqlx" "github.com/pkg/errors" + "math" "strconv" "strings" "time" @@ -282,6 +283,14 @@ func convertDowntimeRows( cancelTime = actualEnd } + // The IDO duration is of type bigint, representing seconds, while Icinga DB's FlexibleDuration is an unsigned + // bigint, representing milliseconds. In theory, there should be no negative value in the IDO and multiplied + // with factor 1,000 should not overflow anything. In theory, at least. To make sure, invalid values are capped. + var flexibleDuration uint64 + if durationSec := row.Duration; durationSec >= 0 && durationSec < math.MaxUint64/1_000 { + flexibleDuration = uint64(durationSec) * 1_000 + } + downtimeHistory = append(downtimeHistory, &history.DowntimeHistory{ DowntimeHistoryEntity: history.DowntimeHistoryEntity{DowntimeId: id}, HistoryTableMeta: history.HistoryTableMeta{ @@ -299,7 +308,7 @@ func convertDowntimeRows( Author: row.AuthorName, Comment: row.CommentData, IsFlexible: types.Bool{Bool: row.IsFixed == 0, Valid: true}, - FlexibleDuration: uint64(row.Duration) * 1000, + FlexibleDuration: flexibleDuration, ScheduledStartTime: scheduledStart, ScheduledEndTime: scheduledEnd, StartTime: startTime, diff --git a/pkg/icingadb/cleanup.go b/pkg/icingadb/cleanup.go index 754e5a33d..ca38e3511 100644 --- a/pkg/icingadb/cleanup.go +++ b/pkg/icingadb/cleanup.go @@ -32,7 +32,7 @@ func (stmt *CleanupStmt) CleanupOlderThan( defer db.Log(ctx, q, &counter).Stop() for { - var rowsDeleted int64 + var rowsDeleted uint64 err := retry.WithBackoff( ctx, @@ -45,7 +45,10 @@ func (stmt *CleanupStmt) CleanupOlderThan( return database.CantPerformQuery(err, q) } - rowsDeleted, err = rs.RowsAffected() + i, err := rs.RowsAffected() + if err == nil && i >= 0 { + rowsDeleted = uint64(i) + } return err }, @@ -57,7 +60,7 @@ func (stmt *CleanupStmt) CleanupOlderThan( return 0, err } - counter.Add(uint64(rowsDeleted)) + counter.Add(rowsDeleted) for _, onSuccess := range onSuccess { if err := onSuccess(ctx, make([]struct{}, rowsDeleted)); err != nil { @@ -65,7 +68,7 @@ func (stmt *CleanupStmt) CleanupOlderThan( } } - if rowsDeleted < int64(count) { + if rowsDeleted < count { break } } diff --git a/pkg/icingadb/delta_test.go b/pkg/icingadb/delta_test.go index 5067ecd3a..faacad14b 100644 --- a/pkg/icingadb/delta_test.go +++ b/pkg/icingadb/delta_test.go @@ -208,14 +208,14 @@ func testDeltaVerifyResult(t *testing.T, name string, expected map[uint64]uint64 } func BenchmarkDelta(b *testing.B) { - for n := 1 << 10; n <= 1<<20; n <<= 1 { - b.Run(strconv.Itoa(n), func(b *testing.B) { + for n := uint64(1 << 10); n <= 1<<20; n <<= 1 { + b.Run(strconv.FormatUint(n, 10), func(b *testing.B) { benchmarkDelta(b, n) }) } } -func benchmarkDelta(b *testing.B, numEntities int) { +func benchmarkDelta(b *testing.B, numEntities uint64) { chActual := make([]chan database.Entity, b.N) chDesired := make([]chan database.Entity, b.N) for i := 0; i < b.N; i++ { @@ -231,20 +231,20 @@ func benchmarkDelta(b *testing.B, numEntities int) { binary.BigEndian.PutUint64(e.PropertiesChecksum, checksum) return e } - for i := 0; i < numEntities; i++ { + for i := uint64(0); i < numEntities; i++ { // each iteration writes exactly one entity to each channel var eActual, eDesired database.Entity switch i % 3 { case 0: // distinct IDs - eActual = makeEndpoint(1, uint64(i), uint64(i)) - eDesired = makeEndpoint(2, uint64(i), uint64(i)) + eActual = makeEndpoint(1, i, i) + eDesired = makeEndpoint(2, i, i) case 1: // same ID, same checksum - e := makeEndpoint(3, uint64(i), uint64(i)) + e := makeEndpoint(3, i, i) eActual = e eDesired = e case 2: // same ID, different checksum - eActual = makeEndpoint(4, uint64(i), uint64(i)) - eDesired = makeEndpoint(4, uint64(i), uint64(i+1)) + eActual = makeEndpoint(4, i, i) + eDesired = makeEndpoint(4, i, i+1) } for _, ch := range chActual { ch <- eActual diff --git a/pkg/icingadb/history/retention.go b/pkg/icingadb/history/retention.go index f10048cf2..68c1afc88 100644 --- a/pkg/icingadb/history/retention.go +++ b/pkg/icingadb/history/retention.go @@ -11,6 +11,7 @@ import ( "github.com/icinga/icingadb/pkg/icingaredis/telemetry" "github.com/pkg/errors" "go.uber.org/zap" + "math" "time" ) @@ -173,6 +174,25 @@ func (r *Retention) Start(ctx context.Context) error { continue } + // Ensure that the time cannot overflow. + // + // First, make sure that days do not exceed max int, as otherwise casts will overflow and later subtractions + // may overflow as well. As the database schema uses an unsigned timestamp, it is ensured that the relative + // offset, later called olderThan, is not before Unix epoch. The final check verifies if Time.AddDate may have + // overflown, resulting in a time after the starting point. + // + // For the periodic.Start function below, time marches forward while the value of days is static. Subtracting + // days from an increasing timestamp will also not result in a negative or overflowing value. + if days > math.MaxInt { + r.logger.Errorf("Skipping history retention for category %s as %d days will overflow", stmt.Category, days) + continue + } + // #nosec G115 -- days in [1, math.MaxInt] => -days >= -math.MaxInt => -days > math.MinInt and cannot overflow + if date := time.Now().AddDate(0, 0, -int(days)); date.Before(time.Unix(0, 0)) || !time.Now().After(date) { + r.logger.Errorf("Skipping history retention for category %s as %d days will overflow", stmt.Category, days) + continue + } + r.logger.Debugw( fmt.Sprintf("Starting history retention for category %s", stmt.Category), zap.Uint64("count", r.count), @@ -182,7 +202,7 @@ func (r *Retention) Start(ctx context.Context) error { stmt := stmt periodic.Start(ctx, r.interval, func(tick periodic.Tick) { - olderThan := tick.Time.AddDate(0, 0, -int(days)) + olderThan := tick.Time.AddDate(0, 0, -int(days)) // #nosec G115 -- int overflow checked above r.logger.Debugf("Cleaning up historical data for category %s from table %s older than %s", stmt.Category, stmt.Table, olderThan) diff --git a/pkg/icingadb/types/notification_type.go b/pkg/icingadb/types/notification_type.go index ce88c2efb..24887810e 100644 --- a/pkg/icingadb/types/notification_type.go +++ b/pkg/icingadb/types/notification_type.go @@ -3,7 +3,6 @@ package types import ( "database/sql/driver" "encoding" - "github.com/icinga/icinga-go-library/types" "github.com/pkg/errors" "strconv" ) @@ -15,17 +14,12 @@ type NotificationType uint16 func (nt *NotificationType) UnmarshalText(text []byte) error { s := string(text) - i, err := strconv.ParseUint(s, 10, 64) + i, err := strconv.ParseUint(s, 10, 16) if err != nil { - return types.CantParseUint64(err, s) + return errors.Wrapf(err, "can't parse %q into uint16", s) } n := NotificationType(i) - if uint64(n) != i { - // Truncated due to above cast, obviously too high - return badNotificationType(s) - } - if _, ok := notificationTypes[n]; !ok { return badNotificationType(s) }