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

Benjamin Kaduk <kaduk@mit.edu> Tue, 04 September 2018 17:08 UTC

Return-Path: <kaduk@mit.edu>
X-Original-To: sidrops@ietfa.amsl.com
Delivered-To: sidrops@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id 7A27B130F52; Tue, 4 Sep 2018 10:08:29 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -4.201
X-Spam-Level:
X-Spam-Status: No, score=-4.201 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, RCVD_IN_DNSWL_MED=-2.3, SPF_PASS=-0.001] autolearn=ham autolearn_force=no
Received: from mail.ietf.org ([4.31.198.44]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id xmX-ALcYPIvG; Tue, 4 Sep 2018 10:08:27 -0700 (PDT)
Received: from dmz-mailsec-scanner-5.mit.edu (dmz-mailsec-scanner-5.mit.edu [18.7.68.34]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id 6D30D130DE2; Tue, 4 Sep 2018 10:08:26 -0700 (PDT)
X-AuditID: 12074422-4cdff700000065b6-94-5b8ebc0652d0
Received: from mailhub-auth-2.mit.edu ( [18.7.62.36]) (using TLS with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client did not present a certificate) by dmz-mailsec-scanner-5.mit.edu (Symantec Messaging Gateway) with SMTP id 52.B6.26038.70CBE8B5; Tue, 4 Sep 2018 13:08:24 -0400 (EDT)
Received: from outgoing.mit.edu (OUTGOING-AUTH-1.MIT.EDU [18.9.28.11]) by mailhub-auth-2.mit.edu (8.13.8/8.9.2) with ESMTP id w84H8IAQ018194; Tue, 4 Sep 2018 13:08:19 -0400
Received: from kduck.kaduk.org (24-107-191-124.dhcp.stls.mo.charter.com [24.107.191.124]) (authenticated bits=56) (User authenticated as kaduk@ATHENA.MIT.EDU) by outgoing.mit.edu (8.13.8/8.12.4) with ESMTP id w84H89pa025808 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NOT); Tue, 4 Sep 2018 13:08:11 -0400
Date: Tue, 4 Sep 2018 12:08:09 -0500
From: Benjamin Kaduk <kaduk@mit.edu>
To: Oleg Muravskiy <oleg@ripe.net>
Cc: The IESG <iesg@ietf.org>, draft-ietf-sidrops-rpki-tree-validation@ietf.org, Chris Morrow <morrowc@ops-netman.net>, sidrops-chairs@ietf.org, sidrops@ietf.org
Message-ID: <20180904170806.GI91593@kduck.kaduk.org>
References: <153547412129.23827.3324575091222487462.idtracker@ietfa.amsl.com> <4A92D060-389A-4104-A7CF-49984773596B@ripe.net>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <4A92D060-389A-4104-A7CF-49984773596B@ripe.net>
User-Agent: Mutt/1.9.1 (2017-09-22)
X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFlrBKsWRmVeSWpSXmKPExsUixG6nosuxpy/a4PxMNYuOlweYLWb8mchs cXnhRzaL/g1trBa/fz5ltvi2/zibA5vHkiU/mTweTDrK7rHyQG0AcxSXTUpqTmZZapG+XQJX xuerV5gKej0rOmf8Y2pg/GDZxcjJISFgIvHyzBOWLkYuDiGBxUwSc7q/skM4GxglNv6cwQrh XGGSePH8CAtIC4uAisTOY/2sIDYbkN3QfZkZxBYRUJLoam5gBmlgFtjEKPFt6nOwImGBLIm9 v1aA2bxA+56+uwc2SEigXmLyoU6ouKDEyZlPwOLMAuoSf+ZdAhrEAWRLSyz/xwERlpdo3job bBengI3EuvNr2UFsUQFlib19h9gnMArOQjJpFpJJsxAmzUIyaQEjyypG2ZTcKt3cxMyc4tRk 3eLkxLy81CJdU73czBK91JTSTYzgSHBR2sE48Z/XIUYBDkYlHt6Gtr5oIdbEsuLK3EOMkhxM SqK8WTuAQnxJ+SmVGYnFGfFFpTmpxYcYJTiYlUR4o1uAcrwpiZVVqUX5MClpDhYlcV6nc63R QgLpiSWp2ampBalFMFkZDg4lCV693UCNgkWp6akVaZk5JQhpJg5OkOE8QMO7d4EMLy5IzC3O TIfIn2LU5fjzfuokZiGWvPy8VClxXjOQQQIgRRmleXBzQAlMInt/zStGcaC3hHkVQap4gMkP btIroCVMQEuWHOgBWVKSiJCSamB0Y4r0i5z727k7XK1lBZdQ0l3u9+5/tbM0Q98d2aW8+9+x z+eS2ebefm28sV5M3lZEVGOKueG1JTOWyFx5kNSo3VnS5p8ZufHO9M2JEwonZS5RW+3+XefH xCdc71K2HuCP6ly/PVGpWn7Rk31XLC9HHOfc2pVc9d9jQ8Kn28qZYQ611/wq8mSVWIozEg21 mIuKEwE3P/7gOwMAAA==
Archived-At: <https://mailarchive.ietf.org/arch/msg/sidrops/f2i6NmhliJ2WpylgrUJQ5xsFcMo>
Subject: Re: [Sidrops] Benjamin Kaduk's No Objection on draft-ietf-sidrops-rpki-tree-validation-02: (with COMMENT)
X-BeenThere: sidrops@ietf.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: A list for the SIDR Operations WG <sidrops.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/sidrops>, <mailto:sidrops-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/sidrops/>
List-Post: <mailto:sidrops@ietf.org>
List-Help: <mailto:sidrops-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/sidrops>, <mailto:sidrops-request@ietf.org?subject=subscribe>
X-List-Received-Date: Tue, 04 Sep 2018 17:08:29 -0000

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 <kaduk@mit.edu> 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?

> > Some additional section-by-section comments follow.
> > 
> > Section 3.1
> > 
> > The key properties needed of cryptographic hash functions are either
> > "collision resistance" or "second preimage resistance", depending on
> > whether the attacker gets to pick both hash inputs (the first case) or only
> > one of them (the second).  Which one is needed here would probably depend
> > on whether the CA/issuer is trusted to not be an attacker or not, but
> > regardless, please use the more detailed terminology.
> 
> I’m going for the “collision resistance properties” in that paragraph.
> But also note that it isn’t only about attacks on the repository objects: if there are legitimate objects anywhere in fetched repositories that produce the same hashes, they will collide in our implementation.

Of course.  Even with the "broken" SHA-1 hash, though, such accidental
collisions (with different body content) should be vanishingly rare.

> > Section 3.2
> > 
> >   [...], or use all objects whose AKI extension
> >   matches the Subject Key Identifier (SKI) extension (Section 4.2.1 of
> >   [RFC5280]) of a CA certificate.
> > 
> > "all objects" as discovered within what scope?
> 
> Essentially these are all objects known to a validator at the time the comparison is performed.
> I’m changing the draft to read “all known repository objects”.
> 
> > Section 4.2
> > 
> > Please expand SIA on first usage.
> 
> ACK
> 
> > In bullet point 1, it might help the uninitiated reader to say something
> > after "and pass it to the object fetcher" to note that "the fetcher will
> > then fetch all objects available from that repository”.
> 
> ACK
> 
> > In bullet point 3, please be more clear about which manifest is "this
> > manifest object" that is used to continue validation processing.
> 
> ACK
> 
> >   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.

> >       (Note that this implementation uses the operator configuration to
> >       decide which algorithm to use for path validation.  It applies
> >       selected algorithm to all resource certificates, rather than
> > 
> > nit: "the selected algorithm”
> 
> ACK
> 
> > Please expand ROA on first usage.
> 
> ACK
> 
> > 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.

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

> > Section 9.2
> > 
> >   In case of a mismatch described above this implementation will not
> >   exclude an object from further validation merely because it's actual
> > 
> > nit: "its" (no apostrophe)
> 
> ACK
> 
> > Section 9.3
> > 
> > nit: Which behavior is allowed-but-not-required -- a CA signing things not
> > listed in the manifest, or an RP ignoring things not listed in the
> > manifest?  I suggest "This RP behavior is allowed [...]”.
> 
> ACK
> 
> > Section 9.4
> > 
> > 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.

-Benjamin

> 
> > Section 9.5
> > 
> > It might be useful to reference Section 5.1.1 and how we sometimes fetch
> > the entire repository contents, not limited by a manifest listing.
> 
> ACK
> 
> 
> Cheers,
> Oleg