scionproto / scion

SCION Internet Architecture
https://scion.org
Apache License 2.0
382 stars 160 forks source link

slayers: add packet validation #4278

Open matzf opened 1 year ago

matzf commented 1 year ago

The slayers package is designed to be purely a parsing layer with only minimal validation, in order to allow parsing both valid and invalid packets. The general idea is that the consumers of the slayers package are responsible for the validation. Currently, there is no validation "library" that these consumers can rely on, and so it seems likely that packets will not be validated consistently.


Proposal: we add a generic validation functionality that checks for the well-formed-ness of parsed packets. It explicitly should not include implementation-specific checks, like checking for supported address types -- these checks should still only happen during processing of the packet.

The following checks should be covered by this functionality:

(*) in scion.Raw we currently check for truncation explicitly even though this is not actually required for parsing. (**) SegLen is currently validated during parsing

These checks are relatively straight forward. It seems appropriate implement all of these checks directly in the slayers package, by implementing an interface with a Validate() error in all slayers layer and path types.


Note: this generalizes #4133, triggered by the discussion in #4270.

jiceatscion commented 6 months ago

In terms of refactoring practicality. I suspect there are more places that need or do validation than places that don't want it. If this suspicion is correct, it means that we could rename all the DecodeFromBytes methods to something like "UnsafeDecodeFromBytes" and give the original name to the equivalent validating. Then move all the checks from above or below into them at our leisure.