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

Provide proper Triples explaination #343

Merged
merged 8 commits into from
Nov 20, 2024
Merged

Conversation

yogeshbdeshpande
Copy link
Collaborator

Fixes #310

@yogeshbdeshpande yogeshbdeshpande marked this pull request as ready for review October 31, 2024 17:50
@yogeshbdeshpande yogeshbdeshpande changed the title First revision of triples explination Provide proper Triples explaination Oct 31, 2024
draft-ietf-rats-corim.md Outdated Show resolved Hide resolved
draft-ietf-rats-corim.md Outdated Show resolved Hide resolved
draft-ietf-rats-corim.md Outdated Show resolved Hide resolved
draft-ietf-rats-corim.md Outdated Show resolved Hide resolved
draft-ietf-rats-corim.md Outdated Show resolved Hide resolved
draft-ietf-rats-corim.md Outdated Show resolved Hide resolved
draft-ietf-rats-corim.md Outdated Show resolved Hide resolved
@yogeshbdeshpande
Copy link
Collaborator Author

@nedmsmith, @thomas-fossati and @andrew-draper Kindly review the same, as discussed during the review!

If the search criteria are satisfied, the matching entry is re-asserted, except with the Reference Value Provider's authority.
By re-asserting Evidence using the RVP's authority, the Verifier can avoid mixing Reference Values (reference state) with Evidence (actual state).
See {{-rats-endorsements}}.
Re-asserted Evidence using RVP authority is said to be "corroborated".

#### Endorsed Values Triple {#sec-comid-triple-endval}

[^issue] https://github.com/ietf-rats-wg/draft-ietf-rats-corim/issues/310
An Endorsed Values triple provides additional Endorsements for an existing Target Environment.
For Endorsed Values Claims, the _subject_ is a Target Environment, the _object_ contains Endorsement Claims for the Environment, and the _predicate_ defines semantics for how the _object_ relates to the _subject_.
Copy link
Collaborator

Choose a reason for hiding this comment

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

unclear why _ is used here (and not in the ref-value triple explainer)

@@ -1151,7 +1156,8 @@ The new entry is added to the existing set of entries using the Endorser's autho

#### Conditional Endorsement Triple {#sec-comid-triple-cond-endors}

[^issue] https://github.com/ietf-rats-wg/draft-ietf-rats-corim/issues/310
A Conditional Endorsement Triple declares one or more conditions that, once they match, cause every entry in the endorsements to be added to the accepted state.
Copy link
Collaborator

@thomas-fossati thomas-fossati Nov 13, 2024

Choose a reason for hiding this comment

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

have we already defined "accepted state" at this point? If not, I suggest using a more intelligible term (e.g., actual state, as per I-D.ietf-rats-endorsements)


The series object is an array of `conditional-series-record` that has both Reference and Endorsed Values.
Each conditional-series-record record is evaluated in the order it appears in the series array.
The Endorsed Values are accepted if the series condition in a `conditional-series-record` matches the ACS.
Copy link
Collaborator

Choose a reason for hiding this comment

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

This one too seems too deep too soon: reference to internals, mention of ACS.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We can change ACS to attester's actual state

Copy link
Collaborator Author

@yogeshbdeshpande yogeshbdeshpande Nov 13, 2024

Choose a reason for hiding this comment

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

The triple is hard to explain with out the context of ACS, The matching semantics needs ACS. Few alternatives below:

  1. Forward reference to ACS and request reader to understand ACS before arriving here!
  2. Change ACS to attester's actual state. Tried but does not improve the situation dramatically, infact matching on attester's actual state yield more ambIguity!

Comment on lines 1185 to 1186
The first `conditional-series-record` that successfully matches an ACS Entry terminates the matching and the corresponding Endorsed Values are accepted.
If none of the series conditions match an ACS Entry, the triple is not matched, and no Endorsed values are accepted.
Copy link
Collaborator

Choose a reason for hiding this comment

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

ditto

@@ -1151,7 +1156,8 @@ The new entry is added to the existing set of entries using the Endorser's autho

#### Conditional Endorsement Triple {#sec-comid-triple-cond-endors}

[^issue] https://github.com/ietf-rats-wg/draft-ietf-rats-corim/issues/310
A Conditional Endorsement Triple declares one or more conditions that, once they match, cause every entry in the endorsements to be added to the accepted state.
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Suggested change
A Conditional Endorsement Triple declares one or more conditions that, once they match, cause every entry in the endorsements to be added to the accepted state.
A Conditional Endorsement Triple declares one or more conditions that, once they match, cause every entry in the endorsements to be added to the actual state.

Copy link
Collaborator Author

@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.

I have addressed most of the comments!

Copy link
Collaborator Author

@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.

I added my own comments to the earlier comments!

Copy link
Collaborator

@nedmsmith nedmsmith left a comment

Choose a reason for hiding this comment

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

multiple comments

draft-ietf-rats-corim.md Outdated Show resolved Hide resolved
draft-ietf-rats-corim.md Outdated Show resolved Hide resolved
draft-ietf-rats-corim.md Outdated Show resolved Hide resolved
yogeshbdeshpande and others added 8 commits November 20, 2024 15:22
Fixes #310

Signed-off-by: Yogesh Deshpande <[email protected]>
Add suggestions from Ned and Dionna

Co-authored-by: Dionna Amalie Glaze <[email protected]>
Co-authored-by: Ned Smith <[email protected]>
Apply simple edits that improve the language of the draft!

Co-authored-by: Thomas Fossati <[email protected]>
Co-authored-by: Dionna Amalie Glaze <[email protected]>
Co-authored-by: Dionna Amalie Glaze <[email protected]>
Co-authored-by: Thomas Fossati <[email protected]>
@yogeshbdeshpande yogeshbdeshpande merged commit 50960a3 into main Nov 20, 2024
2 checks passed
@yogeshbdeshpande yogeshbdeshpande deleted the triples-explaination branch November 20, 2024 15:23
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.

Better Initial description for the Triples Description
5 participants