rescrv/libmacaroons

binary serialization format could be improved

Closed this issue · 2 comments

  • fields are limited to ~64K bytes. Although web cookies
    are limited to 4K, macaroons may have much wider
    use, and fields can grow large when arbitrary info
    is encoded into third party caveat ids.
  • caveat boundaries are not well defined. This
    makes it hard to tell where the last caveat ends,
    and therefore hard to add a new field to either
    caveats or the macaroon itself, which limits
    the scope of potential future backwardly compatible
    format changes.

All that said though, I do appreciate the simplicity,
ASCII-visibility and efficiency of the current format.
Most possible format changes would compromise
on at least one of the above qualities to some degree.

I'm raising this issue because I think the subject
should be at least discussed.

The rationale for the current format is that it is binary safe and easy to parse correctly. If I remember correctly, a similar protocol is used by Git internally (and the framing is similar to what we do in BusyBee, although that is not ascii-visible).

I limited fields to ~64K because that information should be more than sufficient to encode any information into a caveat. If it is a third party caveat that contains information that will not fit, it should be provided to the third party in advance and provided a smaller identifier. Alternatively, it could be broken down into multiple third party caveats. Overall, having a fixed size significantly reduces the risk of bugs stemming from arbitrary sizes, and provides ample space for caveats. If there's a concrete example of something that requires more space, I'd be happy to reconsider the space restriction. I could not think of one that wasn't contrived.

Caveats start with a CID packet and then contain at most one VID or CL packet afterwards. It would be easy to future proof the implementation to discard extra packets, but this feels wrong to me. I cannot think of a field type that would be added that couldn't be contained within the location field. I feel pretty safe with just the fields from the paper, because Google is using macaroons internally and did not describe additional fields in their paper. I have no insight into their implementation, but I do believe it follows from the paper.

I'll leave this issue open for further discussion. In my mind, any changes should (ideally) preserve the same level of simplicity, improve robustness, and be outright rejected by the existing parser unless totally compatible with the existing parser.

Merged.