- 
                Notifications
    You must be signed in to change notification settings 
- Fork 15
Clarify need to validate the signature on signed subject tokens #253
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
|  | ||
| * MUST validate the requesting workload client authentication and determine if that workload is authorized to obtain the Txn-Tokens with the requested values. The authorization policy for determining such issuance is out of scope for this specification. | ||
| * Next, the Transaction Token Service MUST validate the `subject_token`, including verifying the signature, if it is signed. | ||
| * The Txn-Token Service determines the value to specify as the `sub` of the Txn-Token and MUST ensure the `sub` value is unique within the Trust Domain defined by the `aud` claim. | 
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.
If workloads are going to be obtaining transaction tokens, and sub represents the workload "class" then it won't be unique. Is it ok to drive workload use cases to use an instance identifier for the workload rather than the "class" identifier?
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.
@gffletch - it is up to the transaction token service to determine the "sub" claim. The way in which that is interpreted is domain specific.
| * The Transaction Token Service MUST set the `txn` claim to a unique ID specific to this transaction. | ||
| * The Transaction Token Service MAY set the `iss` claim of the Txn-Token to a value defining the entity that signed the Txn-Token. This claim MUST be omitted if not set. | ||
| * The Transaction Token Service MUST evaluate the value specified in the `scope` parameter of the request to determine the `purp` claim of the issued Txn-Token. | ||
| * If a `request_context` parameter is present in the Txn-Token Request, the data SHOULD be added to the `rctx` object of the Txn-Token. In addition, the Transaction Token Service SHOULD add the authenticated requesting workload identifier in the `rctx` object as the `req_wl` claim. | 
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.
Is adding the requesting workload a SHOULD because we don't want to be too prescriptive? My preference would be to make this a MUST.
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.
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 had a different opinion of the rctx content in the TraT. I feel the TTS should take the request_context into consideration, but really use its own logic to arrive at the content of the rctx field in the TraT. So, I'd rather be silent in the spec rather than use either "SHOULD" or "MUST" regarding this.
My logic is that the requester may send it a number of different things in the request_context, and the TTS may decide to use only a subset of it, or may need to process the values for internal consumption before putting it into rctx.
Thoughts?
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.
@tulshi, @gffletch This PR was about making it clear that the signature should be verified - the rest was formatting...
However, if we want to discuss rctx content (which is not the subject of this PR, but clearly a valid thing to discuss), perhaps we can merge this and open a separate issue and discuss request_context and tctx there instead?
My preference is to contain the PR to the issue it was meant to address (clarifying that the signatture needs to be verified).
Co-authored-by: George Fletcher <[email protected]>
Co-authored-by: George Fletcher <[email protected]>
See #214