Re: [Sidrops] Benjamin Kaduk's No Objection on draft-ietf-sidrops-rpki-tree-validation-02: (with COMMENT)

[Benjamin Kaduk <[e-mail]>]

On Wed, Sep 05, 2018 at 09:56:46AM +0200, Oleg Muravskiy wrote:
> HI Benjamin,
> 
> > On 4 Sep 2018, at 19:08, Benjamin Kaduk <[e-mail]> wrote:
> > 
> > On Mon, Sep 03, 2018 at 12:19:38AM +0200, Oleg Muravskiy wrote:
> >> Hi Benjamin,
> >> 
> >> Thanks for your comments. I am updating the draft, please see my inline comments below:
> >> 
> >>> On 28 Aug 2018, at 18:35, Benjamin Kaduk <[e-mail]> wrote:
> >>> ----------------------------------------------------------------------
> >>> COMMENT:
> >>> ----------------------------------------------------------------------
> >>> 
> >>> This document is Informational, so I will make my comments non-blocking,
> >>> but I think that the security considerations could be improved.  It
> >>> currently does a good job talking about cases where the procedure can
> >>> receive inconsistent inputs (and how it behaves in the face of those
> >>> inputs), as well as some other considerations, and this is great!  I think
> >>> that the discussion could be more powerful if it also considered how those
> >>> inconsistent inputs could be produced, in particular which capabilities an
> >>> attacker could have.  There seem to be roughly three classes of actors for
> >>> active attacks -- an attacker in the network could modify the transferred
> >>> content (including number of files and directory layout) over unecrypteed
> >>> rsync or http, an attacker that compromises the webserver's certificate
> >>> (or obtains a fraudulent one) could do the same over encrypted https, and
> >>> an attacker that compromises the TA signing key could produce
> >>> fake-but-"valid" manifests, CRLs, etc..  (Is there more that could be done
> >>> with a compromised non-TA CA?  The potential hazards to this algorithm
> >>> don't seem to be noticably distinct, though.)  Some consumers may be
> >>> willing to trust in the TA's integrity or even the Web PKI and not worry
> >>> about those risks.  Though there is always risk of accidental error, of
> >>> course, and the current coverage in this document seems adequate for that
> >>> risk.
> >> 
> >> In the Security Considerations we tried to describe issues specific to our implementation.
> >> What you suggest seems to be a general discussion of vulnerabilities in the RPKI repository architecture that is defined by current standards. I agree that it is missing in the current set of RFCs and it would be great to have it, but on the other hand I do not think it should be done it this document. It would also require another round of going through the WG discussion, I believe. So I left this section as is.
> > 
> > I can't fault you for that.  Do you think there is any energy to put into a
> > standalone document that covers these topics?
> 
> Maybe :)

I'd be happy to see one, though I probably could not be the driving force
myself.

> >>>  4.  Perform manifest entries discovery and validation as described in
> >>>      Section 4.2.2.
> >>> 
> >>> nit: "manifest entry discovery" (singular "entry”)
> >> 
> >> The manifest contains an entry per object in the publication point, so there are at least 2 entries – for a CA cert and a manifest.
> >> I guess plural “entries” is appropriate here.
> > 
> > Ah.  I would suggest "discovery of manifest entries", then, though of
> > course the RFC Editor would probably have some suggestsions as well.
> 
> Sounds good
> 
> >>> Section 5.1.1, 5.1.2
> >>> 
> >>> The syntactic verification performed here is done on what should be
> >>> considered "untrusted input", which means that the verification code needs
> >>> to be written in a robust manner.  (Given the historical recurrences of,
> >>> e.g., ASN.1 decoder security vulnerabilities, we probably need to
> >>> explicitly state this in the security considerations.)
> >> 
> >> Well, I could say that the validation “is written and performed in a robust manner”, but I do not see how this will improve the content...
> > 
> > I was thinking more along the lines of a note in the security
> > considerations:  "The syntactic validation steps performed in Sections 5.1.1
> > and 5.1.2 are operating on untrusted input, so particular care should be
> > taken to avoid buffer overflows and similar attacks."  But this is just a
> > suggestion and I won't be offended if you decide to not use it.
> 
> Well, this document describes the existing implementation, not how it should be done.
> But there is https://datatracker.ietf.org/doc/draft-ietf-sidrops-rp/, I think it would fit there quite well.

Fair enough.  Do I need to send mail or make a pull request or anything
like that, or are the wheels already moving on this one?

Thanks,

Benjamin

> >>> Section 9.1
> >>> 
> >>> If I understand correctly, RFC 6485 allows for (and predicts the need for)
> >>> hash agility in the file hash algorithm.  In this case, it would probably
> >>> be appropriate to say something about how "the security of the system as a
> >>> whole is limited to that of the weakest hash function allowed by consumers,
> >>> but the hash agility provided for by RFC 6485 allows new (stronger) hashes
> >>> to be introduced and old hash functions phased out before they are
> >>> critically broken”.
> >> 
> >> The sentence you propose describes the current situation of the RPKI in general, it is not specific to our implementation, or to RPKI tree validation. 
> >> 
> >> RFC 6485 said:
> >>   The recommended procedures to implement such a
> >>   transition of key sizes and algorithms is not specified in this
> >>   document.
> >> 
> >> As such, our validator does not support any other algorithms for keys and hashes.
> >> By now, however, RFC 6485 has been obsoleted by 7935, which refers to RFC 6916 for details of algorithm agility.
> >> 
> >> I’m adding a sentence that agility is not supported in our implementation.
> > 
> > Okay.  That should give the reader a place to start, if they're interested
> > (please informatively cite 6916, too).
> 
> OK
> 
> >>> This kind of behavior would be seen as an unacceptable vulnerability in a
> >>> standards-track protocol, though since this document is only informational
> >>> it does not block publication.
> >> 
> >> From the RP perspective it is not possible to know whether the object it received from the remote repository is genuine or tampered with. Current standards require that invalid objects must not be used for validation, and this is what our implementation does. There is no requirement to keep and use an “old” object (whatever that might be) if the “new” one is not valid.
> > 
> > Allowing untrusted data that is not authenticated (e.g., by a digital
> > signature) to overwrite existing data in the store that has been
> > authenticated, is something I would not want to see in an IETF protocol.
> > It would only be allowed after the latest manifest has been authenticated
> > and fully processed, and when the "old" resource is no longer referenced by
> > the manifest.  This is a matter of the order of operations for processing,
> > more than a perceived requirement to retain an "old" object that is no
> > longer referenced.  But, since we are not objecting to the current text,
> > feel free to ignore this discussion point.
> 
> OK, thanks for your input.
> I do agree with that, and I think it would not be difficult to fix it in our code and make a maintenance release.
> 
> 
> 
> — 
> Oleg