Subject: Re: [sidr] WGLC for draft-ietf-sidr-rpki-rtr-rfc6810-bis-03

From: Rob Austein <[email-masked]>
Date: Fri, 12 Jun 2015 15:34:37 -0400

[Finally addressing last call comments...sorry for the delay, the
 other part of my day job has been taking up all of my time for months]

At Tue, 17 Mar 2015 00:46:23 -0400, David Mandelberg wrote:
> 
> Section 5 says "Fields with unspecified content MUST be zero on 
> transmission and MAY be ignored on receipt" and Section 5.1 says "Fields 
> shown as zero or reserved MUST be zero.  The value of such a field MUST 
> be ignored on receipt." Are there three separate things (unspecified, 
> zero, and reserved) to which these different rules apply? Or are these 
> all the same thing, and the MAY in the first quote conflicts with the 
> second MUST in the second quote?

Good catch.  There shouldn't be any unspecified fields, and we used
"zero" or "reserved - zero" interchangeably, with most of the
variation having to do with column widths in the PDU diagrams.

The version I will push out as soon as I've addressed all the WGLC
comments does not refer to "unspecified" fields, and uses "zero" to
mark all of the reserved zeroed fields.

The business of requiring reserved fields to be zero on transmission
and ignored on receipt is an idea we stole from RIPv2.  It's probably
not strictly necessary, since every PDU carries an explicit version
number, but it's harmless and makes the specification a bit more
robust the next time somebody finds something we specified badly.

I split the difference between MAY and MUST, yielding "SHOULD be
ignored on receipt."

> The Router Key PDU (section 5.10) uses a fixed-size field for the SKI. 
> What's the plan for algorithm agility, if the size of the SKI changes? 
> if the size of the SKI does not change, but the algorithm does?

I'm gonna weasel on this one and say

a) A SKI is a SKI and it's none of this protocol's business how it's
   calculated, so we don't care about algorithm changes per se here,
   only about the length; and

b) If and when the SKI length changes, we redesign the PDU and bump
   the version number.

The alternative would be to add a SKI-Length field or some such,
either in the reserved ("zero") field just after the flags field, or
just after the PDU length field.

> Section 7 adds the requirement that "A router MUST start each transport 
> session by issuing either a Reset Query or a Serial Query." However, I 
> don't see an additional requirement that the cache can't start sending 
> PDUs (e.g., a Serial Notify) before it receives a Query from the router. 
> Maybe there should be? Our current (version 0) implementation sends a 
> Serial Notify whenever there is a new serial number, even if the router 
> has not yet made any queries. If a version 1 router connects to our 
> version 0 cache and we somehow manage to send a Serial Notify before the 
> router is able to send a Query, what should the router do? Likewise, 
> what should the router do if it receives any version 1 PDU before it has 
> sent a Query?

Good catch.  I think the correct answer would be for the router to
ignore wrong-version Serial Notify PDUs received before the initial
version negotiation has completed.

In the current protocol, Serial Notify is the only unsolicited PDU
type, the only thing the router is required to do with that PDU is
refrain from crashing (solely) because of receiving it, and the only
other thing the router would ordinarily do with it would be issue the
Reset Query or Serial Query it's in the process of issuing anyway as
part of the version negotiation.  So absolving the router from the
requirement to break the session due to the version mismatch doesn't
lose us anything except an unintended self-DoS.

> What happens if version 1 is successfully negotiated, but a later PDU 
> is sent (in either direction) with its version set to 0? Should the 
> receiving side send an Error Report and disconnect? Likewise, what 
> happens if version 0 is successfully negotiated, but a later PDU has a 
> version of 1? Should the receiving side send an Error Report and 
> disconnect? Even if the version 1 PDU is a Query (router to cache) and 
> the cache supports version 1? I have no issue with disallowing 
> re-negotiation of the protocol version, but I think it should be clearly 
> specified that negotiation happens only at the beginning of a transport 
> session.

