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

Profiles #29

Merged
merged 3 commits into from
Jul 18, 2024
Merged

Profiles #29

merged 3 commits into from
Jul 18, 2024

Conversation

setrofim
Copy link
Contributor

@setrofim setrofim commented Jul 3, 2024

This adds platform claims implemented as a "profile" of psatoken. This also refactors realm claims to make use of similar mechanisms (and generally remove duplication between this and psatoken

Copy link
Contributor

@yogeshbdeshpande yogeshbdeshpande left a comment

Choose a reason for hiding this comment

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

Nits, but in general LGTM!

setrofim added 3 commits July 18, 2024 11:39
Add CCA platform claims under platform sub-package. These claims on the
old psatoken CCA platform claims that were used prior, but have been
modified to use the new profiles mechanisms that would allow layering
further customisations on top of existing structures.

Existing realm claims are moved into realm sub-package.

As platform and realm sub-packages are introduced, there is also a minor
refactor of the CBOR encoding code. Most if it is moved into encoding
sub-package, with each sub-package (including top level) instantiation
it's own encoding/decoding modes. This organisation is necessary so that
each sub-package can define CBOR tags for its types and avoid cyclic
imports.

The encoding refactor also means that there are now small cbor.go files
instantiating the CBOR modes for their sub-package. These files have 50%
coverage that cannot be increased (they contain panics guarding against
errors that cannot happen). As go tools evaluate coverage for
sub-packages by averaging coverages of individual files, this
significantly impacts the coverage metric for the top level, where there
is now little code. Rather than lowering the CI minimum coverage
requirements module-wide, the coverage script is modified to take
overrides for specific sub-packages. Finally, to avoid confusion, an
unused duplicate of the coverage script under .github/scripts/ is
removed (the CI actually uses the script under top-level scripts/).

BREAKING CHANGE: realm claims have been factored out into their own
sub-package. E.g. github.com/veraison/ccatoken.IRealmClaims is now
github.com/veraison/ccatoken/realm.IClaims.

BREAKING CHANGE: psatoken's ICcaPlatformClaims has been replaced by the
local platform.IClaims.

BREAKING CHANGE:  minimum go version bumped to 1.21 to align with
updated psatoken.

Signed-off-by: Sergei Trofimov <[email protected]>
Refactor realm claims to align with PSA token profile definitions and to
allow them to be potentially extended in an analogous way via CCA
profiles.

- Use psatoken.IClaimsBase to define realm.IClaims. This exposes
  operations common to all claims objects, namely marshalling and
  validation.
- Expose previously internal realm.validate() as realm.ValidateClaims().
  This will help with implementing profiles that do not embed the
  original claims structure.
- Expose individual claim field validators and rename them from
  isValidXXX (which implies a boolean return) to ValidateXXX. This will
  be useful if, e.g., client code wants to make sure a hash is a valid
  realm challenge without needing a claims structure.
- Reuse errors defined inside psatoken, rather then re-defining them for
  realm claims. Errors such as "syntax error" or "missing mandatory
  claims" are in effect part of the generic IClaimsBase interface (i.e.
  common to all profile-able claims objects).
- Do not use json tags inside error messages, as they may be different
  for profiles that implement their own claims without embedding
  existing ones.
- Some minor stylistic tidying (consistent spacing, etc).

Signed-off-by: Sergei Trofimov <[email protected]>
Signed-off-by: Sergei Trofimov <[email protected]>
@setrofim setrofim changed the title [RFC] Profiles Profiles Jul 18, 2024
@setrofim setrofim marked this pull request as ready for review July 18, 2024 10:41
Copy link
Contributor

@yogeshbdeshpande yogeshbdeshpande left a comment

Choose a reason for hiding this comment

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

LGTM!

@setrofim setrofim merged commit 27e89f6 into main Jul 18, 2024
7 checks passed
@setrofim setrofim deleted the profiles branch July 18, 2024 11:09
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.

2 participants