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

Explicitly state that RPs cannot in general choose attestation type/format #1660

Merged
merged 3 commits into from
Sep 9, 2021

Conversation

emlun
Copy link
Member

@emlun emlun commented Aug 6, 2021

Fixes issue #1659.

Marking this as draft for now because I'm not entirely sure this addition is worth its weight. I'd like to hear what some others think.


Preview | Diff

Copy link
Contributor

@equalsJeffH equalsJeffH left a comment

Choose a reason for hiding this comment

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

Hm. I agree we ought to make it explicitly clear that the authenticator chooses the attestation type&format. However, the proposed statement confuses attestation type&format with attestation conveyance, which is a different thing.

@emlun emlun requested review from equalsJeffH and agl August 18, 2021 17:49
Copy link
Contributor

@equalsJeffH equalsJeffH left a comment

Choose a reason for hiding this comment

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

thx @emlun, nominally looks OK to me, but I wonder about the "and client" portion of the clause:

[=attestation statement format=] is chosen by the [=authenticator=] and [=client=]

...is that referring to the "none" attestation conveyance case where the client modifies the returned attestation statement? If so, I'm inclined to excise the "and [=client=]" and then it LGTM.

@emlun
Copy link
Member Author

emlun commented Aug 20, 2021

That and the "indirect" attestation conveyance case. But yeah, we can probably just leave it at saying just the authenticator chooses. Any reader who wants to know more about attestation will easily find the exceptions to that anyway.

@emlun emlun marked this pull request as ready for review August 25, 2021 13:41
Copy link
Contributor

@sbweeden sbweeden left a comment

Choose a reason for hiding this comment

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

LGTM

@emlun
Copy link
Member Author

emlun commented Sep 9, 2021

Since @equalsJeffH commented:

nominally looks OK
[...]
I'm inclined to excise the "and [=client=]" and then it LGTM.

it seems like this is ready to merge. Thanks everyone!

@emlun emlun merged commit f6a3179 into main Sep 9, 2021
@emlun emlun deleted the issue-1659-explicit-no-attstmt-choice branch September 9, 2021 12:12
github-actions bot added a commit that referenced this pull request Sep 9, 2021
SHA: f6a3179
Reason: push, by @emlun

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.

4 participants