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

Fix CDDL inconsistencies in attStmtType and compound format #2216

Merged
merged 4 commits into from
Dec 11, 2024

Conversation

emlun
Copy link
Member

@emlun emlun commented Nov 27, 2024

Closes #2210. Also fixes a couple of other issues I noticed while working on this:

  • §6.5.2. Attestation Statement Formats refers to a CDDL extension point $attStmtFormat that (no longer?) exists, it should be $$attStmtType.
  • The .within { fmt: text .ne "compound" } control on nonCompoundAttStmt inadvertently makes nonCompoundAttStmt unable to match anything, because maps are matched exhaustively (see CDDL, so the right-hand-side of .within forbids additional fields beyond fmt.

Preview | Diff

I don't think the expression `attStmtTemplate .within $$attStmtType`
successfully encodes the intent "Every attestation statement format must have
the above fields", for two reasons: it does not define a CDDL rule since it
contains no = sign, and even if it did, the `.within` control operator would apply
only to the new type defined by that rule, but not to the `attObj` type.

CDDL generally makes a distinction between types and groups, and only mentions
control operators applying to types, so I don't think we can apply `.within` to
`$$attStmtType` directly. This is why we need to duplicate the `authData` field
in `attStmtTemplate`.
This is required by the new "compound" attestation statement format.
[CDDL][1] defines that:

>A map matches a specification given as a group when the group matches
>a sequence of name/value pairs such that all of these name/value
>pairs are present in the map and the map has no name/value pair that
>is not covered by the group.

Therefore the control `.within { fmt: text .ne "compound" }` forbids any maps
that contain additional fields besides `fmt`, which is clearly not what was
intended.

[1]: https://datatracker.ietf.org/doc/html/rfc8610#section-2.1
@emlun emlun added this to the L3 Candidate Recommendation milestone Nov 27, 2024
@emlun emlun self-assigned this Nov 27, 2024
@emlun emlun changed the title Issue 2210 compound att stmt template Fix CDDL inconsistencies in attStmtType and compound format Nov 27, 2024
Copy link
Member

@timcappalli timcappalli left a comment

Choose a reason for hiding this comment

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

Thanks for fixing this!

@agl agl merged commit 3bc8301 into main Dec 11, 2024
2 checks passed
github-actions bot added a commit that referenced this pull request Dec 11, 2024
SHA: 3bc8301
Reason: push, by agl

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Compound attestation statement format is incompatible with attStmtTemplate
4 participants