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

Jump to bottom

Allow associating an ID with a biscuit's root key #151

Merged
merged 2 commits into from
Nov 13, 2024
Merged

Allow associating an ID with a biscuit's root key #151

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8ea6cb5
Allow associating an ID with a biscuit's root key
seh Oct 31, 2024
9b7f488
Amend "(*Biscuit).RootKeyID" to return by pointer
seh Nov 7, 2024
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
36 changes: 32 additions & 4 deletions biscuit.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,23 @@ var (
UnsupportedAlgorithm = errors.New("biscuit: unsupported signature algorithm")
)

func New(rng io.Reader, root ed25519.PrivateKey, baseSymbols *datalog.SymbolTable, authority *Block) (*Biscuit, error) {
if rng == nil {
rng = rand.Reader
type biscuitOptions struct {
rng io.Reader
rootKeyID *uint32
}

type biscuitOption interface {
applyToBiscuit(*biscuitOptions) error
}

func newBiscuit(root ed25519.PrivateKey, baseSymbols *datalog.SymbolTable, authority *Block, opts ...biscuitOption) (*Biscuit, error) {
options := biscuitOptions{
rng: rand.Reader,
}
for _, opt := range opts {
if err := opt.applyToBiscuit(&options); err != nil {
return nil, err
}
}

symbols := baseSymbols.Clone()
Expand All @@ -66,7 +80,7 @@ func New(rng io.Reader, root ed25519.PrivateKey, baseSymbols *datalog.SymbolTabl

symbols.Extend(authority.symbols)

nextPublicKey, nextPrivateKey, _ := ed25519.GenerateKey(rng)
nextPublicKey, nextPrivateKey, _ := ed25519.GenerateKey(options.rng)

protoAuthority, err := tokenBlockToProtoBlock(authority)
if err != nil {
Expand Down Expand Up @@ -102,6 +116,7 @@ func New(rng io.Reader, root ed25519.PrivateKey, baseSymbols *datalog.SymbolTabl
}

container := &pb.Biscuit{
RootKeyId: options.rootKeyID,
Authority: signedBlock,
Proof: proof,
}
Expand All @@ -113,6 +128,14 @@ func New(rng io.Reader, root ed25519.PrivateKey, baseSymbols *datalog.SymbolTabl
}, nil
}

func New(rng io.Reader, root ed25519.PrivateKey, baseSymbols *datalog.SymbolTable, authority *Block) (*Biscuit, error) {
var opts []biscuitOption
if rng != nil {
opts = []biscuitOption{WithRNG(rng)}
}
return newBiscuit(root, baseSymbols, authority, opts...)
}

func (b *Biscuit) CreateBlock() BlockBuilder {
return NewBlockBuilder(b.symbols.Clone())
}
Expand Down Expand Up @@ -432,6 +455,10 @@ func (b *Biscuit) BlockCount() int {
return len(b.container.Blocks)
}

func (b *Biscuit) RootKeyID() *uint32 {
return b.container.RootKeyId
}
Copy link
Contributor

Choose a reason for hiding this comment

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

would returning a pointer instead of a tuple also work to signal optionality?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I vacillated over this decision for a while before implementing this new method. I'm glad you asked about it.

Yes, it would work to return a pointer, though it's fairly typical in Go to use this style, especially when the primary return value's type has a sensible zero value.

This paired value style comes from and is adapted somewhat unjustly from a few of Go's statements and expressions that benefit from it:

  • Channel receive statements that can include an optional second untyped boolean value to check whether the channel is closed.
  • Map index expressions that can use an optional second untyped boolean value to check whether the given key is present in the map.
  • Type assertions can use an optional second untyped boolean value to check whether the interface value's dynamic type matches the asserted type.

In each of these cases, callers are free to omit the assignment of this second value, and I don't mean just not assigning it to a named variable. Callers can omit mention of the second value entirely if they don't care about it. By contrast, if a map index expression returned by pointer to indicate whether or not the key was bound to a value in the map, all callers would have to confront the possibility of absence.

My use of that idiom here doesn't benefit from that same treatment, though. A caller of my (*Biscuit).RootKeyID cannot ignore the presence of that second return value. They are free to ignore its value, though, and use the first value unconditionally, since the zero value for uint32 is, well, zero, which is a valid key ID.

Given that, I'm now coming back around to thinking that returning by pointer would be a safer choice here. Please allow me to make that change, and then you can decide if you like that by-pointer approach better.

Copy link
Contributor

Choose a reason for hiding this comment

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

i also think returning a pointer would be a bit safer. The rust implementation distinguishes between no id and 0. Since biscuit uses protobuf 2, I think the serialization also makes that distinction.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Please see commit 9b7f488 for this change. I'll squash the commits before merging.

Copy link
Contributor

Choose a reason for hiding this comment

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

looks good, thanks! in can also squash-merge the PR if that’s simpler for you

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, that's fine with if you squash the commits. I recommend preserving the message from the first one.

After we merge this, I'll carry on with addressing the rest of #150—selecting a public key in calls to the (b *Biscuit).Authorizer method, or a new sibling method, per #150 (comment).


func (b *Biscuit) String() string {
blocks := make([]string, len(b.blocks))
for i, block := range b.blocks {
Expand All @@ -449,6 +476,7 @@ Biscuit {
blocks,
)
}

func (b *Biscuit) Code() []string {
blocks := make([]string, len(b.blocks))
for i, block := range b.blocks {
Expand Down
27 changes: 20 additions & 7 deletions biscuit_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,13 @@ import (

func TestBiscuit(t *testing.T) {
rng := rand.Reader
const rootKeyID = 123
publicRoot, privateRoot, _ := ed25519.GenerateKey(rng)

builder := NewBuilder(privateRoot)
builder := NewBuilder(
privateRoot,
WithRNG(rng),
WithRootKeyID(rootKeyID))

builder.AddAuthorityFact(Fact{
Predicate: Predicate{Name: "right", IDs: []Term{String("/a/file1"), String("read")}},
Expand All @@ -28,13 +32,23 @@ func TestBiscuit(t *testing.T) {

b1, err := builder.Build()
require.NoError(t, err)
{
keyID := b1.RootKeyID()
require.NotNil(t, keyID, "root key ID present")
require.EqualValues(t, rootKeyID, *keyID, "root key ID")
}

b1ser, err := b1.Serialize()
require.NoError(t, err)
require.NotEmpty(t, b1ser)

b1deser, err := Unmarshal(b1ser)
require.NoError(t, err)
{
keyID := b1deser.RootKeyID()
require.NotNil(t, keyID, "root key ID present after round trip")
require.EqualValues(t, rootKeyID, *keyID, "root key ID after round trip")
}

block2 := b1deser.CreateBlock()
block2.AddCheck(Check{
Expand Down Expand Up @@ -202,8 +216,8 @@ func TestBiscuitRules(t *testing.T) {
require.NoError(t, err)

// b1 should allow alice & bob only
//v, err := b1.Verify(publicRoot)
//require.NoError(t, err)
// v, err := b1.Verify(publicRoot)
// require.NoError(t, err)
verifyOwner(t, *b1, publicRoot, map[string]bool{"alice": true, "bob": true, "eve": false})

block := b1.CreateBlock()
Expand Down Expand Up @@ -235,13 +249,12 @@ func TestBiscuitRules(t *testing.T) {
require.NoError(t, err)

// b2 should now only allow alice
//v, err = b2.Verify(publicRoot)
//require.NoError(t, err)
// v, err = b2.Verify(publicRoot)
// require.NoError(t, err)
verifyOwner(t, *b2, publicRoot, map[string]bool{"alice": true, "bob": false, "eve": false})
}

func verifyOwner(t *testing.T, b Biscuit, publicRoot ed25519.PublicKey, owners map[string]bool) {

for user, valid := range owners {
v, err := b.Authorizer(publicRoot)
require.NoError(t, err)
Expand Down Expand Up @@ -318,7 +331,7 @@ func TestGenerateWorld(t *testing.T) {
b, err := build.Build()
require.NoError(t, err)

StringTable := (build.(*builder)).symbols
StringTable := (build.(*builderOptions)).symbols
world, err := b.generateWorld(defaultSymbolTable.Clone())
require.NoError(t, err)

Expand Down
73 changes: 43 additions & 30 deletions builder.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package biscuit

import (
"crypto/ed25519"
"crypto/rand"
"errors"
"io"

Expand All @@ -26,9 +25,10 @@ type Builder interface {
Build() (*Biscuit, error)
}

type builder struct {
rng io.Reader
root ed25519.PrivateKey
type builderOptions struct {
rng io.Reader
rootKey ed25519.PrivateKey
rootKeyID *uint32

symbolsStart int
symbols *datalog.SymbolTable
Expand All @@ -38,38 +38,40 @@ type builder struct {
context string
}

type builderOption func(b *builder)
type builderOption interface {
applyToBuilder(b *builderOptions)
}

func WithRandom(rng io.Reader) builderOption {
return func(b *builder) {
b.rng = rng
}
type symbolsOption struct {
*datalog.SymbolTable
}

func (o symbolsOption) applyToBuilder(b *builderOptions) {
b.symbolsStart = o.Len()
b.symbols = o.Clone()
}

// WithSymbols supplies a symbol table to use when composing biscuits.
func WithSymbols(symbols *datalog.SymbolTable) builderOption {
return func(b *builder) {
b.symbolsStart = symbols.Len()
b.symbols = symbols.Clone()
}
return symbolsOption{symbols}
}

func NewBuilder(root ed25519.PrivateKey, opts ...builderOption) Builder {
b := &builder{
rng: rand.Reader,
root: root,
b := &builderOptions{
rootKey: root,
symbols: defaultSymbolTable.Clone(),
symbolsStart: defaultSymbolTable.Len(),
facts: new(datalog.FactSet),
}

for _, o := range opts {
o(b)
o.applyToBuilder(b)
}

return b
}

func (b *builder) AddBlock(block ParsedBlock) error {
func (b *builderOptions) AddBlock(block ParsedBlock) error {
for _, f := range block.Facts {
if err := b.AddAuthorityFact(f); err != nil {
return err
Expand All @@ -91,7 +93,7 @@ func (b *builder) AddBlock(block ParsedBlock) error {
return nil
}

func (b *builder) AddAuthorityFact(fact Fact) error {
func (b *builderOptions) AddAuthorityFact(fact Fact) error {
dlFact := fact.convert(b.symbols)
if !b.facts.Insert(dlFact) {
return ErrDuplicateFact
Expand All @@ -100,26 +102,37 @@ func (b *builder) AddAuthorityFact(fact Fact) error {
return nil
}

func (b *builder) AddAuthorityRule(rule Rule) error {
func (b *builderOptions) AddAuthorityRule(rule Rule) error {
dlRule := rule.convert(b.symbols)
b.rules = append(b.rules, dlRule)
return nil
}

func (b *builder) AddAuthorityCheck(check Check) error {
func (b *builderOptions) AddAuthorityCheck(check Check) error {
b.checks = append(b.checks, check.convert(b.symbols))
return nil
}

func (b *builder) Build() (*Biscuit, error) {
return New(b.rng, b.root, b.symbols, &Block{
symbols: b.symbols.SplitOff(b.symbolsStart),
facts: b.facts,
rules: b.rules,
checks: b.checks,
context: b.context,
version: MaxSchemaVersion,
})
func (b *builderOptions) Build() (*Biscuit, error) {
opts := make([]biscuitOption, 0, 2)
if v := b.rng; v != nil {
opts = append(opts, WithRNG(b.rng))
}
if v := b.rootKeyID; v != nil {
opts = append(opts, WithRootKeyID(*v))
}
return newBiscuit(
b.rootKey,
b.symbols,
&Block{
symbols: b.symbols.SplitOff(b.symbolsStart),
facts: b.facts,
rules: b.rules,
checks: b.checks,
context: b.context,
version: MaxSchemaVersion,
},
opts...)
}

type Unmarshaler struct {
Expand Down
51 changes: 51 additions & 0 deletions options.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
package biscuit

import "io"

type compositionOption interface {
builderOption
biscuitOption
}

type rngOption struct {
io.Reader
}

func (o rngOption) applyToBuilder(b *builderOptions) {
if r := o.Reader; r != nil {
b.rng = o
}
}

func (o rngOption) applyToBiscuit(b *biscuitOptions) error {
if r := o.Reader; r != nil {
b.rng = r
}
return nil
}

// WithRNG supplies a random number generator as a byte stream from which to read when generating
// key pairs with which to sign blocks within biscuits.
func WithRNG(r io.Reader) compositionOption {
return rngOption{r}
}

type rootKeyIDOption uint32

func (o rootKeyIDOption) applyToBuilder(b *builderOptions) {
id := uint32(o)
b.rootKeyID = &id
}

func (o rootKeyIDOption) applyToBiscuit(b *biscuitOptions) error {
id := uint32(o)
b.rootKeyID = &id
return nil
}

// WithRootKeyID specifies the identifier for the root key pair used to sign a biscuit's authority
// block, allowing a consuming party to later select the corresponding public key to validate that
// signature.
func WithRootKeyID(id uint32) compositionOption {
return rootKeyIDOption(id)
}
Loading