Skip to content
This repository has been archived by the owner on Jul 15, 2021. It is now read-only.

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

Closed
job opened this issue Mar 23, 2020 · 11 comments
Closed

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

job opened this issue Mar 23, 2020 · 11 comments

Comments

@job
Copy link
Member

job commented Mar 23, 2020

In a MITM scenario where an attacker intercepts and manipulates the rsync channel (for example strategically withholding 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 is valid (also 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 APNIC 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 eligeble for further validation, except any data from the publication point where files were missing.

In other words: if one or a few files are missing from the repository, the repository should be considered 'down', and no attempt should be made to start guessing what can be salvaged and what not.

@job
Copy link
Member Author

job commented Mar 24, 2020

To offer an example:

In manifest 1-tcQDnftkRnWbiMhu2cR1-dgmCs.mft, a number of ROAs and a CRL are referenced:

1-tcQDnftkRnWbiMhu2cR1-dgmCs.crl
IM3xMiMU1QChuWMOGgnbDwYolrU.roa
LkKeUPYrfgzjsOIejLjsHGk44cU.roa
LkKeUPYrfgzjsOIejLjsHGk44cU.roa
fIeCcC8KpdJQd-olU2APdxNZkyA.roa
r5vtD4isKhTzGXNGulSnZ7XCS04.roa
r7TSyWn_GbYPjNWvt4r5ewSNAsk.roa

If an attacker withholds all but LkKeUPYrfgzjsOIejLjsHGk44cU.roa, so the MITM does the equivalent of:

cd repository/DEFAULT/3e/01d411-d915-4277-8fe2-76b0dda2bf3e/1/
rm LkKeUPYrfgzjsOIejLjsHGk44cU.roa \
    fIeCcC8KpdJQd-olU2APdxNZkyA.roa \
    IM3xMiMU1QChuWMOGgnbDwYolrU.roa \
    NpI_Uj3OZb6jvwfUTX0-eOk9QV4.roa

and the validator does not consider 1-tcQDnftkRnWbiMhu2cR1-dgmCs.mft in its entirety invalid due to one or more missing .roa or .crl files, in this specific example the remaining ROA will render all BGP announcements equal to or covered under 80.128.0.0/11 invalid.

If the manifest (which references a missing .roa file in its entirety is ignored or considered invalid), the remaining r7TSyWn_GbYPjNWvt4r5ewSNAsk.roa ROA which would've been transformed into VRP 80.128.0.0/11 AS 0 is also ignored. This renders all BGP announcements under that /11 not-found rather than invalid, which is a preferable situation.

rpki-client now has a patch where if a file is referenced the manifest, but is missing (either due to CA operational error, or because of a MITM attack - we can't differentiate the two), that specific manifest is considered invalid. It will depend on the structure of how CA Certificate trees whether this measure is sufficient or whether a single missing file has to result in the entire publication point being considered invalid.

But until we have more information about other attack vectors, I recommend that a manifest as a whole is considered invalid, when it references a non-existing or wrongly checksummed file. The onus is on the CA publication points to publish correct valid RPKI data, the validator's can't be expected to compensate for CA operator errors (or MITM attacks)

@job
Copy link
Member Author

job commented Mar 24, 2020

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.

@omuravskiy
Copy link
Member

Hi Job,

I think it would be much more valuable if you bring these examples back to sidrops@ and we continue discussion there, rather then in this bug report.

@job
Copy link
Member Author

job commented Mar 24, 2020

@omuravskiy This discussion takes place at multiple levels: I am a user of the RIPE NCC RPKI Cache Validator software, and I am reporting a security issue to my vendor (RIPE NCC) via the usual channel (github issues). Another angle of this discussion is whether the specifications could've been worded differently or improved. To me those discussions are orthogonal.

While it is worthwhile to discuss ambiguities of the RFC specifications in the sidrops working group, you as vendor don't need the blessing of the SIDROPS working group whether to publish security patches for the users of your software or not.

A result of discussion in sidrops could be "according to the specifications this is perfectly valid", and at the same time a vendor may still choose to implement stricter validation to better protect the users of the software, because even if the input data is valid (for some value of valid), the end result could be a security vulnerability (such as described in this github issue).

@omuravskiy
Copy link
Member

I think at this stage it is better to contain the discussion to smaller number of different places.
Your examples would bring more to the discussion on sidrops, and I would rather have a common agreement there than having all "vendors" invent their own solutions.
(This is what we already have, sort of.)

@job
Copy link
Member Author

job commented Mar 30, 2020

I think at this stage it is better to contain the discussion to smaller number of different places.

Stephen Kent tells us that discussing Monkey-In-The-Middle attacks is a 'distraction'. This makes productive discussion in sidrops slightly harder.

I believe this is a security issue that needs to be addressed urgently.

@geeohgeegeeoh
Copy link

I do not believe missing files listed in a Manifest should invalidate other content validly detected in the system. I do not believe this change should be implemented, and I would like to see further discussion in SIDROPS and amongst validator authors and RPs

@job
Copy link
Member Author

job commented Mar 31, 2020

I do not believe missing files listed in a Manifest should invalidate other content validly detected in the system. I do not believe this change should be implemented, and I would like to see further discussion in SIDROPS and amongst validator authors and RPs

In light of transparency, who are you?

Why do you believe listed missing files should not invalidate content at the same level? You offer no motivation and do not provide an alternative solution to mitigate the negative impact of the outlined Money-in-the-middle attack.

@geeohgeegeeoh
Copy link

geeohgeegeeoh commented Mar 31, 2020 via email

@job
Copy link
Member Author

job commented Mar 31, 2020

I operate a repository, and you are changing the basis of validation rules regarding my state. Therefore, I am concerned this is happening as bugfix and not (for transparency reasons) in a standards forum,

thanks for clarifying!

@lolepezy
Copy link
Contributor

lolepezy commented Jul 7, 2020

Addressed by introducing 'strict mode' in
https://github.com/RIPE-NCC/rpki-validator-3/releases/tag/3.1-2020.07.06.14.28

@lolepezy lolepezy closed this as completed Jul 7, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants