biscuit-auth / biscuit-java

Java implementation of the Biscuit authentication and authorization token
https://biscuitsec.org/
Apache License 2.0
28 stars 13 forks source link

Enable adding policies as object #52

Closed henrydaly closed 1 year ago

henrydaly commented 1 year ago

Our organization has built a client library on top of biscuit-java. As part of our library, we need the ability to do the following:

We've built our own conversion logic to biscuit-java datalog objects (e.g., predicates, rules, facts, etc) and wanted the same function to add policies. Today, we can only add policies as strings. This PR enables introspection of datalog in an unverified biscuit and also enables policy objects to be added directly, rather than requiring string parsing

henrydaly commented 1 year ago

@KannarFr @Geal I don't think I can add reviewers myself, so tagging you just to let you know about this. I'm happy to make changes to the PR, just let me know.

KannarFr commented 1 year ago

LGTM. Can you add simple tests to prevent regressions for upcoming commits?

Geal commented 1 year ago

@henrydaly could you explain why you need to look at the datalog of UnverifiedBiscuit? The entire point of that type is that you cannot trust anything in the biscuit because the signatures have not been checked, so it's raising a flag here

henrydaly commented 1 year ago

@Geal good question! I'd be happy to explain. We are aware that until a biscuit signature is verified, the datalog scope is not verified. What we want to do is look into what's provided in an UnverifiedBiscuit to get clues as to what its scope might be, then verify it before we authorize anything.

Let me give a bit more context:

If the above conditions are met, we authorize the request - simple enough. Now things get interesting. Let's say a user wants to access two resources in the same request (we allow manipulation of multiple resources - about as specific as I can get). In case both resources have the same public key, then fine - one biscuit will suffice. However, if the resources have different public keys, then the user must provide two different biscuits. Now how can we know which biscuit to use to authorize each request fragment? Let me explain request fragmentation via an example:

Now, if we get two biscuits, we could potentially just try to authorize each fragment with both biscuits and see if there's some combination that works. This I believe is poor engineering, and most certainly not scalable as we grow the size of fragmentation. Thus - if we can find out what the payload of biscuit 1 and biscuit 2 look like, we can infer which biscuit should authorize which fragment (by looking into the datalog of UnverifiedBiscuit). Hope that is a sufficient explanation :)

@KannarFr will do

Geal commented 1 year ago

ok, so if I understand correctly, what you mainly need is finding out which token maps to which public key. There are nicer ways to do that:

Trying both keys should still be fine though, the signature verification is cheap enough

henrydaly commented 1 year ago

@Geal Both your above suggestions are useful for sure, but neither quite fits our needs. Biscuit -> public key mapping isn't actually that hard to do, like you said signature checks are cheap enough. We know based on the user request what resources are being accessed, so it's easy enough to go retrieve the public key for the biscuit. Matching biscuit capability against the request fragment is the more difficult challenge.

henrydaly commented 1 year ago

@KannarFr Unit tests added

KannarFr commented 1 year ago

@Geal WDYT?

Geal commented 1 year ago

We aim to design APIs where it is hard to write unsafe code, so that's why I am wary of adding a way to extract and execute datalog at the UnverifiedBiscuit stage.

@henrydaly could you precise some things?

henrydaly commented 1 year ago

@Geal we're not trying to execute any datalog from an UnverifiedBiscuit, only to introspect and see what has been specified. But let me try to answer your questions:

All I'm trying to accomplish in this PR is to enable querying datalog from an unverified biscuit. If the data is already available inside the library, why can't we query it? I didn't add a path for users to construct an authorizer from an unverified biscuit, only read the datalog inside.

Geal commented 1 year ago

@Geal we're not trying to execute any datalog from an UnverifiedBiscuit, only to introspect and see what has been specified. But let me try to answer your questions:

The difference may seem large enough, but querying and executing are not that different. Being able to query and check policies (even if it's some preliminary check) from UnverifiedBiscuit is something we explicitely want to avoid; in security related APIs, if there's a way to use it unsafely, someone at some point will use it without thinking :sweat_smile: I think for your case there's a way to do what you want safely.

  • Correct, it's entirely possible that we have 3 request fragments and 2 biscuits where 2 fragments are authorized by one biscuit and the 3rd is authorized by the other biscuit. I don't think it's even possible (and definitely not a good idea) to create one authorizer from multiple biscuits and try to authorize the full request. It would immediately break our design wherein scope on a given resource for a biscuit is validated against the public key of that resource.

right, that makes sense

  • We receive a request and a list of biscuits from the end user. That request is then broken down into fragments by the server, and each fragment must be associated with a biscuit. So it is not possible to have the fragment indicate which biscuit is needed - that's the whole point of what we're trying to solve.

ah, that's the bit I was missing, thanks!

  • I think it's quite reasonable (and almost guaranteed) that we will see requests that can be broken down into 10+ fragments.

so, kind of batching operations, ok

  • We cannot require users to explicitly tell us which biscuit should be used for each request fragment as they do not see the request fragments, they only provide us the request. In addition, the datalog specified in the biscuits is already enough to tell us which biscuit matches to which fragment (why I want introspection in the first place), so requiring the user to provide this information twice is bad UX even if we could require it.
  • I'm honestly not sure that signature verification is that cheap, just going off of what you said. I don't understand why we have to do signature verification before we can query data.

because you cannot trust the content of the token if the signature was not verified. If we add the key id to this library (which we have to do) there would be only one signature verification per token, no need to test every key.

All I'm trying to accomplish in this PR is to enable querying datalog from an unverified biscuit. If the data is already available inside the library, why can't we query it? I didn't add a path for users to construct an authorizer from an unverified biscuit, only read the datalog inside.

Could you test what I was hinting at in my previous message:

From what you told me, this looks like a safe way to solve the issue (unless I'm still missing something, which is very possible)

henrydaly commented 1 year ago

@Geal alright. I think this will work, but for sure will be less performant until we can have the key-id check. Any idea on when that will be available?

Separately from the UnverifiedBiscuit code changes, I also added the ability to add a Datalog Policy object to an existing authorizer. Can I remove the other changes and get the policy additions merged as part of this PR?

Geal commented 1 year ago

I'll look into the key id in the next few days. For add_policy, yes sure it makes sense to add it!

henrydaly commented 1 year ago

@Geal @KannarFr removed introspection code and updated the PR description. Feel free to merge whenever.

Geal commented 1 year ago

@henrydaly as a follow up, the PR adding support for the root key id https://github.com/biscuit-auth/biscuit-java/pull/53

Geal commented 1 year ago

It's now published under org.biscuitsec.biscuit at version 2.3.1 https://search.maven.org/artifact/org.biscuitsec/biscuit