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

[Oleg Muravskiy <[e-mail]>]

On 5 Sep 2018, at 23:22, Benjamin Kaduk <[e-mail]> wrote:
> 
> 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?

There’s been a call for input: https://mailarchive.ietf.org/arch/msg/sidrops/zs0-pbiIVzeuhINRbYuBBpXhLME


>>> 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”.

>>> 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.

I made changes to our code and the new release will come soon. So I removed this section from the -03 version of the draft.

Cheers,
Oleg