Hi Alvaro, in-line
> On 07 Jan 2017, at 00:04, Alvaro Retana (aretana) <aret...@cisco.com> wrote: > > On 12/28/16, 11:43 AM, "Tim Bruijnzeels" <t...@ripe.net > <mailto:t...@ripe.net>> wrote: > > Tim: > > Happy New Year!! Thank you, you (all) too!! > > > | Thank you for your in-depth review :) It really helps to clarify the > document. Replies in-line. > | > | I attached an updated document, but.. I did not discuss this with > co-authors yet. So, I invite any of them to > | disagree or suggest changes to the things below. > > Thanks for that. I have some comments below – I want us to discuss the > Update issue (M16) so we can start the IETF Last Call. > > Thanks! > > Alvaro. > > > | On 20 Dec 2016, at 14:15, Alvaro Retana (aretana) <aret...@cisco.com > <mailto:aret...@cisco.com>> wrote: > … > | | M2. Section 3.2. (Certificate Authority Use): “Relying Parties that do > not support this delta protocol MUST > | | NOT reject a CA certificate merely because it has an SIA extension > containing this new kind of > | | AccessDescription.” By definition, an RP that has never even considered > this document will not support > | | the delta protocol – IOW, trying to specify the behavior of RPs that may > have never even seen this > | | document makes no sense to me, and can’t be enforced. What is the > current behavior when an RP receives > | | the extra SIA extension? > | > | The point of documenting this is to give RP software the option of > recognising that a new protocol exists, > | even if they do not fully support it yet. In practice all current RPs do > this (see below), and future RPs should > | consider this document. We have in fact been publishing these additional > SIAs in the RIPE NCC production > | RPKI for more than a year without problems. I changed it slightly to this: > | > | "Relying Parties that choose not to support this delta protocol yet, MUST > NOT reject a CA certificate merely > | because it has an SIA extension containing this new kind of > AccessDescription." > | > | But, I can also live with taking this sentence out altogether. > > I would prefer it if we do that. > ok > > … > | | M4. From 3.3.2. (Publishing Updates): “The update notification file > SHOULD be kept small…older delta files > | | that…will result in total size of deltas exceeding the size of the > snapshot, MUST be excluded.” Here the > | | “SHOULD” and the “MUST” are in contradiction: you either do it always > (MUST) or there may be cases when > | | you don’t (SHOULD). s/SHOULD/should > | > | Ok, I moved the file size concern to the next bullet point instead, so that > we have: > | > | o Any older delta files that, when combined with all more recent delta > files, will result > | in a total size of deltas exceeding the size of the snapshot, MUST be > excluded to avoid > | that RPs download more data than necessary. > | > | o The server MAY also exclude more recent delta files if it finds that > their usage by a > | small number of RPs that would be forced to perform a full > synchronisation is outweighed > | performance benefits of having a smaller update notification file. > However, the repository > | server MUST include all deltas that it has available for the last two > hours. > | > | I hope that explains it better. I changed the SHOULD in the second bullet > point to a MUST. The unspoken > | reason for the SHOULD was that a publication server may not have deltas. > > The problem that I think can still exist is time dependent: “older delta > files…MUST be excluded…MUST include all deltas that is has available for the > last two hours”. What happens if all “older delta files” are less than two > hours old, but the size would be too big? Is there the possibility of a case > where there are a lot of changes that happen close together (but more than a > minute apart) that would then result in many delta files? Besides not > postponing publication for more than a minute, I don’t remember other > mechanisms that would avoid a large number of changes… If this case is not > possible, then we should be ok – I would still like to understand why. > Let me clarify first. No, it should never exceed snapshot file. But I understand the wording is confusing, how about this instead then? o The server MAY also choose to exclude old delta files, even though their combined size does not exceed the size of the snapshot file, if they find that files of a certain age are only very infrequently requested. Since most Relying Parties will keep close track of new deltas, excluding such deltas will help to keep the size of the Notification File small and thus reduce the load on the Publication Server in particular. However, if a server chooses to use this strategy they MUST not exclude such deltas if they are less than two hours old, so that Relying Parties can safely use a one hour refresh interval and suffer short (maintenance) outages without being forced to do a full re-synchronisation. And, yes, two hours is arbitrary. I just felt that there should be some reasonable limit to this. Another approach could be to say that the servers MUST(?) establish first where some threshold lies of the time window where e.g. on average 99.5% (again arbitrary of course) of all requests ever for deltas are typically done. This is something that a publication server can monitor over time. It might find that 99.5% of such requests happen within 1 hour, 2, 5.. we don't really know at this time - this needs deployment. Long story short.. I am also fine with leaving this out of the specification altogether for now. But then I will probably seek to add some wording on this in a -bis or update sometime later once we have more operational experience (and data). > … > | | M7. 3.4.1. (Processing the Update Notification File): “MUST issue an > operator error” What is an “operator > | | error” and how do you issue one? I didn’t see an error definition in the > schema. > | > | I see your point, but.. afaik none of the other sidr RFCs and I-Ds define > this, and just avoid talking about this > | altogether. This was undefined in over five years of deployment with rsync, > and yet operational issues with > | rsync also happen and get reported and resolved somehow. In short I suggest > that I just leave it out here as > | well. > > No. Sorry, but “Others didn’t do it” is not a valid reason for an incomplete > specification. > > | But to continue: All RPs do have the common sense to inform their users > about issues (e.g. also things like: > | hey, this certificate is invalid because..), but there is no standard > defined, so they use various formats and > | ways. I do see some benefit in defining at least a common standard because > it might help (1) monitoring, (2) > | normative documenting of error conditions and responses, and (3) interop > testing between RPs (Rob, David > | and I have done this a few times and it's a bit of a hassle). Doing so is > imo major work and something for a > | separate sidr-ops document. > > One of the reasons I added this comment is because it uses a “MUST”: you’re > mandating the behavior as part of the specification. If it is a mandatory > behavior, then there should be something that goes with is as to how it is > done. One way forward is to clarify what you mean by an “operator error”, > which (interpreting the paragraph above) is really a log message. > > I am fine (if you want to mandate the behavior) for you to say something > like: “MUST log the condition for the operator to be aware of the problem”. > No need to specify the contents, a common format, etc. > > I agree that if you did want to define common formats, contents, etc. then > that would belong somewhere else. Okay, so is your concern addressed if I change "MUST issue an operator error" to “MUST log the condition for the operator to be aware of the problem”? > > … > | See section 3.4.5 of attached file for clarifications that I think we can > do in the context of this document. > | Following your previous remark the only normative addition I feel I can add > is this: "In order to help prevent > | this Publication Servers MUST perform regular validation of their own RRDP > repository." > > The new section looks ok, except for the paragraph that says: “This protocol > document cannot define normative text…” No need to put that in here. ok, will exclude > > > | | M11. From 3.4.3. (Processing Delta Files): “it is RECOMMENDED that a RP > uses additional strategies to > | | determine if an object is still relevant for validation before removing > it from its local storage”. Ok, like > | | what? > | > | I think going into too much detail is out of scope because it cannot be > explained without having a document > | formally describing validation strategies. But I added this: > | > | In particular objects should not be removed if they are included in a > current validated manifest. > > Ok. As before, the reason for the comment is the use of “RECOMMENDED”. > > > … > | | M16. Section 5. (Security Considerations): “RRDP replaces the use of > rsync by HTTPS…”. Given the > | | statement in 3.4.1 about using “an alternate repository retrieval > mechanism” and the rest of the text in this > | | section, I would assume that the intent is not really to “replace the use > of rsync” (with an update to > | | RFC6480). Maybe I’m reading too much into the phrase above, but please > clarify. > | > | As explained above it is the intent to replace rsync. But.. a migration > document should be written as a > | separate effort. > | > | That said I would be fine with just taking out: > | > | The original RPKI transport mechanism is rsync, which offers no channel > | security mechanism. RRDP replaces the use of rsync by HTTPS; > | > | Please let me know if you prefer this. > > > Let me try to make the point again – I didn’t do a good job above. In fact, > reading this over I want to change it… > > In this document you’re saying: here’s a new protocol that SHOULD be used > instead of rsync (3.4.1). > > And, as you mentioned above, the intent is to eventually phase out rsync in > favor of RRDP. Time lines to phase it out are really out of the scope of > this (or I think any other RFC) document because it is something the RPKI > community agrees on and migrates to, just like any other new protocol. > > All that is good. > > However, both RFC6480 and RFC6481 mandate the use of rsync: “all publication > points MUST be accessible via rsync” and “publication repository MUST be > available using rsync”. Which means that to comply with the RPKI > architecture rsync MUST be supported forever, regardless of whether RRDP is > used, or a different protocol that may come along in the future…or even if it > is not needed at all anymore (not even for eventual backup). L > > The question about updating RFC6480 above should have been a statement: this > document should update RFC6480/RFC6481 so that rsync is not required anymore. > > Changing the text in RFC6480/RFC6481 to say “MUST use RRDP” would just result > in another update later if another protocol ever comes along, so I think this > document should be explicit in the change (to allow the move away from > rsync), but still generic…for example, the text in RFC6480/RFC6481 can be > replaced with something like: “The publication point MUST be accessible using > a retrieval mechanism consistent with the accessMethod element value(s). > Multiple retrieval mechanisms can be supported at the repository operator’s > discretion”. > > A change like that would require that this document be tagged to Update > RFC6480/RFC6481 (and would of course return RFC6481 to be Normative). Ok, I understand. But I will need a bit of time for the text. (baby number two has his due date this Thursday so bear with me). > > > … > | | P4. “version 4 UUID” Please provide a reference. > | > | ack, I assumed informative. > > Normative, because the document says that it MUST be used. BTW, please put > the reference on the first mention of UUID. ok, will do Thanks Tim
_______________________________________________ sidr mailing list sidr@ietf.org https://www.ietf.org/mailman/listinfo/sidr
