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.

Sign up for GitHub

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

Add pure version of authorizeBiscuit #79

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Add pure version of authorizeBiscuit #79

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
e707dbd
Add pure version of authorizeBiscuit
kephas Mar 29, 2023
213567e
Add haddoc comment with security warning
kephas Mar 30, 2023
6b1bae4
Fix comment
kephas Apr 1, 2023
4b11e45
Remove code duplication
kephas Apr 1, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 25 additions & 1 deletion biscuit/src/Auth/Biscuit/Token.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,8 @@ module Auth.Biscuit.Token
, serializeBiscuit
, authorizeBiscuit
, authorizeBiscuitWithLimits
, authorizeBiscuitNoTimeout
, authorizeBiscuitWithLimitsNoTimeout
, fromOpen
, fromSealed
, asOpen
Expand Down Expand Up @@ -92,7 +94,8 @@ import Auth.Biscuit.Datalog.ScopedExecutor (AuthorizationSuccess,
collectWorld,
queryAvailableFacts,
queryGeneratedFacts,
runAuthorizerWithLimits)
runAuthorizerWithLimits,
runAuthorizerNoTimeout)
import qualified Auth.Biscuit.Proto as PB
import Auth.Biscuit.ProtoBufAdapter (blockToPb, pbToBlock,
pbToProof,
Expand Down Expand Up @@ -578,6 +581,24 @@ authorizeBiscuitWithLimits l biscuit@Biscuit{..} authorizer =
(toBlockWithRevocationId <$> blocks)
authorizer

authorizeBiscuitWithLimitsNoTimeout :: Limits -> Biscuit proof Verified -> Authorizer -> Either ExecutionError (AuthorizedBiscuit proof)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could you add a haddoc comment explaining its purpose and outlining the danger?
it might good to refer to it from the doc items of authorizeBiscuitWithLimits

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know Prolog can easily have infinite loops but I don't know about Datalog or the way it's evaluated in Biscuit, so I'll write the doc warning about that too.

Copy link
Collaborator

@divarvel divarvel Mar 31, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The semantics for this Datalog flavour means it always converges, so it's not about infinite loops per se, but about ensuring that it does within an acceptable duration

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment fixed.

authorizeBiscuitWithLimitsNoTimeout l biscuit@Biscuit{..} authorizer =
let toBlockWithRevocationId ((_, block), sig, _, eSig) = (block, sigBytes sig, snd <$> eSig)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

on one hand it might be possible to extract this common code in a helper function, but OTOH it's just plumbing with no important logic in it so it's not too big an issue.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, this PR was more a proof-of-concept than a definitive alternative. I wondered if it would be better to factor out the common parts but I went for the easiest path first.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The common part could be authorizeBiscuitWithRunner :: Functor f => (Limits -> BlockWithRevocationId -> [BlockWithRevocationId] -> Authorizer -> f (Either ExecutionError AuthorizationSuccess)) -> Limits -> Biscuit proof Verified -> Authorizer -> f (Either ExecutionError (AuthorizedBiscuit proof)) and it would have the exact same code as the current authorizeBiscuitWithLimits with runAuthorizerWithLimits replaced by the first argument.

Then authorizeBiscuitWithLimitsNoTimeout = unIdentity . authorizeBiscuitWithRunner runAuthorizerNoTimeout…

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that's what I had in mind

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added it to the PR.

-- the authority block can't be externally signed. If it carries a signature, it won't be
-- verified. So we need to make sure there is none, to avoid having facts trusted without
-- a proper signature check
dropExternalPk (b, rid, _) = (b, rid, Nothing)
withBiscuit authorizationSuccess =
AuthorizedBiscuit
{ authorizedBiscuit = biscuit
, authorizationSuccess
}
in withBiscuit <$>
runAuthorizerNoTimeout l
(dropExternalPk $ toBlockWithRevocationId authority)
(toBlockWithRevocationId <$> blocks)
authorizer

-- | Given a biscuit with a verified signature and an authorizer (a set of facts, rules, checks
-- and policies), verify a biscuit:
--
Expand All @@ -594,6 +615,9 @@ authorizeBiscuitWithLimits l biscuit@Biscuit{..} authorizer =
authorizeBiscuit :: Biscuit proof Verified -> Authorizer -> IO (Either ExecutionError (AuthorizedBiscuit proof))
authorizeBiscuit = authorizeBiscuitWithLimits defaultLimits

authorizeBiscuitNoTimeout :: Biscuit proof Verified -> Authorizer -> Either ExecutionError (AuthorizedBiscuit proof)
authorizeBiscuitNoTimeout = authorizeBiscuitWithLimitsNoTimeout defaultLimits

-- | Retrieve the `PublicKey` which was used to verify the `Biscuit` signatures
getVerifiedBiscuitPublicKey :: Biscuit a Verified -> PublicKey
getVerifiedBiscuitPublicKey Biscuit{proofCheck} =
Expand Down