From 684c21c7a40bd3750c17eed6b2363bfe98955ab1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20Mur=C3=A9?= Date: Thu, 19 Sep 2024 11:16:33 +0200 Subject: [PATCH] delegation: tune the validation step - avoid a double parsing when the flow already parsed (command, policy) - don't require a did:key, as other types are legal (but might require a resolver, TODO) - make the command a struct instead of a pointer: we don't need to avoid copy, and the pointer can be interpreted as nil - make the nonce parameter optional, but generate one if none is given --- capability/command/command.go | 34 +++++++++---------- capability/command/command_test.go | 2 +- delegation/delegation.go | 54 ++++++++++++++++++++---------- delegation/delegation_test.go | 9 +++-- 4 files changed, 60 insertions(+), 39 deletions(-) diff --git a/capability/command/command.go b/capability/command/command.go index de4b08b..6e6efc2 100644 --- a/capability/command/command.go +++ b/capability/command/command.go @@ -20,11 +20,10 @@ type Command struct { segments []string } -// New creates a validated command from the provided list of segment -// strings. An error is returned if an invalid Command would be -// formed -func New(segments ...string) *Command { - return &Command{segments: segments} +// New creates a validated command from the provided list of segment strings. +// An error is returned if an invalid Command would be formed +func New(segments ...string) Command { + return Command{segments: segments} } // Parse verifies that the provided string contains the required @@ -32,26 +31,26 @@ func New(segments ...string) *Command { // Command. // // [segment structure]: https://github.com/ucan-wg/spec#segment-structure -func Parse(s string) (*Command, error) { +func Parse(s string) (Command, error) { if !strings.HasPrefix(s, "/") { - return nil, ErrRequiresLeadingSlash + return Command{}, ErrRequiresLeadingSlash } if len(s) > 1 && strings.HasSuffix(s, "/") { - return nil, ErrDisallowsTrailingSlash + return Command{}, ErrDisallowsTrailingSlash } if s != strings.ToLower(s) { - return nil, ErrRequiresLowercase + return Command{}, ErrRequiresLowercase } // The leading slash will result in the first element from strings.Split // being an empty string which is removed as strings.Join will ignore it. - return &Command{strings.Split(s, "/")[1:]}, nil + return Command{strings.Split(s, "/")[1:]}, nil } // MustParse is the same as Parse, but panic() if the parsing fail. -func MustParse(s string) *Command { +func MustParse(s string) Command { c, err := Parse(s) if err != nil { panic(err) @@ -62,11 +61,10 @@ func MustParse(s string) *Command { // [Top] is the most powerful capability. // // This function returns a Command that is a wildcard and therefore represents the -// most powerful abilily. As such it should be handle with care and used -// sparingly. +// most powerful ability. As such, it should be handled with care and used sparingly. // // [Top]: https://github.com/ucan-wg/spec#-aka-top -func Top() *Command { +func Top() Command { return New() } @@ -78,18 +76,18 @@ func IsValid(s string) bool { // Join appends segments to the end of this command using the required // segment separator. -func (c *Command) Join(segments ...string) *Command { - return &Command{append(c.segments, segments...)} +func (c Command) Join(segments ...string) Command { + return Command{append(c.segments, segments...)} } // Segments returns the ordered segments that comprise the Command as a // slice of strings. -func (c *Command) Segments() []string { +func (c Command) Segments() []string { return c.segments } // String returns the composed representation the command. This is also // the required wire representation (before IPLD encoding occurs.) -func (c *Command) String() string { +func (c Command) String() string { return "/" + strings.Join(c.segments, "/") } diff --git a/capability/command/command_test.go b/capability/command/command_test.go index cd35d69..bf878b7 100644 --- a/capability/command/command_test.go +++ b/capability/command/command_test.go @@ -74,7 +74,7 @@ func TestParseCommand(t *testing.T) { cmd, err := command.Parse(testcase.inp) require.ErrorIs(t, err, testcase.err) - require.Nil(t, cmd) + require.Equal(t, command.Command{}, cmd) }) } }) diff --git a/delegation/delegation.go b/delegation/delegation.go index c06e7c2..b89560d 100644 --- a/delegation/delegation.go +++ b/delegation/delegation.go @@ -1,6 +1,7 @@ package delegation import ( + "crypto/rand" "errors" "fmt" "time" @@ -21,7 +22,7 @@ type Token struct { // Principal that the chain is about (the Subject) subject did.DID // The Command to eventually invoke - command *command.Command + command command.Command // The delegation policy policy policy.Policy // A unique, random nonce @@ -35,7 +36,7 @@ type Token struct { } // New creates a validated Token from the provided parameters and options. -func New(privKey crypto.PrivKey, aud did.DID, cmd *command.Command, pol policy.Policy, nonce []byte, opts ...Option) (*Token, error) { +func New(privKey crypto.PrivKey, aud did.DID, cmd command.Command, pol policy.Policy, opts ...Option) (*Token, error) { iss, err := did.FromPrivKey(privKey) if err != nil { return nil, err @@ -48,7 +49,7 @@ func New(privKey crypto.PrivKey, aud did.DID, cmd *command.Command, pol policy.P command: cmd, policy: pol, meta: meta.NewMeta(), - nonce: nonce, + nonce: nil, } for _, opt := range opts { @@ -57,6 +58,13 @@ func New(privKey crypto.PrivKey, aud did.DID, cmd *command.Command, pol policy.P } } + if len(tkn.nonce) == 0 { + tkn.nonce, err = generateNonce() + if err != nil { + return nil, err + } + } + if err := tkn.validate(); err != nil { return nil, err } @@ -64,7 +72,7 @@ func New(privKey crypto.PrivKey, aud did.DID, cmd *command.Command, pol policy.P return tkn, nil } -func Root(privKey crypto.PrivKey, aud did.DID, cmd *command.Command, pol policy.Policy, nonce []byte, opts ...Option) (*Token, error) { +func Root(privKey crypto.PrivKey, aud did.DID, cmd command.Command, pol policy.Policy, opts ...Option) (*Token, error) { sub, err := did.FromPrivKey(privKey) if err != nil { return nil, err @@ -72,7 +80,7 @@ func Root(privKey crypto.PrivKey, aud did.DID, cmd *command.Command, pol policy. opts = append(opts, WithSubject(sub)) - return New(privKey, aud, cmd, pol, nonce, opts...) + return New(privKey, aud, cmd, pol, opts...) } // Issuer returns the did.DID representing the Token's issuer. @@ -95,7 +103,7 @@ func (t *Token) Subject() did.DID { } // Command returns the capability's command.Command. -func (t *Token) Command() *command.Command { +func (t *Token) Command() command.Command { return t.command } @@ -128,23 +136,16 @@ func (t *Token) validate() error { var errs error requiredDID := func(id did.DID, fieldname string) { - if !id.Key() { - errs = errors.Join(errs, fmt.Errorf("a \"did:key\" is required for %s: %s", fieldname, id.String())) + if !id.Defined() { + errs = errors.Join(errs, fmt.Errorf(`a valid did is required for %s: %s`, fieldname, id.String())) } } requiredDID(t.issuer, "Issuer") requiredDID(t.audience, "Audience") - if t.subject != did.Undef { - requiredDID(t.subject, "Subject") - } - if _, err := command.Parse(t.command.String()); err != nil { - errs = errors.Join(errs, err) - } - - if _, err := t.policy.ToIPLD(); err != nil { - errs = errors.Join(errs, err) + if len(t.nonce) < 12 { + errs = errors.Join(errs, fmt.Errorf("token nonce too small")) } return errs @@ -205,6 +206,15 @@ func WithSubject(sub did.DID) Option { } } +// WithNonce sets the Token's nonce with the given value. +// If this option is not used, a random 12-byte nonce is generated for this required field. +func WithNonce(nonce []byte) Option { + return func(t *Token) error { + t.nonce = nonce + return nil + } +} + // tokenFromModel build a decoded view of the raw IPLD data. // This function also serves as validation. func tokenFromModel(m tokenPayloadModel) (*Token, error) { @@ -265,3 +275,13 @@ func tokenFromModel(m tokenPayloadModel) (*Token, error) { return &tkn, nil } + +// generateNonce creates a 12-byte random nonce. +func generateNonce() ([]byte, error) { + res := make([]byte, 12) + _, err := rand.Read(res) + if err != nil { + return nil, err + } + return res, nil +} diff --git a/delegation/delegation_test.go b/delegation/delegation_test.go index 62feeae..27e8954 100644 --- a/delegation/delegation_test.go +++ b/delegation/delegation_test.go @@ -86,7 +86,8 @@ func TestConstructors(t *testing.T) { require.NoError(t, err) t.Run("New", func(t *testing.T) { - dlg, err := delegation.New(privKey, aud, cmd, pol, []byte(nonce), + dlg, err := delegation.New(privKey, aud, cmd, pol, + delegation.WithNonce([]byte(nonce)), delegation.WithSubject(sub), delegation.WithExpiration(exp), delegation.WithMeta("foo", "fooo"), @@ -105,7 +106,8 @@ func TestConstructors(t *testing.T) { t.Run("Root", func(t *testing.T) { t.Parallel() - dlg, err := delegation.Root(privKey, aud, cmd, pol, []byte(nonce), + dlg, err := delegation.Root(privKey, aud, cmd, pol, + delegation.WithNonce([]byte(nonce)), delegation.WithExpiration(exp), delegation.WithMeta("foo", "fooo"), delegation.WithMeta("bar", "barr"), @@ -134,7 +136,8 @@ func privKey(t *testing.T, privKeyCfg string) crypto.PrivKey { } func TestKey(t *testing.T) { - t.Skip() + // TODO: why is this broken? + t.Skip("TODO: why is this broken?") priv, _, err := crypto.GenerateEd25519Key(rand.Reader) require.NoError(t, err)