-
Notifications
You must be signed in to change notification settings - Fork 7
Allow optional claims-list #436
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
base: main
Are you sure you want to change the base?
Conversation
Addresses issue #435 where `claims-list` can be optional. `authorized-by` is added into the `condition` since `claims-list` is optional.
Introduces a specialized measurement-map with optional mval.
Co-authored-by: Dionna Amalie Glaze <[email protected]>
Co-authored-by: Dionna Amalie Glaze <[email protected]>
draft-ietf-rats-corim.md
Outdated
@@ -1368,12 +1368,16 @@ The Conditional Endorsement Series Triple has the following structure: | |||
The `conditional-endorsement-series-triple-record` has the following parameters: | |||
|
|||
* `condition`: Search criteria that locates Evidence, corroborated Evidence, or Endorsements. | |||
The condition is a record containing an `environment`, optional `claims-list`, and optional `authorized-by`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@nedmsmith This text is not aligned with what CDDL renders itself to.
In CDDL each claim is having an Authority which seems incorrect.
To me each of the condition: for each of the environment
should have the authority
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@nedmsmith This text is not aligned with what CDDL renders itself to. In CDDL each claim is having an Authority
Can you point to the CDDL that you think is not aligned?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The text here at line 1371 reads:
The condition is a record containing an
environment, optional
claims-list, and optional
authorized-by.
While CDDL says
condition: [
environment: environment-map
? claims-list: [ + measurement-series-map ]
]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
condition record does not contain authorised by, the claims list contains authorized by
please correct your wording
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@nedmsmith please see my comments above
condition: [ | ||
environment: environment-map | ||
? claims-list: [ + measurement-series-map ] | ||
] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where is the authorized-by on line 5?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In a stateful-environment-record authorized-by is buried inside of measurement-map. In order to specify authorized-by in the query, you also have to query on at least one measurement (mval is not optional).
Therefore I can't form a query like:
select * where environment=E1 AND authority=A1
I'm forced into queries like:
select * where environment=E1 AND authority=A1 AND measurement=M1
measurement-series-map = non-empty<{ | ||
? &(mkey: 0) => $measured-element-type-choice | ||
? &(mval: 1) => measurement-values-map | ||
? &(authorized-by: 2) => [ + $crypto-key-type-choice ] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do you need authorised by on each claim ? This is not clear..
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Currently, authorized-by is within measurement-map. Measurement-series-map is no different from measurement-map in that regard. The only difference is mval is now optional. This allows authorized-by to be populated even when there are not measurement values (which was the main point of the PR).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please have a look at the comments!
draft-ietf-rats-corim.md
Outdated
@@ -1368,12 +1368,16 @@ The Conditional Endorsement Series Triple has the following structure: | |||
The `conditional-endorsement-series-triple-record` has the following parameters: | |||
|
|||
* `condition`: Search criteria that locates Evidence, corroborated Evidence, or Endorsements. | |||
The condition is a record containing an `environment`, optional `claims-list`, and optional `authorized-by`. | |||
If both `authorized-by` and `claims-list`.`authorized-by` are populated then `claims-list`.`authorized-by` is ignored. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't this a change in the security behavior of authorized-by
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't this a change in the security behavior of
authorized-by
?
No, because in the previous case it wasn't possible to create a query of the form:
Case A: select * where environment=E1 AND authority=A1
However, queries of the form:
Case B: select * where environment=E1 AND authority=A1 AND measurement=M1
are also possible. However, because the triples are expressed as records/arrays [ ]
- a decision made early in the lifetime of corim, it is not possible to add authorized-by directly into the triple (it is neither subject or object).
The compromise approach is authorized-by appears in multiple places and if both are used (Case B above), then the scope of authorized-by needs to be clarified. Since the previous semantics were that authorized-by in measurement-map applies to environment-map, there was no way to bifurcate authority. The proposed wording preserves that convention by ignoring one of them. The alternative would be a security change which allows bifurcation of authority.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would like to think about the security consequences of this PR before accepting it.
@nedmsmith : We want to make progress about this prior to IETF, so please work on this, time permitting, we would like to CLOSE this by next week!!! |
Cleaned up errant and ambiguous text.
ignore .includes.mk
Fixed wording to match cddl.
I believe this is ready to merge. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Addresses issue #435 where
claims-list
can be optional.authorized-by
is added into thecondition
sinceclaims-list
is optional.