NLnetLabs / routinator

An RPKI Validator and RTR server written in Rust
https://nlnetlabs.nl/projects/routing/routinator/
BSD 3-Clause "New" or "Revised" License
470 stars 71 forks source link

Largest possible manifest number is not supported #943

Closed tomhrr closed 8 months ago

tomhrr commented 8 months ago

See https://github.com/APNIC-net/rpki-mft-number-demo and https://github.com/APNIC-net/rpki-mft-number-demo/blob/main/current-results.txt. The test result when the manifest contains the largest possible manifest number is like so:

manifest-number-largest-value (routinator v0.13.2)
writing RSA key
TAL path: /data/repo/7D1D4374BFE5A78CCA6A3F7895561527B25E64E1.tal
TAL path written to /last-tal-path
Initial validator run:
End validator run
Validator run (largest possible manifest number value):
[2024-03-13T23:02:35] [WARN] rsync://localhost/repo/elctfiab/7D1D4374BFE5A78CCA6A3F7895561527B25E64E1.mft: failed to decode manifest.
End validator run
Validator run (largest possible manifest number value reused):
[2024-03-13T23:02:37] [WARN] rsync://localhost/repo/elctfiab/7D1D4374BFE5A78CCA6A3F7895561527B25E64E1.mft: failed to decode manifest.
End validator run

(Possibly relevant: in 0.12.0, a more specific error message of "serial number longer than 20 bytes" is reported, whereas in 0.13.2 and 0.11.0, the generic "failed to decode manifest" error is reported.)

Not a critical issue, just reporting it because it came up as part of more general manifest number behaviour testing.

partim commented 8 months ago

I interpreted the requirements in RFC 5280 and RFC 9286 as accepting values with the DER-encoded content of the integer to be up to 20 octets. Because integers are always encoded as signed but serial numbers are defined as unsigned, a serial number with 160 significant bits actually encodes as 21 octets – the left-most bit must be zero, so you need to pad the number with an extra octet.

So under this interpretation, the largest allowed value is actually 159 bits all set to 1.

tomhrr commented 8 months ago

Ah yep, thanks, makes sense. I think you are right about this, so I've updated the test repository to have separate 160-bit signed/unsigned tests to highlight the diverging interpretations, pending some consensus on what the correct largest value should be.

tomhrr commented 8 months ago

FWIW, after looking into this in more detail, the next update of the manifest number document will note that the maximum acceptable value here is the largest signed 160-bit integer, per how Routinator handles this field. Thanks for looking at this.