I was going to say "just send an Error Report and disconnect", but it
looks like we'll need to add another error code to report this
properly ("Unsupported Protocol Version" isn't right here).

I'll add something to the version negotiation discussion stating that
this only happens during startup.

---

From: Rob Austein <[email-masked]>
Date: Fri, 12 Jun 2015 16:03:08 -0400

[More old WGLC comments]

At Tue, 17 Mar 2015 14:29:34 -0400, George, Wes wrote:
> 
> Section 6 Expire interval - this seems out of step with the way that we've
> done most things in RPKI, i.e. what to do with the information provided is
> usually thought of as a matter of local policy. I realize that there is
> increasing risk to keeping the data beyond the expiry time as it grows
> more stale, but there isn't much justification for why this MUST NOT is
> present. I think that perhaps the tradeoff between staleness and
> everything failing back to unknown status is something that the operator
> needs to weigh themselves. We could provide guidance that [MUST/SHOULD]
> NOT keep data from a certain cache beyond expiry time *if* another cache
> is available, but acknowledge that in some cases stale info may be more
> desirable than nothing at all (unless that's not actually true and I'm
> missing something...) This is discussed in the failover scenarios in the
> bottom of section 10 (only keep data if you don't have a full set from
> another cache) but figured I'd mention it in case we think there's some
> tweaks necessary in the section 6 text.

The basic problem is that the router does not have the data necessary
to make an informed decision.  We've stripped all of the timestamps
off of the public data (certificate expiration, CRL and manifest
nextUpdate, etc), and some of the information (cache's own interval
for polling the global RPKI) is not public.

I could (maybe, sort of) see an argument for downgrading MUST NOT here
to SHOULD NOT, but the intent here is to send a (very) strong signal
to the router that once it's past the expiration time, it would be
better (in the opinion of the cache operator) for the router to let
everything go to unknown than to keep using data this stale.

Keep in mind that the security model for this protocol pretty much
assumes that the router and cache are within the same organization.
So it's your cache telling your router what to do, and if you need a
local policy override, sure, go ahead, configure it -- on your cache.

> Also -
> Since we say in section 10 that it is permissible to hold data from
> multiple caches, the doc appears to be missing guidance on what the router
> side of RPKI-router should do in the case where there are multiple caches
> that disagree with one another. It does say MUST NOT distinguish between
> data sources when validating, but that may not cover this scenario. This
> may be as simple as recommending that in the case where data from multiple
> caches is held and specific entries conflict with one another, there
> SHOULD be an odd number of caches so that there is basis for comparison to
> determine which cache is out of sync or providing incorrect info. (i.e.
> Have 3 so that you can go with the 2/3 that agree)

With respect, you may be over-thinking this.  The whole point of the
protocol is to make the router's job simpler, not harder.  Having the
router try to second guess the caches defeats that purpose.  Pick a
cache and use it; if it breaks, pick another one.

There's is a role for some kind of monitoring tool here, operating in
the router protocol role talking to all of your caches to see what's
up, and perhaps even whacking things (cache server, NOC monkey,
whatever works in your shop) as necessary to fix problems, but this
seems like a job for a NOC tool running on a cheap VM, not code in the
slow path on every one of your expensive routers.

---

From: Rob Austein <[email-masked]>
Date: Fri, 12 Jun 2015 19:31:30 -0400

[Last one for today, I hope]

[Skipping points already covered in other messages]

At Thu, 16 Apr 2015 00:37:13 -0400, Richard Hansen wrote:
> 
>   * The protocol is mostly query-response lockstep, but there are no
>     timeouts.  If the cache is taking unreasonably long to respond to a
>     query, what should the router do?  How long is unreasonably long?
>     If timeouts are added, should the router reset its timeout timer
>     for each response PDU (Cache Response, payload, and End of Data),
>     or only after it receives the End of Data PDU?

Um, section 6, "Protocol Timing Parameters"?

>   * Should the cache time out the router if the router doesn't send a
>     Query soon after connecting?

Local decision, but section 9, "Transport", recommends keep-alives
(misnamed and always have been, should be called "make-deads").

>   * The name "Session ID" is misleading.  Section 2 clearly defines it,
>     but unless you pay attention to the definition it's easy to assume
>     that "session" refers to the transport session with the peer.  I
>     would prefer a different name such as "Cache Instance ID", though
>     that name may be insufficient when you consider the protocol
>     upgrade problem brought up by David in
>     <http://article.gmane.org/gmane.ietf.sidr/6896>.  Maybe something
>     like "Data Series ID"?

Sorry, update to an existing protocol, not changing names of protocol
elements now.

>   * In Section 5.1 (fields) under "Session ID", what is the definition
>     of "completely drop the session"?  Do you mean send a fatal error
>     PDU, do a transport-layer disconnect, and let the router reconnect
>     (possibly to a more preferred cache)?  Or do you mean send a Cache
>     Reset (cache->router) or Reset Query (router->cache) and continue
>     the existing transport session?  Or is either reaction acceptable?

Um, "drop" means "drop", not "send a reset PDU".

>   * What is the definition of "payload PDU", mentioned in Sections 5.3,
>     5.5, 8.1, 8.2, and 8.3?  (I assume it means IPv4 Prefix, IPv6
>     Prefix, and Router Key, but it should be explicitly stated.)

OK.

>   * Suppose an IPv4 Prefix was announced in serial 5 and withdrawn in
>     serial 6, and a router does a Serial Query against serial 4.  Is
>     it OK if the cache elides the announce/withdraw pair?

Yes.

>     MUST it?

Not specified.  Design assumption is that the cache will elide, since
the point of the protocol is to offload work from the router to the
cache, thus making the router follow a pointless series of actions
would be counterproductive, even if more convenient for the cache.

Speaking as an implementer, I can tell you that I just (pre)compute
the straight difference from one serial directly to the other, ie, my
implementation always elides.

>     If it doesn't, it seems like the cache MUST send the payload
>     PDUs in serial number order, and the router MUST process the
>     payload PDUs in serial number order (which implies that the
>     transport MUST provide in-order delivery of the PDUs because the
>     router has no idea which PDUs correspond to which serial
>     number).

All the specified transports are reliable in any case.

I would be OK with making this a MUST.

>   * Section 5.1 (fields) says that the serial number is the serial
>     number of the cache, but Section 5.3 (Serial Query) talks about
>     serial numbers as if they are properties of a PDU.  Perhaps 5.3
>     should be worded like:
> 
>         The router sends a Serial Query to ask the cache for the
>         announcements and withdrawals that have occurred since the
>         Serial Number in the Serial Query.
> 
>     Section 5.5 (Cache Response) has similarly problematic wording.

OK.  I think the odd wording in the Serial Query and Cache Response
sections was a holdover from some odd assumptions that somebody years
ago made about internal data structures on either the cache or the
router.  I agree that it was not very clear.

> 
>   * The two sentences in 5.3 (Serial Query) paragraph 2 seem to
>     contradict each other in the case where there are no (net?)
>     changes:  The first sentence suggests that the cache sends a Cache
>     Response (maybe followed by something?), while the second suggests
>     that it only sends an End of Data (no Cache Response).  I think the
>     intention is for the cache to send a Cache Response immediately
>     followed by an End of Data.  Is that correct?

Yes.  Tweaked wording, maybe it's better.

>   * I don't think the set of valid responses to a Query (Reset or
>     Serial) is clearly specified.  I think the intention is for these
>     to be the only valid responses:
> 
>       - Reset Query:
>           * Cache Response followed by 0 or more payload PDUs followed
>             by End of Data
>           * Error Report
>       - Serial Query:
>           * Cache Response followed by 0 or more payload PDUs followed
>             by End of Data
>           * Error Report
>           * Cache Reset
> 
>     Is this correct?

