Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

when using RSYNC transport: for a given CA, ASPA files break validation of sibling ROA files #155

Open
frankarinnet opened this issue Jan 21, 2025 · 10 comments
Milestone

Comments

@frankarinnet
Copy link

frankarinnet commented Jan 21, 2025

We were testing the fort-validator against a repository with ASPA files in our development environment.

When falling back to RSYNC from RRDP, we noticed a failure for any Certificate Authority which had an ASPA file: all associated ROAs of that CA failed to validate. This will be a problem for relying parties (end-users) once the rollout of ASPA begins in earnest.

Any customers with a current (or older) version of FORT will start to see some originations disappear from their list of VRPS -- although only when using RSYNC and only for those CAs that have an ASPA . It would be good to start getting an "aspa-safer" version of FORT into the hands of relying parties.

We think this could probably be addressed by adding "--include=*.asa" to the "recursive_rsync_args" default. You might also want to think about "Signed Checklists / rfc9323 / *.rsc" for similar future proofing (and maybe even rfc8635?).

@ydahhrk
Copy link
Member

ydahhrk commented Jan 21, 2025

We think this could probably be addressed by adding "--include=*.asa" to the "recursive_rsync_args" default

Any objections to just removing those filters (--include and --exclude)? They don't seem like a terribly effective means to prevent garbage caching.

You might also want to think about "Signed Checklists / rfc9323 / *.rsc" for similar future proofing (and maybe even rfc8635?).

You mean allow .rsc files and 8635 certificates?

Or are you implying those RFCs contain a mechanism to filter repository content?

Sorry. I've read 9323 three times by this point, and I still don't feel any closer to understanding its purpose.

@frankarinnet
Copy link
Author

I, personally, have no objections to removing the filters. I doubt that they have much practical effect today -- but I don't know the full history of the feature.

I just brought up the other two RFCs because they are two additional types of objects that the IETF has discussed putting in the repository at some point. Since I think they both would have unique file extensions, I believe they might someday cause a similar sort of "collateral damage" to ROAs that the ASPAs did.

Certainly, if you're removing the filters, you won't have to worry about them triggering this rsync bug. Otherwise, I would tentatively suggest also adding "*.rsc" to the "include" list.

@ties
Copy link

ties commented Jan 22, 2025

I think other implementations (rpki-client) only transfer the files they parse. I think hashes for files that would not be checked by the validator are ignored (@job: is that correct?)

@job
Copy link
Contributor

job commented Jan 22, 2025

I think other implementations (rpki-client) only transfer the files they parse. I think hashes for files that would not be checked by the validator are ignored (@job: is that correct?)

This is the way.

The filters are useful, I wouldn’t remove them.

*.rsc should not be added to these filters because RSC files aren’t distributed through RRDP or RSYNC

@frankarinnet
Copy link
Author

frankarinnet commented Jan 22, 2025

I was indeed incorrect about the RSC files. But the main issue, about the ASPA files, is a real (approaching) issue.

@ydahhrk
Copy link
Member

ydahhrk commented Jan 22, 2025

Ok. To be clear, the reason I was thinking about dropping the filters is because they get in the way of checking this tentatively awkward 9286 requirement:

The RP MUST acquire all of the files enumerated in the manifest (fileList) from the publication point. If there are files listed in the manifest that cannot be retrieved from the publication point, the RP MUST treat this as a failed fetch.

It seems I can't validate this if rsync filters files.

But, as time has grown me weary of reckless RFC conformance, I guess making an exception for unknown files is perfectly reasonable.

Thank you, everyone.

ydahhrk added a commit that referenced this issue Jan 22, 2025
RFC 9286:

> The RP MUST acquire all of the files enumerated in the manifest
> (fileList) from the publication point. If there are files listed in
> the manifest that cannot be retrieved from the publication point,
> the RP MUST treat this as a failed fetch.

This was clashing with Fort's default rsync filters because they were
preventing unknown extensions from being downloaded:

> rsync (...) --include=*.cer --include=*.crl --include=*.gbr \
>	--include=*.mft --include=*.roa --exclude=* (...)

Which will be a problem whenever the IETF defines new legal repository
extensions, such as .asa.

Therefore, ignore unknown manifest fileList extensions. This technically
violates RFC 9286, but it's necessary evil given that we can't trust
repositories to always only serve proper RPKI content.

Fixes #155.
ydahhrk added a commit that referenced this issue Jan 22, 2025
We've agreed extension filters are useful, and the manifest code no
longer drops RPPs due to unknown file-not-founds.

So prevent unknown file extensions from contaminating the RRDP side of
the cache as well.

Complements #155.
@ydahhrk
Copy link
Member

ydahhrk commented Jan 23, 2025

@frankarinnet It seems the patches above fixed the problem. You can find them in branch issue155.

Can you confirm it's working as expected on your end?

@frankarinnet
Copy link
Author

Sorry for the slow response. Will check sometime this week (hopefully today or tomorrow).

@frankarinnet
Copy link
Author

frankarinnet commented Jan 27, 2025

We built the issue155 branch locally, and the bug appears fixed. Against a repository with intermixed ROAs and ASPAs, we got consistent results between:

  • running FORT with http disabled
  • running FORT with http enabled
  • running rpki-client
  • running routinator

From our point-of-view, issue155 correctly addresses the bug.

@ydahhrk
Copy link
Member

ydahhrk commented Jan 27, 2025

Thanks! I'll wait for the #156 patch before releasing.

@ydahhrk ydahhrk added this to the 1.6.6 milestone Jan 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants