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

V-09 #25

Merged
merged 76 commits into from Mar 2, 2020
Merged

V-09 #25

merged 76 commits into from Mar 2, 2020

Conversation

fpalombini
Copy link
Contributor

@fpalombini fpalombini commented Jan 28, 2020

Preparing for v-09 following Ben's review.

Copy link
Contributor

@kaduk kaduk left a comment

Generally looks good; just a few comments inline

draft-ietf-ace-oscore-profile.xml Outdated Show resolved Hide resolved
</t>

<t>
The use of random nonces during the exchange prevents the reuse of AEAD nonces and keys with different messages, in case of re-derivation of the Security Context both for Clients and Resource Servers from an old non-expired access token, e.g. in case of re-boot of either the client or RS. In fact, by using random nonces as part of the Master Salt, the request to the authz-info endpoint posting the same token results in a different Security Context, since Master Secret, Sender ID and Recipient ID are the same but Master Salt is different. Therefore, the main requirement for the nonces is that they have a good amount of randomness. If random nonces were not used, a node re-using a non-expired old token would be susceptible to on-path attackers provoking the creation of OSCORE messages using old AEAD keys and nonces.
Finally, the client sends a request protected with OSCORE to the RS. If the request verifies, the server stores the complete Security Context state that is ready for use in protecting messages, and uses it in the response, and in further communications with the client, until token expiration. This Security Context is discarded when a token (whether the same or different) is used to successfully derive a new Security Context.
Copy link
Contributor

@kaduk kaduk Feb 25, 2020

Choose a reason for hiding this comment

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

Do we want "used to successfully derive a new Security Context for the same client"?

Copy link
Contributor Author

@fpalombini fpalombini Feb 25, 2020

Choose a reason for hiding this comment

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

Oh I had missed that, added "for that client".

draft-ietf-ace-oscore-profile.xml Outdated Show resolved Hide resolved
</t>

<t>
We assume in this document that a resource is associated to one single AS, which makes it possible to assume unique identifiers for each client requesting a particular resource to a RS. If this is not the case, collisions of identifiers may appear in the RS, in which case the RS needs to have a mechanism in place to disambiguate identifiers or mitigate their effect.
We assume in this document that a resource is associated to one single AS, which makes it possible for the AS to enforce uniqueness of identifiers for each client requesting a particular resource to a RS. If this is not the case, collisions of identifiers may occur at the RS, in which case the RS needs to have a mechanism in place to disambiguate identifiers or mitigate the effect of the collisions.
<!-- TODO: Add error response definition in the document for the RS to indicate "Id collision"-->
Copy link
Contributor

@kaduk kaduk Feb 25, 2020

Choose a reason for hiding this comment

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

Is this TODO still active? :)

Copy link
Contributor Author

@fpalombini fpalombini Feb 25, 2020

Choose a reason for hiding this comment

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

Possibly, yes. I agree on removing the RS-assigned id (pending WG interim), but as Jim pointed out in his mail, there might be more cases where this might be useful. One of the agenda points for the interim.