Yes.  Seems clear enough to me. :)

>   * Is there a particular reason for omitting a payload PDU count field
>     from the Cache Response PDU?  If one was present, the router could
>     pre-allocate an appropriate amount of memory to handle the payload
>     PDUs (and perform additional sanity checks).
> 
>     I guess a PDU count field would prevent an implementation from
>     opportunistically sending additional PDUs if there happened to be a
>     serial number bump during the middle of a Cache Response.
>     (Instead, the cache would have to follow the End of Data PDU with a
>     Serial Notify, which is almost as good.)

Existing protocol, you're proposing a new change, so out of scope
after WGLC unless the WG really wants this.  Customers for this would
primarily be the router folks, and I haven't heard them asking for it.

>   * Section 5.6 (IPv4 Prefix) mentions duplicates, but are redundant
>     entries OK?  Examples:
>       - {65536,192.0.2.0/24-26} and {65536,192.0.2.0/26-26} (the latter
>         is redundant)
>       - {65536,192.0.2.0/24-26} and {65536,192.0.2.0/24-25} (the latter
>         is redundant)

We just present the data we find in the ROAs, sir.  We de-dup to keep
the protocol semantics simple, but otherwise we just send what we got.

>   * Section 5.11 (Error Report) says that Error Reports are only sent
>     as responses to other PDUs.  Why the restriction?  This prevents a
>     side from raising a timeout error, and it prevents the cache from
>     raising an internal error if a problem is detected when it's time
>     to send a Serial Notify.

From which we infer that you've not had the dubious pleasure of
operating a protocol that permits error message duels.

Don't go there.

>   * If error reports are only sent as responses to other PDUs, how is
>     it possible for an Error Report to not be associated with the PDU
>     to which it is responding?  (Section 5.11 paragraph 4)

You're not allowed to send unsolicited Error Report PDUs, but it is
not necessarily the case that the error you're reporting has much of
anything to do with the request.

Router: "Please tell me about new stuff since serial xyz."

Cache:  "I'm sorry, my CPU caught fire half an hour ago and this is
         the first chance I've had to tell you, bye."

>   * For version negotiation, what is supposed to happen if the router
>     starts with a PDU with version > 1?  There is an Unsupported
>     Protocol Version error type, but nothing requires that to be sent.

Good catch.

>   * Suppose a router connects and issues a v0 Query.  If the cache
>     doesn't support protocol v0, Section 7 says it MUST either
>     downgrade or disconnect.  Can it issue an Error Report before
>     disconnecting?  I would prefer it if the server MUST issue an
>     Unsupported Protocol Version Error Report before disconnecting.

And what protocol version should that Error Report PDU be, given that
the router asked for v0 and you just said the cache doesn't speak v0?

>   * The second-to-last paragraph of Section 10 talks about deleting
>     data from a cache when it has been unable to refresh from that
>     cache for twice the polling period (by default).  Why not have the
>     time to delete equal the Expire Interval as specified in Section 6?

Good catch.

Thanks!

---

From: David Mandelberg <[email-masked]>
Date: Mon, 22 Jun 2015 23:58:11 -0400

>    only about the length; and

Agreed.

> just after the PDU length field.

If it wouldn't hold anything up too much, I'd prefer adding a 
SKI-Length field now. On the other hand, if adding that field would 
significantly delay this document, then I won't push it. It just seems 
strange to me that any future decision to change the length of the SKI 
will also require changing the the rpki-rtr version.

-- 
David Eric Mandelberg / dseomn
http://david.mandelberg.org/