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

Freshness + challenge-response nonce sizes #48

Merged
merged 3 commits into from
Sep 28, 2023
Merged

Freshness + challenge-response nonce sizes #48

merged 3 commits into from
Sep 28, 2023

Conversation

setrofim
Copy link
Contributor

Document Veraison approach to handling evidence freshness and update challenge-response docs with nonce size restriction.

Add extensions for temporary/working files generated by draw.io to
.gitignore

Signed-off-by: setrofim <[email protected]>
Copy link
Contributor

@thomas-fossati thomas-fossati left a comment

Choose a reason for hiding this comment

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

awesome. A few typos fixed inline.

architecture/verifier/freshness.md Outdated Show resolved Hide resolved
architecture/verifier/freshness.md Outdated Show resolved Hide resolved
architecture/verifier/freshness.md Outdated Show resolved Hide resolved
architecture/verifier/freshness.md Outdated Show resolved Hide resolved
architecture/verifier/freshness.md Outdated Show resolved Hide resolved
architecture/verifier/freshness.md Outdated Show resolved Hide resolved
@setrofim setrofim force-pushed the setrofim branch 2 times, most recently from 2db651a to 848efa2 Compare September 27, 2023 16:43
@yogeshbdeshpande
Copy link
Contributor

@setrofim. thank you for the change, I will review the rest a bit later today

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! Thanks for very well formed draft.

I have made few comments, please do check and take it, if you agree! Otherwise good to go!

@yogeshbdeshpande
Copy link
Contributor

@setrofim All looks good and we are fine to merge. Just one quick question:

In the sequence diagram, for comparison diamond, we depict session == evidence. I know due to space constraint it is done like that, was just curious shall we say something like
s-nonce == ev-nonce

and qualify somewhere below:
s-nonce implies nonce received in session

and
ev-nonce is the nonce decoded in the token.

WDYT?

Copy link
Contributor

@SimonFrost-Arm SimonFrost-Arm left a comment

Choose a reason for hiding this comment

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

LGTM

architecture/verifier/freshness.md Outdated Show resolved Hide resolved
@setrofim
Copy link
Contributor Author

@setrofim All looks good and we are fine to merge. Just one quick question:

In the sequence diagram, for comparison diamond, we depict session == evidence. I know due to space constraint it is done like that, was just curious shall we say something like s-nonce == ev-nonce

and qualify somewhere below: s-nonce implies nonce received in session

and ev-nonce is the nonce decoded in the token.

WDYT?

IMO that would make it more confusing as it's not obvious what "s" and "ev" decode to. Conversely, the entire diagram is about the nonce/challenge -- the inputs going into comparison are nonces.

Document Veraison mechanisms for ensuring evidence freshness.

Signed-off-by: setrofim <[email protected]>
Document limitations for the nonce and nonceSize parameters
for /newSession requests.

Signed-off-by: setrofim <[email protected]>
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 da1d3e0 into main Sep 28, 2023
3 checks passed
@setrofim setrofim deleted the setrofim branch September 28, 2023 09:50
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.

4 participants