go-macaroon / macaroon

A native Go implementation of macaroons
BSD 3-Clause "New" or "Revised" License
182 stars 24 forks source link

Valid UTF-8 caveat ID requirement? #16

Closed titanous closed 7 years ago

titanous commented 8 years ago

I noticed that the v2-unstable branch enforces valid UTF-8 for caveat IDs. Is there a reason for this?

I'm currently working on a macaroon authorization system that will use a binary encoding for caveat IDs as it can be quite a bit more compact and easier to decode.

rogpeppe commented 7 years ago

This is a really good question. Currently, my thinking is that it's a good idea for caveat conditions to be valid utf-8 because we want them to be easily readable and have a common subset of caveats that are understandable by many first party services (for example, a time expiration caveat).

That said, there's no fundamental reason why we shouldn't support arbitrary data in first party caveats. Given that Go strings can contain arbitrary bytes, perhaps the easiest way to allow this is just to drop the utf8.ValidString check in AddFirstPartyCaveat, while leaving the representation as string to leave a strong hint as to the preferred encoding.

jbowens commented 7 years ago

+1

In my opinion, using something like protocol buffers for first-party caveats makes a lot of sense. Since macaroons are meant to be distributed, you may have many clients in many languages adding caveats. Protocol buffers allow you to have a consistent schema and encoding across all these clients. Most importantly, each client doesn't need to reimplement a bespoke text encoding that's used in a security-critical context 😬 .

kr commented 7 years ago

For what it's worth, I agree with @titanous and @jbowens here.

I actually found it confusing at first that the caveat parameter is a string rather than a []byte, since most encoding libraries work with []byte (e.g. https://godoc.org/encoding/json#Unmarshal and https://godoc.org/github.com/golang/protobuf/proto#Unmarshal).

We were hoping to use a standard format (such as protobufs or json, but preferably protobufs or something similarly compact) to encode our predicates, like @jbowens said, to avoid writing custom parsing logic.

rogpeppe commented 7 years ago

@kr OK, I think I'm persuaded that it should be []byte, but I do think that it's very helpful that the first party caveats are human readable. I think that it would be nice to have some kind of standard for first party caveats, so that clients and third parties can add meaningful first party caveats to other services' macaroons and also interpret them (expiration time is a particular concern here).

We've been trying to build up some conventions on top of the basic macaroon encoding - the relevant one here is https://godoc.org/gopkg.in/macaroon-bakery.v2-unstable/bakery/checkers#Namespace. Before a macaroon is bound to use in a request, we keep it bundled with a representation of the first party caveat namespace (in this type https://godoc.org/gopkg.in/macaroon-bakery.v2-unstable/bakery#Macaroon), so it's possible to know what kinds of caveats might be understood by the target service. Currently, the convention is to use a string that represents the caveat and what namespace it's in, followed by a space and caveat-specific data (e.g. std:time-before 2017-10-12T07:36:22Z). This convention could still work OK in much reduced form (single-byte prefix, colon, single-byte caveat type, space, binary-encoded caveat-specific data), and with some conventions around namespaces, and a bit of tooling, it could even be made human readable.

rogpeppe commented 7 years ago

Fixed by #25. And with that, we declare the API stable at v2. Sorry about the wait.