On 2017-08-04 23:41, Jim Schaad wrote:
As promised I finally got finished with this review.
Thank you for your very thorough review Jim. Comments inline (note that
there are a few questions as well).
/Ludwig
1. Need to decide if /token, /introspect and /authz-info are under
/.well-defined or not. If they are then this needs to be noted and there
needs to be an IANA action if this has not already been done for OAuth.
I've added an issue to our issue tracker on this. However I would think
that this is something that would need to apply to OAuth as well.
2. Add some of the actor terms to section 2- such as Resource Owner
Ok, will do
3. Still unhappy about the lack of POP between C and AS. How does the AS
know that the key it is binding to the token is the right one. The RS has
no problems, but assumes that the AS did its job.
I'm not sure what problem that would solve. That C binds the token to a
key it doesn't own? What benefit would C have from that?
If C wants to relay a token to a "friend" it could always share the
pop-key with said friend.
4. If I want to use the SCOPES field as defined by Carsten, then I do not
want the current restriction that is being imposed on the scope parameter in
Section 3 (?) Under "Scopes and Permissions".
I'm just reiterating the definition of the scope parameter from the
OAuth spec. I think Carsten's approach can be modified to fit into that
spec.
5. In section 3.2 - s/so- called// - don't be pejorative.
Ok, I'll rephrase that
6. In section 4 - 2nd paragraph before figure 1 - I want the ACE profile to
provide one method to do this. There may be others that are specific to the
profile however.
I've already made that change in the "insertDiscovery" branch
(https://github.com/LudwigSeitz/ace-oauth/tree/insertDiscovery).
7. In section 4 - Under "ACE Profiles" - RECOMMENDS seems to be an odd
thing to have here - this is not really a protocol recommendation is it?
Are we allowing for JSON to be used rather than CBOR? The text here on what
encodings to use could be made more declarative. Are we expecting any JSON
use at this point for any profile define? If so then a list of tradeoffs
for profile creators and how to detect would be in order rather than the big
RECOMMENDS.
You mean section 5 right?
Yes the intention is to allow profiles to specify JSON (or something
else) as data format instead of CBOR.
8. Section 5.1 - Awkward language "If the client should to act"
Yeah that whole section needed a review. Fixed.
9. Section 5.3 - Is this really what you want to say is that one SHOULD
authenticate C to the AS? I think this is a MUST. I question if this is
really something that profiles should do rather than this document. I
thought that the profiles were targeted at the C <-> RS conversation.
My intention for the profiles is that they specify the communication
(and communication security protocols). This MUST cover C <-> RS and MAY
include AS <-> *any*.
I agree there should be a default fallback for the MAY parts. Adding issue.
10. Section 5.4 appears to create a new endpoint that has not been
discussed in other contexts. Is this intentional?
No that's just an existing OAuth endpoint. Since I've had mostly on
client credentials in mind, this endpoint wasn't discussed that much.
My impression is that this endpoint would mostly be used by
non-constrained clients (towards the obviously non-constrained AS), and
therefore it wouldn't need an ACE profiling, it could just be used as
specified in OAuth.
11. Section 5.5.1 - OK - I am reversing thinking. However I need to get a
summary of what a profile is going to need to specify a long ways before I
get to this point. Perhaps has part of the overview.
The summary of what a profile needs to specify is collected in Appendix
C currently. This appendix reiterates points that are spread out (in
contextually appropriate places) in the main body of the spec.
One of the questions
is going to be can the C-AS section of one profile be used with the C-RS of
another profile and keep the same security guarantees. The overview is
going to need to talk about this at some point.
This seems to be a "it depends" issue. We should have some text on this,
the security considerations seem the right place to me for this.
Creating issue.
12. Section 5.5.1 - I would like to add some additional parameters at this
point. The first is a RS_Data field which is copied from the RS->C message
verbatim. It allows for encrypted data to be passed from the RS to the AS
as part of the request. Data type is COSE_Encrypt.
Are you referring to the "nonce" that can be part of the AS discovery
message? I'm not sure if that should go in this document or in a
separate draft. What does the group think?
13. Section 5.5.1 - I would like to see an AS field documented here so th
that the 4 corner model can work.
Could you be more specific about the "4 corner model"? I'm not familiar
with that.
.
14. Section 5.5.1 - What is the default - use this for "aud"? How is a
client supposed to know what to put here for a new RS?
"aud" should contain be the identifier of the RS or a group of RS that
the client wants to access.
Since the client ought to know which RS it wants to access with the
token, I assumed it would know what to put in this field.
15. Section 5.5.1 - on cnf - Use "It is RECOMMENDED that an AS reject a
request with a symmetric key" as this is positive and where enforcement is
done.
Ok, fixed.
16. Figure 3 - This example no longer looks correct. the content type
should be the same as for Figure 2. That is assuming it is really CoAP and
not HTTP?
Right, fixed.
17. Figure 4 - need to check out if client_secret is a real secret or not -
it seems odd to say don't use symmetric and then have an example of passing
one. Could be something else as I don't have RFC 6749 w/ me.
True, this method is specified on section 2.3.1. of RFC 6749, so we
thought we'd offer an example.
I don't think this is a good feature security-wise.
18. The set of operations should start with the C->RS response structure
from DTLS profile as this is really the first thing that happens.
Sounds reasonable, fixed in the discovery branch. Merge pending approval
of my co-authors.
19. Is there a reason for a client not being able to request a profile
rather than having it be configured into the AS?
We had that in a previous version and decided it was over-engineered.
The client needs to be registered at the AS anyways, and could then also
specify preferred profiles.
20. Section 5.5.2 - You are not being constant about encoding - this is
saying MUST be CBOR while earlier it was only a recommendation,
Right, I'm adding an issue about this. We're also inconsistent in
section 5.5.1. although not in RFC 2119 language.
21. Section 5.5.2 - Post figure 5 it says examples - but only one example
exists.
Fixed.
22. Section 5.5.4.2 - Please make the abbreviations mandatory not optional.
I don't really want to support both.
I'll add an issue, I'm not opposed to that change, but I'd like to have
consensus.
(side note: Congratulations, you just contributed to creating the 100th
issue on this draft on github)
23. Section 5.5.4.4 - Please define profile as being part of a CWT so that
it can be communicated to the RS in the event that more than one profile is
supported. Optional if RS only does one.
Wouldn't the RS be able to determine the profile from the message it
receives from the Client?
Say the AS tells the client to use DTLS, the RS would notice receiving a
DTLS handshake. Are there any scenarios where two profiles could be
confused?
24. Section 5.5.4.5 - I don't know if it would not be cleaner to have a cnf
that is only for the client key info and an rs_cnf (defined for
introspection) that is for dealing with the RS keysWe need to specify that the
. That makes it cleaner
when looking a the case of an AS->C or an AS->R->C message as the fields
will be the same.
Indeed I argued the same way, but I was voted down by my co-authors.
I'll raise the issue again with your newfound support.
25. Section 5.5.4.5 - Figure 9 does not really make sense by itself. It
needs to have some context because it looks the same as for a RS key.
Your point makes sense, but this whole section is going to be rewritten
when we transfer cnf to draft-jones-ace-cwt-proof-of-possession. I'll
take that into account when we do the rewrite.
26. Section 5.5.4.5 - Figure 10 is not formatted correctly as the map
wrapping is missing on unprotected.
Correct, this figure is also bound to disappear, but I'll see that
draft-jones gets the right example instead.
Look @ section 3.3. RFC 6749 about scopes
That look more like a note to self to me, right?
27. Need to know how to think about the idea of a client doing an
introspection. Is the response going to be different than a RS doing the
query? I assume that the distinction would be based on the authentication
and internal knowledge - does not need to be documented except for response
differences.
You mean if a client would do introspection on an access token it
received? Depending on the AS its not sure it would be authorized to do
so (mine has policies for determining who gets to introspect). I believe
this can be left to the implementers. I might be convinced that it needs
a security consideration though (if you insist).We need to specify that the
28. Figure 14 needs to be updated
Done.
29. Section 5.6.2 - Need to add rs_UZWab0RScnf - if more than one key need to
tell
the RS what key to use.
I agree. Issue created
30. Section 5.6.4 - I think this is a required item if introspection is
going to be able to return a random symmetric key. Not needed otherwise.
It could also be used to inform the client about the RS's public key for
authentication.
31. Section 5.6.4 - last para - may want to state that this secret is
established at the same time that the token is established.
Sounds reasonable. Fixed.
32. Section 5.6.4 - establish MTI algorithms here?
For encrypting the client token? Is that really necessary? The AS should
know what algorithms the client supports.
33. Section 5.7 - I think that you need to add rs_cnf in the event of an RS
w/ multiple keys so it knows which to use. usage would be optional.
Agree. I'm extending the existing issue from 31.
34. Section 5.7 - last paragraph - should be JOSE and COSE not JSON and
CBOR.
Right, fixed.
35. Section 5.7.1 - Why would the RS respond with the cti - given a lack of
text it is not clear when the MAY would be triggered.
It's just a suggestion to the implementer, not really necessary but
useful in some scenarios (e.g. if the client later wants to delete or
overwrite the token this could be good to have).
36. Section 5.7.1 - Not sure how much information is being leaked by having
different error codes being returned in these situations. May only want to
have one code.
This is a difficult one. One the one hand you are right some information
is leaked, but on the other hand, withholding that information makes it
very hard for the client to fix erroneous access tokens.
37. Section 5.7.1 - under what circumstances is the introspection request
not made?
If the RS is not configured to do so. E.g. if it has intermittent
connectivity and the token is self contained.
If the introspection request is done async how is the client
token communicated back to the client? Would that be done as part of a
later access. Seems to be dicey.
It's not designed to be done async. We should mention that. Issue created.
38. Random thought - Is there a requirement that the same method be used
for both posting to /authz-info and to the resource? Could one use OSCOAP
for one and DTLS for the other? Question is because we are profiling and
the assumption is that profiles are whole.
I'm assuming that /authz-info can be unprotected. It could use DTLS
without client authentication, but it cannot use OSCOAP with a client
that is not previously known, since the token sent to /authz-info
contains necessary information to generate the OSCOAP context.
39. Section 5.7.2 - Last bullet - should note that tokens need to be shared
to the RS - AS may issue lots of tokens but if RS gets none then no
expirations
I think that bullet is the best we can do in the described usecase. The
last bullet refers to a RS that has not clock at all and no connectivity
to the AS. If the RS had connectivity it could do introspection.
If you have a better suggestion I'm open for suggestions.
40. Section 5.7.2 - Another "long running request" might be the case of
sending back an ACK for a request followed by a 4.01 because the token
expires before the request has finished processing. Re-doing introspection
might also cause this sequence of messages
Do you think it would be useful to give these additional examples?
41. Section 6 - personal preference - remove the first sentence it is not
useful.
I agree, I've heard this more than once now.
42. Section 6 - Protection is from signature or MAC - but we are using AEAD
for most of these. Should probably have a security consideration that AEAD
is preferred to MAC in most cases because of confidentiality as well.
Agree, fixed.
43. Section 6- the phrase 'long-term key shared with RS' implies to me that
this is a symmetric key. May want different language to allow for people
thinking of asymmetric keys as well.
Fixed.
44. Section 6 - Please explain to me how targeting multiple RS with an
asymmetric key is a problem - change it form shared secret to symmetric key.
Yes that sounds like something that was meant to apply for symmetric
keys, which is NOT RECOMMENDED anyways some paragraphs above. Fixed.
45. Section 6- I think you want to use a different term than token replay
in this paragraph. If a RS is rebooted and loses all token information,
there is nothing wrong with a C doing a replay of the token to the RS to get
access to the resources as long as it is still valid.
I reworded the whole paragraph.
46. Section 6 - Is the confidentiality protection a requirement for
asymmetric keys as well? -- Oh - and it may not be a session key, the
session key for DTLS is established later, it is just a POP value (or key if
asymmetric)
That issue relates to your point made in 9. I'll update the issue to
consider this as well. I also replaced "session key" with
"proof-of-possession key".
47. Section 6 - Sentence structure of this paragraph is difficult. The AS
does not use shorter key sizes, it will perhaps create shorter POP keys.
However, even this may be a problem depending on how short they are. A
tradeoff is going to occur here with the ability to record and later decrypt
keys depending how short the keys are.
I've tried to rephrase that paragraph to clarify it. Please check again.
47. Section 6 - the next to last paragraph is a repeat - but probably
clearer.
Fixed: I deleted the other paragraph.
48. Section 6 - Please explain the reasoning behind the last paragraph. It
does not make a great deal of sense to me.
If you issue an access token bound to a symmetric pop-key that is valid
for a group of RS, then any of these RS that receives the token can use
to towards the other RS, impersonating the client.
I'll try to clarify the text in the draft (I also noted that the context
to using a symmetric pop key got lost in the second sentence of that
paragraph).
49. Section 7 - I don't understand what you are trying to say with the last
sentence in the second paragraph. Given that the AS still needs to do ACL
control, this does not make sense to me.
In other grants, you have the authorization performed by the resource
owner and the AS just validates the grant provided by the client. These
grants can be used to hide the client's identity towards the AS.
I'll reformulate to clarify.
For now I skipped IANA considerations and the appendixes.
Noted, those are probably bound to change a bit when we extract cnf.
Let me know if you would prefer that I place these items into the issue list
on github or if you prefer doing it.
Jim
I'm creating issues as I go, thanks for the offer.
--
Ludwig Seitz, PhD
Security Lab, RISE SICS
Phone +46(0)70-349 92 51
_______________________________________________
Ace mailing list
Ace@ietf.org
https://www.ietf.org/mailman/listinfo/ace
