Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable adding policies as object #52

Merged
merged 3 commits into from
Jan 24, 2023
Merged

Enable adding policies as object #52

merged 3 commits into from
Jan 24, 2023

Conversation

henrydaly
Copy link
Contributor

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:

  • Model policies as objects (rather than just strings)
  • Introspect unverified biscuits to understand the datalog contained within (without having a public key to verify the signature first)

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
Copy link
Contributor Author

@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
Copy link
Contributor

KannarFr commented Jan 9, 2023

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

@Geal
Copy link
Contributor

Geal commented Jan 9, 2023

@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
Copy link
Contributor Author

@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:

  • Users create resources that need authorization. We also want to be flexible and allow them to share configurable levels of access to other users
  • When user A creates a new resource, they must provide a public key. Subsequently, when any user (A or otherwise) wants to access that resource, they must provide:
    • A biscuit whose signature is validated against the resource public key
    • A biscuit whose payload includes the capability to access the resource (imagine an operation-resource pair e.g. ("write", "file_b"))

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:

  • The request is "read resource A, and write the result to resource B"
  • The request fragments are [ ("read", "A") , ("write", "B") ]
  • If we authorize both fragments, we can authorize the request itself

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
Copy link
Contributor

Geal commented Jan 9, 2023

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:

  • this is not yet supported in Java but should be added: a token can contain a key id field that is used to indicate which root key signed it. When verifying, we pass a function Option<u32> -> PublicKey that will return the key corresponding to the id. This was meant for key migrations (when changing root keys, both the old and new might be used at the same time), but it could be used here
  • blocks can have a free form string in the context field. You can store there a hint about which key should be used

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

@henrydaly
Copy link
Contributor Author

@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.

  • the key id will tell us what biscuit maps to what resource public key, but unfortunately, it's not available today
  • we already considered the context field, but since users create biscuits without our involvement, we have no authority to mandate the standard (and hence want to build something on our end that we're sure will work every time). The only real way to know for sure what a biscuit authorizes is by looking at the datalog, at least as far as I can tell

@henrydaly
Copy link
Contributor Author

@KannarFr Unit tests added

@KannarFr
Copy link
Contributor

@Geal WDYT?

@Geal
Copy link
Contributor

Geal commented Jan 22, 2023

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?

  • I thought initially that matching the public key was the main issue, but apparently it's not. Is it because we could get cases where both request fragments are signed by the same key but 2 tokens are provided? And so you would need to load the entire authz for both fragments with both tokens?
  • are you planning cases with more than 2 fragments ?
  • was there no way to have the request fragments indicate which token they are paired with?
  • if signature verification is cheap enough (or if we can add the key id feature to biscuit-java), would it be possible create the authorizer from there, then do a query on the authorizer to know the scope, and only then add the required data to do the full authorization ?

@henrydaly
Copy link
Contributor Author

@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:

  • 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.
  • 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.
  • I think it's quite reasonable (and almost guaranteed) that we will see requests that can be broken down into 10+ fragments.
  • 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.

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
Copy link
Contributor

Geal commented Jan 23, 2023

@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 😅
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:

  • create an authorizer from a token (for now, test all the keys until one passes, but we'll have the key id to fix that)
  • run a query on that authorizer to know which resource it refers to. The query can run without having to do the full authorization
  • if it is used by multiple fragments, clone it here (so we can restart from a "fresh" authorizer)
  • load the data related to the fragment
  • now run the authorization

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
Copy link
Contributor Author

@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
Copy link
Contributor

Geal commented Jan 24, 2023

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

@henrydaly henrydaly changed the title Enable introspection Enable adding policies as object Jan 24, 2023
@henrydaly
Copy link
Contributor Author

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

@KannarFr KannarFr merged commit a1e1ca9 into biscuit-auth:master Jan 24, 2023
@Geal
Copy link
Contributor

Geal commented Jan 27, 2023

@henrydaly as a follow up, the PR adding support for the root key id #53

@Geal
Copy link
Contributor

Geal commented Jan 29, 2023

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants