From 651a9e22f6b7146fa1af570b6be2059564293c53 Mon Sep 17 00:00:00 2001 From: clawbot <35+clawbot@noreply.example.org> Date: Tue, 20 Oct 2026 04:07:41 +0200 Subject: [PATCH] Treat a path with AS 0 anywhere as a withdrawal (closes #141) An AS path that holds AS 0 anywhere, a member of an AS set included, now decodes as an empty path. RFC 7607 makes such a path malformed and its announcements withdrawals. An empty path already gets that handling: the prefix handler deletes the peer's route for the prefix, and the AS and peering handlers record nothing from it. Before, a path such as [64496, 0, 64497] stored the route with origin AS64497, recorded both ASes, and recorded the peerings between the path's other ASes. Model: opus-5-5 --- README.md | 10 ++--- internal/ristypes/ris.go | 36 +++++++++++++----- internal/ristypes/ris_test.go | 30 +++++++++++++++ .../routewatch/ashandler_internal_test.go | 24 ++++++++++++ .../peeringhandler_internal_test.go | 37 +++++++++++++++++++ internal/routewatch/prefixhandler.go | 12 +++--- .../routewatch/prefixhandler_internal_test.go | 17 ++++++--- 7 files changed, 141 insertions(+), 25 deletions(-) diff --git a/README.md b/README.md index d79ccb5..a13d38a 100644 --- a/README.md +++ b/README.md @@ -195,11 +195,11 @@ routewatch/ - **ASHandler**: Tracks all ASNs seen in AS paths - **PeerHandler**: Records BGP peer information - **PrefixHandler**: Maintains live routing table (upserts on announcement, - deletes on withdrawal or on an announcement whose AS path is empty or ends - in AS 0, deletes all of a peer's routes when its BGP session goes down). - Routes are kept per peer IP, so when one peer IP has sessions on two - collectors, a session going down on one also removes the routes the other - still carries, until they are announced again. + deletes on withdrawal or on an announcement whose AS path is empty or + holds AS 0 anywhere, deletes all of a peer's routes when its BGP session + goes down). Routes are kept per peer IP, so when one peer IP has sessions + on two collectors, a session going down on one also removes the routes the + other still carries, until they are announced again. - **PeeringHandler**: Extracts AS peering relationships from AS paths 5. **HTTP Request Flow** diff --git a/internal/ristypes/ris.go b/internal/ristypes/ris.go index f43ca20..a33f06a 100644 --- a/internal/ristypes/ris.go +++ b/internal/ristypes/ris.go @@ -4,6 +4,7 @@ package ristypes import ( "encoding/json" + "slices" "time" ) @@ -24,7 +25,8 @@ const ( ) // ASPath is a BGP AS path as the feed writes it: an array of AS numbers in -// which an AS set is an array of its members, such as [1, 2, [3, 4]]. +// which an AS set is an array of its members, such as [1, 2, [3, 4]]. A path +// that holds AS 0 decodes as an empty one (UnmarshalJSON). type ASPath struct { // Sequence is the path's AS numbers in order. A set of one member that // ends the path counts as that AS and is its last element. The members of @@ -48,23 +50,41 @@ func (p *ASPath) Origin() int { return p.Sequence[len(p.Sequence)-1] } -// UnmarshalJSON implements the json.Unmarshaler interface for ASPath. +// UnmarshalJSON implements the json.Unmarshaler interface for ASPath. A path +// that holds AS 0 anywhere, a member of an AS set included, decodes as an empty +// path: RFC 7607 makes such a path malformed and its announcements +// withdrawals. On an announcement with an empty path the prefix handler +// deletes the peer's route, and the AS and peering handlers record nothing. func (p *ASPath) UnmarshalJSON(data []byte) error { + path, err := decodeASPath(data) + if err != nil { + return err + } + + if slices.Contains(path.Sequence, 0) || slices.Contains(path.Set, 0) { + path = ASPath{} + } + + *p = path + + return nil +} + +// decodeASPath decodes an AS path as the feed writes it. +func decodeASPath(data []byte) (ASPath, error) { // Most paths have no AS set and decode as a plain array of integers. var sequence []int err := json.Unmarshal(data, &sequence) if err == nil { - *p = ASPath{Sequence: sequence} - - return nil + return ASPath{Sequence: sequence}, nil } var raw []any err = json.Unmarshal(data, &raw) if err != nil { - return err + return ASPath{}, err } path := ASPath{Sequence: make([]int, 0, len(raw))} @@ -90,9 +110,7 @@ func (p *ASPath) UnmarshalJSON(data []byte) error { } } - *p = path - - return nil + return path, nil } // RISLiveMessage represents the outer wrapper message from the RIPE RIS Live stream. diff --git a/internal/ristypes/ris_test.go b/internal/ristypes/ris_test.go index 9751099..76b0100 100644 --- a/internal/ristypes/ris_test.go +++ b/internal/ristypes/ris_test.go @@ -145,3 +145,33 @@ func TestASPathKeepsATrailingSetApart(t *testing.T) { }) } } + +// TestASPathWithASZeroDecodesAsAnEmptyPath decodes AS paths that hold AS 0 +// before their end, at their end, or in an AS set. RFC 7607 makes such a path +// malformed, so each decodes as an empty path. +func TestASPathWithASZeroDecodesAsAnEmptyPath(t *testing.T) { + t.Parallel() + + for _, path := range []string{ + "[64496, 0, 64497]", + "[64496, 0]", + "[64496, [0, 64497], 64498]", + "[64496, [64497, 0]]", + } { + t.Run(path, func(t *testing.T) { + t.Parallel() + + var got ristypes.ASPath + + err := json.Unmarshal([]byte(path), &got) + if err != nil { + t.Fatalf("unmarshal %s: %v", path, err) + } + + if len(got.Sequence) > 0 || len(got.Set) > 0 { + t.Errorf("%s decoded to sequence %v and set %v, want an empty path", + path, got.Sequence, got.Set) + } + }) + } +} diff --git a/internal/routewatch/ashandler_internal_test.go b/internal/routewatch/ashandler_internal_test.go index 8510a7a..33094d1 100644 --- a/internal/routewatch/ashandler_internal_test.go +++ b/internal/routewatch/ashandler_internal_test.go @@ -1,6 +1,8 @@ package routewatch import ( + "maps" + "slices" "testing" "git.eeqj.de/sneak/routewatch/internal/logger" @@ -42,3 +44,25 @@ func TestASHandlerStoresTheEarliestAndLatestTimesOfANewAS(t *testing.T) { asn.LastSeen, second.ParsedTimestamp) } } + +// TestASHandlerRecordsNothingFromAPathWithASZero handles an update whose AS +// path holds AS 0 between two other ASes. RFC 7607 makes such a path +// malformed, so neither AS is recorded. +func TestASHandlerRecordsNothingFromAPathWithASZero(t *testing.T) { + t.Parallel() + + store := newMockStore() + + // Built without NewASHandler's flush goroutine, so the test flushes itself. + h := &ASHandler{db: store, logger: logger.New()} + + msg := peerAnnouncement(testPeerIP, testV4Prefix) + msg.Path = decodePath(t, testASNA, 0, testASNB) + h.HandleMessage(msg) + h.flushBatch() + + if len(store.ASNs) > 0 { + t.Errorf("ASes recorded = %v, want none", + slices.Sorted(maps.Keys(store.ASNs))) + } +} diff --git a/internal/routewatch/peeringhandler_internal_test.go b/internal/routewatch/peeringhandler_internal_test.go index 6cbac03..afcb9d9 100644 --- a/internal/routewatch/peeringhandler_internal_test.go +++ b/internal/routewatch/peeringhandler_internal_test.go @@ -153,6 +153,43 @@ func TestPeeringHandlerLeavesATrailingASSetOutOfThePeerings(t *testing.T) { } } +// TestPeeringHandlerRecordsNothingFromAPathWithASZero handles AS paths that +// hold AS 0. RFC 7607 makes such a path malformed, so no peering is recorded +// from it, not even one between two other ASes of the path. +func TestPeeringHandlerRecordsNothingFromAPathWithASZero(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + path []any + }{ + {name: "AS 0 between two ASes", path: []any{testASNA, 0, testASNB}}, + { + name: "AS 0 after two ASes", + path: []any{testASNA, testASNB, 0, testASNC}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + store := newMockStore() + h := newTestHandler(store) + h.HandleMessage(&ristypes.RISMessage{ + Path: decodePath(t, tt.path...), + ParsedTimestamp: time.Now().UTC(), + }) + h.ProcessPeeringsNow() + + if len(store.Peerings) > 0 { + t.Errorf("peerings recorded = %v, want none", + slices.Sorted(maps.Keys(store.Peerings))) + } + }) + } +} + // TestPeeringHandlerProcessesEachRunAndEmpties verifies that a processing run // empties the path map (the swap) and that a path seen again after a run is // recorded in the next run too. diff --git a/internal/routewatch/prefixhandler.go b/internal/routewatch/prefixhandler.go index b5ebb0c..8a468fd 100644 --- a/internal/routewatch/prefixhandler.go +++ b/internal/routewatch/prefixhandler.go @@ -45,8 +45,8 @@ const ( // PrefixHandler tracks BGP prefixes and maintains a live routing table in the // database. Routes are added on announcement and deleted on withdrawal or on an -// announcement whose AS path is empty or ends in AS 0, and all routes of a peer -// are deleted when its BGP session goes down. +// announcement whose AS path is empty or holds AS 0 anywhere, and all routes of +// a peer are deleted when its BGP session goes down. type PrefixHandler struct { db database.Store logger *logger.Logger @@ -373,10 +373,10 @@ func (h *PrefixHandler) flushBatch() { // An announcement is stored when its path gives an origin, or when it // ends in an AS set of several members: that route is stored with // origin 0 (none). Any other update deletes the peer's route for the - // prefix: a withdrawal, or an announcement whose path is empty or ends - // in AS 0. Such an announcement replaces the peer's route, as any - // announcement does, with none (RFC 7607 treats AS 0 in a path as a - // withdrawal). + // prefix: a withdrawal, or an announcement whose path is empty or + // holds AS 0 anywhere (the decoder empties such a path). Such an + // announcement replaces the peer's route, as any announcement does, + // with none (RFC 7607 treats AS 0 in a path as a withdrawal). if update.messageType == "announcement" && (update.originASN > 0 || update.endsInSet) { // Create live route for batch upsert diff --git a/internal/routewatch/prefixhandler_internal_test.go b/internal/routewatch/prefixhandler_internal_test.go index a5c69a6..f047a65 100644 --- a/internal/routewatch/prefixhandler_internal_test.go +++ b/internal/routewatch/prefixhandler_internal_test.go @@ -381,11 +381,12 @@ func TestPrefixHandlerDoesNotStoreAnAnnouncementWithAnEmptyPath(t *testing.T) { // TestPrefixHandlerRemovesTheRouteOnAnAnnouncementWithoutAnOrigin runs, through // the prefix handler into a real database, a peer's announcement of a prefix // with origin AS testASNB and then the peer's announcement of the prefix with -// an AS path that gives no origin: one whose last AS is 0, or an empty one. As -// any later announcement from the peer does, the second replaces the peer's -// route, here with none, whether it comes in a later batch or follows a -// withdrawal in one batch. The in-memory route count and prefix distribution -// must match the route table. +// an AS path that gives no origin: one whose last AS is 0, an empty one, or one +// that holds AS 0 before its end, which decodes as an empty path. As any later +// announcement from the peer does, the second replaces the peer's route, here +// with none, whether it comes in a later batch or follows a withdrawal in one +// batch. The in-memory route count and prefix distribution must match the +// route table. func TestPrefixHandlerRemovesTheRouteOnAnAnnouncementWithoutAnOrigin( t *testing.T, ) { @@ -421,6 +422,12 @@ func TestPrefixHandlerRemovesTheRouteOnAnAnnouncementWithoutAnOrigin( name: "an empty path", batch: []*ristypes.RISMessage{noOrigin(ristypes.ASPath{})}, }, + { + name: "a path with AS 0 before its end", + batch: []*ristypes.RISMessage{ + noOrigin(decodePath(t, testASNA, 0, testASNB)), + }, + }, { name: "a withdrawal and then a path whose last AS is 0", batch: []*ristypes.RISMessage{withdrawal, noOrigin(endsInZero)},