search

NLnetLabs / krill

RPKI Certificate Authority and Publication Server written in Rust
https://nlnetlabs.nl/projects/routing/krill/
Mozilla Public License 2.0
280 stars 37 forks source link

Don't create an RRDP delta for publishers entries without content #1181

Closed timbru closed 4 weeks ago

timbru commented 6 months ago

This should prevent empty RRDP deltas from being produced by not staging empty publication (RFC8181) deltas for the publisher and skipping them in the decision process to decide whether to produce a new RRDP delta.

As far as I can tell Krill CAs do not send empty publication deltas but issue #1180 shows that empty RRDP deltas were produced possibly because non-Krill publishers sent them.

ximon18 commented 6 months ago

Notes to self:

timbru commented 6 months ago

Notes to self:

  • Ideally I think this PR should include a test that shows that the issue occurs and is fixed.

Ideally, yes. But it was not trivial to do this...

There are tests in place (under the tests directory) that have CAs publish at a Krill Server. However, Krill CAs do not send empty publication requests. Setting up an intentionally broken (or well, at least different) publishing CA is not trivial. Unit testing at the current function level is even less trivial because it would require a lot of set up and new code.

Extracting if self.staged_elements.values().any(|el| !el.0.is_empty()) { to a function could allow for some unit testing of that function with specific instances of self, but when implementing this I felt that was taking things a bit too far, as the test then becomes very tied to the implementation and become rather trivial.

Anyway... not saying any of this in protest against adding a test. Just trying to explain why there isn't one.

  • Given that the RFC defines an XML schema to validate against, do the Krill tests include anywhere XML schema compliance validation and if not, should they? (though perhaps not as part of this PR)

I am not sure, this kind of leads to formal schema validation in the XML library - which is not supported. Maybe, but it would need case by case evaluation. In this particular case it would not help as the internal state with an empty delta should be prevented. The XML is just a representation of that that state. So failing the XML generation would not help, and could even lead to irrecoverable issues (theoretically).

ximon18 commented 6 months ago

  1. Thanks for the insights, appreciated.

  2. Re XML validation I was referring more generally to showing that the XML produced is RFC compliant, not that XML representation is the right level to test this change at.