@@ -477,7 +489,7 @@ A5 # map(5)
"exp" : "1360289224",
"scope" : "temperature_h",
"cnf" : {
"kid" : b64'qA'
"kid" : h'43814100'
Copy link
Contributor

@kaduk kaduk Feb 25, 2020

Choose a reason for hiding this comment

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

Any reason why we're changing the (binary) kid value?

Copy link
Contributor Author

@fpalombini fpalombini Feb 25, 2020

Choose a reason for hiding this comment

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

Yes, when I moved from b64 to h'' I realized that the kid format did not match the CDDL definition: bstr wrapped CBOR array of one element (in this example only clientId is sent). Sorry, forgot to point that out in my response!

Copy link
Contributor

@kaduk kaduk Feb 25, 2020

Choose a reason for hiding this comment

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

How broad of a scope do we need to do that in? (There were some h'00' and h'01' floating around somewhere but I forget in what context.)

@@ -493,7 +505,7 @@ A5 # map(5)
<section anchor="oscore-sec-ctx" title="OSCORE_Security_Context Object">

<t>
An OSCORE_Security_Context is an object that represents part or all of an OSCORE Security Context (Section 3.1 of <xref target="I-D.ietf-core-object-security"/>). The OSCORE_Security_Context object can either be encoded as JSON object or as CBOR map. In both cases, the set of common parameters that can appear in an OSCORE_Security_Context object can be found in the IANA "OSCORE Security Context Parameters" registry (Section <xref target="sec-ctx-params-reg"/>) and is defined below.
An OSCORE_Security_Context is an object that represents part or all of an OSCORE Security Context, i.e., the local set of information elements necessary to carry out the cryptographic operations in OSCORE (Section 3.1 of <xref target="RFC8613"/>). Conversely, the OSCORE_Security_Context object is defined to be serialized and transported between nodes, as specified by this document, but can also be used in this way by other specifications if needed. The OSCORE_Security_Context object can either be encoded as a JSON object or as a CBOR map. The set of common parameters that can appear in an OSCORE_Security_Context object can be found in the IANA "OSCORE Security Context Parameters" registry (<xref target="sec-ctx-params-reg"/>), defined for extensibility, and is specified below.
Copy link
Contributor

@kaduk kaduk Feb 25, 2020

Choose a reason for hiding this comment

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

I'm not sure that "Conversely" is the right conjunction here. Maybe "additionally" or "in particular"?

Copy link
Contributor Author

@fpalombini fpalombini Feb 25, 2020

Choose a reason for hiding this comment

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

Replaced with "In particular".

with a 4.03 (Forbidden). If RS has an access token for the client
but it does not cover the action that was requested on the resource,
RS MUST reject the request with a 4.05 (Method Not Allowed).
The response code must be 4.01 (Unauthorized) in case the client has a valid token associated with that Security Context, but the Security Context has not been used before, as the proof-of-possession in this profile is performed by both parties verifying that they have established the same Security Context.
Copy link
Contributor

@kaduk kaduk Feb 25, 2020

Choose a reason for hiding this comment

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

I'm not 100% sure I follow what's going on here anymore where we talk about the security context "has not been used before", and why we are only mentioning 4.01 and not the 4.03 and 4.05 that were in the previous text. Sorry for my bad memory :(

Copy link
Contributor Author

@fpalombini fpalombini Feb 25, 2020

Choose a reason for hiding this comment

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

No worries, totally understandable, it took me a while :)

So the "has not been used before" was my attempt at clarifying point 77. :

  1. The response code MUST be 4.01 (Unauthorized) in case the client has
    not used the Security Context associated with the access token, or if
    RS has no valid access token for the client. If RS has an access

The causality here seems a bit weird; how would the RS know what token was
intended just from an OSCORE request that's using a random or different
security context?

FP: The RS needs to keep this association between Security Context and token. This first section covers the case where the client is using a sec ctx not associated to any token or where that sec ctx was not used before (so the full OSCORE setup did not go through). Different (“valid”) security context associated to a token is covered in the next section. Is there anything I am missing here?

I think I'm just complaining about the wording: if a request comes in using
some other (wrong) security context, how does the RS know what the relevant
"the access token" is for that request?

And removing 4.03 and 4.05 was for removing duplicated framework text (commit 6dc9f4d), as you suggested.

Copy link
Contributor

@kaduk kaduk Feb 25, 2020

Choose a reason for hiding this comment

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

Looking at the old text (in the -08) more carefully, I think the "has not used the Security Context associated with the token" is more in the sense of "used some other, different, security context". The new text with "has not been used before" doesn't give me that same impression.

kaduk
kaduk approved these changes Feb 28, 2020
Copy link
Contributor

@kaduk kaduk left a comment

Looks good, modulo potentially more 'kid' value changes if needed.

@fpalombini fpalombini merged commit 8722ba6 into master Mar 2, 2020
2 checks passed
@fpalombini fpalombini deleted the v-09 branch Mar 2, 2020
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.

None yet

2 participants