bgp / stayrtr

RPKI-To-Router server implementation in Go
BSD 3-Clause "New" or "Revised" License
85 stars 13 forks source link

Ensure session ID uniqueness #85

Closed ties closed 1 year ago

ties commented 1 year ago

After the patch of #84 I realised that while the default is a random session ID, the argument that allows you to set the session ID causes potential indeterminate behaviour.

I can imagine cases where you want to set the session ID for debugging or development - but this is a nice footcannon.

https://www.rfc-editor.org/rfc/rfc8210#section-5.1

Should a cache erroneously reuse a Session ID so that a router does not realize that the session has changed (old Session ID and new Session ID have the same numeric value), the router may become confused as to the content of the cache. The time it takes the router to discover that it is confused will depend on whether the Serial Numbers are also reused. If the Serial Numbers in the old and new sessions are different enough, the cache will respond to the router's Serial Query with a Cache Reset, which will solve the problem. If, however, the Serial Numbers are close, the cache may respond with a Cache Response, which may not be enough to bring the router into sync. In such cases, it's likely but not certain that the router will detect some discrepancy between the state that the cache expects and its own state. For example, the Cache Response may tell the router to drop a record which the router does not hold or may tell the router to add a record which the router already has. In such cases, a router will detect the error and reset the session. The one case in which the router may stay out of sync is when nothing in the Cache Response contradicts any data currently held by the router.

job commented 1 year ago

What do you propose? Change the --help to something like the below?

  -rtr.sessionid int
        Set session ID (Warning: only use when debugging!)
ties commented 1 year ago

I can see how we would need it in debug builds.

Perhaps we can note that it is deprecated and remove it in two releases? It is a foot cannon, I don't like those :)

job commented 1 year ago

I'm OK with just removing the -rtr.sessionid command line option.

On the client side (rtrdump) I actively use the equivalent flag, but on the server side I've not needed it so far.

ties commented 1 year ago

I'm OK with just removing the -rtr.sessionid command line option.

Let's go with that.