NICMx / FORT-validator

RPKI cache validator
MIT License
49 stars 24 forks source link

Missing files don't result in a manifest being considered invalid #28

Closed job closed 4 years ago

job commented 4 years ago

In a MITM scenario where an attacker forces a downgrade of the connection from RRDP to rsync, and strategically withholds certain .roa files from the view of the validator being attacked, the resulting set of VRPs will be incomplete and can cause severe operational issues.

When a manifest is valid (manifest is parsable, CRL exists and is not expired, and manifest is signed with keys not revoked by the CRL), and references files which do not exist in the repository at hand, the publication point should be considered compromised.

So in the case of LACNIC where an End User (self-hosted) RPKI publication point misses a few .roa files, the validator can proceed to consider all data from all RPs it could reach valid, except the data from the publication point where files were missing.

pcarana commented 4 years ago

Hi Job, this has taken me to review the RFC. Here's what I've found:

RFC 6486 section 6.1, point 2:

 If there exist files at the publication point that do not appear
 on any manifest, or files listed in a manifest that do not appear
 at the publication point, then see Section 6.5, but still continue
 with the following test.

And then going to section 6.5:

If there exists files listed on the manifest that do not appear in the repository, then these objects are likely to have been improperly (via malice or accident) deleted from the repository. A primary purpose of manifests is to detect such deletions. Therefore, in such a case, this situation SHOULD result in a warning to the effect that: "The following files that should have been present in the repository at are missing . This indicates an attack against this publication point, or the repository, or an error by the publisher."

I get your point, a missing ROA may cause severe operational issues, but the RFC isn't very specific regarding this. What I've understood, is that this scenario should result in a warning, and still the manifest (and therefore the publication point) can be considered valid.

Either way, we'll be reviewing this.

job commented 4 years ago

@pcarana - you are a software developer and you and your team now have a choice to make about what is more important:

1) following RFC and making use of ambiguity or flexibility the specification allows 2) protecting the users of FORT against MITM partial-withholding attacks.

I think we as software developers have a responsibility towards our end users more so than towards IETF documents. What choice is safest for the BGP Operator who may assume 100% of the VRPs coming via RTR are trustworthy. But trustworthiness can only be gauged if the full context is provided, and in the case of a manifest file referencing .roa files which do not exist - an attacker may have rendered the view of the CA into a compromised state.

I would argue that we can only advise network operators to use software that goes above and beyond to truly ensure that any failure in the RPKI (be it operational failure or malicious attacks) does not render any BGP announcements invalid, but rather not-found. All the RIRs have given classes were the message was that RPKI 'fails open'.

As long as we cannot rid the ecosystem of the unecrypted rsync transport mode, we must assume the RPKI data may have been altered in-flight, which means we can't afford to trust the publication point if files are missing.

pcarana commented 4 years ago

you and your team now have a choice to make about what is more important

I agree, this isn't the first case where the specs aren't fully clear and we must decide what to do.

The problem that you state can be an "Incidence" (see https://nicmx.github.io/FORT-validator/incidence.html). So, what we'll do is add such incidence, with a default action of "error" (the file will be treated as invalid, and therefore the manifest won't be considered valid).

job commented 4 years ago

Excellent

job commented 4 years ago

An additional check is needed: if one of the .roa files a manifest references is corrupt (the MITM didn't delete strategically file, but filled some .roa files with garbage), the fix no longer works:

rpki-client:  ...trace: error:0DFFF08E:asn1 encoding routines:CRYPTO_internal:not enough data
rpki-client: rpki.ripe.net/repository/DEFAULT/3e/01d411-d915-4277-8fe2-76b0dda2bf3e/1/fIeCcC8KpdJQd-olU2APdxNZkyA.roa: RFC 6488: failed CMS parse

corrupting that one .roa file results in an incomplete (operationally problematic) set of VRPs:

job@anton ~$ grepcidr 80.128.0.0/11 /var/db/rpki-client/openbgpd
    80.128.0.0/12 source-as 3320
    80.128.0.0/11 source-as 0
    80.128.0.0/11 source-as 3320
    80.144.0.0/13 source-as 3320
    80.152.0.0/14 source-as 3320
    80.156.0.0/16 source-as 3320
    80.157.0.0/16 source-as 3320
    80.157.8.0/21 source-as 3320
    80.157.16.0/20 source-as 3320
    80.158.0.0/17 maxlen 24 source-as 34086
    80.159.224.0/19 maxlen 24 source-as 2792

In the above eaxmple there should be ~ 19 VRPs in total, fIeCcC8KpdJQd-olU2APdxNZkyA.roa contained 8 VRPs.

In summary:

If a manifests references a non-existing file, or if there is a checksum mismatch between the hash described in the manifest and the hash as derived from the .roa file, the RP MUST consider the whole manifest invalid and not produce VRPs with the remaining .roa files.

pcarana commented 4 years ago

Sure, this will be treated as an incidence as well.

For this specific scenario (the file hash doesn't match the one listed at the manifest) the RFC does mention that it can be a matter of "local policy".

The new incidences, by default, will mark both issues as an error: inexistent file, and hash doesn't match.

job commented 4 years ago

@pcarana when will version 1.2.1 be released with these fixes?

pcarana commented 4 years ago

Yes @job, this updates will be released with v1.2.1 :+1:

pcarana commented 4 years ago

Sorry, my bad, didn't read correctly. Currently the updates are being tested by our QA team, the release (hopefully) will be in a few weeks.

pcarana commented 4 years ago

Version 1.2.1 has just been released.