Hi!
I performed an AD review on draft-ietf-oauth-dpop-11. Thanks for this
document. Comments below.
** The document has 6 listed authors. Could this rationale for this be
explained on the list and captured in the shepherd write-up.
** Section 2.
(CRIME,
BREACH, Heartbleed, and the Cloudflare parser bug are some examples). There
have also been numerous published token theft attacks on OAuth
implementations themselves
Good pointers. Could informative references be provided for them.
** Section 4.2.
alg: a digital signature algorithm identifier such as per
[RFC7518].
Shouldn't this text constraint the algorithm identifier as coming from a
registry pointed to in RFC7518 (i.e.,
https://www.iana.org/assignments/jose/jose.xhtml#web-signature-encryption-algorithms)?
** Section 4.1.
htm: The HTTP method of the request to which the JWT is attached,
as defined in [RFC9110].
Shouldn't the values in this field come from the registry HTTP methods registry:
https://www.iana.org/assignments/http-methods/http-methods.xhtml? RFC9110
specifies the semantics of the fields, not the values.
** Section 4.1. Shouldn't these text read like the description of htm:
OLD
The HTTP target URI (Section 7.1 of [RFC9110]), without query
and fragment parts.
NEW
The HTTP target URI (Section 7.1 of [RFC9110]), without query and fragment
parts of the request to which the JWT is attached.
** Section 4.2. Editorial.
OLD
But that it be a minimal subset of the HTTP data so as to
avoid the substantial difficulties inherent in attempting to
normalize HTTP messages.
NEW
This design approach of using only a minimal subset of the HTTP header data is
to avoid the substantial difficulties inherent in attempting to normalize HTTP
messages.
** Section 4.3. Editorial. Clarifying.
OLD
the header field value is a well-formed JWT,
NEW
the DPoP HTTP request header field value is a well-formed JWT,
** Section 4.3.
the alg JOSE header parameter indicates an asymmetric digital
signature algorithm, is not none, is supported by the application,
and is deemed secure,
-- check feedback already provided in Section 4.2 about this field pointing to
a value in a JOSE registry
-- "is deemed secure" seems open ended. Perhaps "and is acceptable per local
policy"
** Section 5. Normative language
OLD
The client must discard the response in this case,
NEW
The client MUST ...
** Section 5.1. Cite the relevant registry which would the source of JWS alg
values in the dpop_signing_alg_values_supported array
** Section 7
Binding the token value to the proof in this way prevents a
calculated proof to be used with multiple different access token
values across different requests.
What makes a proof "calculated"?
** Section 7.
its inclusion strongly
binds the access token value to the holder of the key used to
generate the signature.
Is "strongly binds" the same as "cryptographically binds"? "Strong" is used in
the next paragraph too. Please review it.
** Section 10.1. Editorial.
OLD
The dpop_jkt parameter can be used as described above to bind the
issued authorization code to a specific key.
NEW
The dpop_jkt parameter can be used as described in Section 10 to bind the
issued authorization code to a specific key.
** Section 10.1. Typo. s/distingush/distinguish/
** Section 11.1. Editorial. Recommend unpacking this dense sentence.
OLD
To prevent multiple uses of the same DPoP proof servers can store, in
the context of the target URI, the jti value of each DPoP proof for
the time window in which the respective DPoP proof JWT would be
accepted and decline HTTP requests to the same URI for which the jti
value has been seen before.
NEW
To prevent multiple uses of the same DPoP proof, servers can store, in the
context of the target URI, the jti value of each DPoP proof for the time
window in which the respective DPoP proof JWT would be accepted. HTTP
requests to the same URI for which the jti value has been seen before would be
declined.
** Section 11.1. Recommend unpacking this dense sentence.
OLD
Because clock skews between servers
and clients may be large, servers may choose to limit DPoP proof
lifetimes by using server-provided nonce values containing the time
at the server rather than comparing the client-supplied iat time to
the time at the server, yielding intended results even in the face of
arbitrarily large clock skews.
NEW
Because clock skews between servers
and clients may be large, servers MAY limit DPoP proof lifetimes by using
server-provided nonce values containing the time at the server rather than
comparing the client-supplied iat time to the time at the server. Nonces
created in this way yield the same result even in the face of arbitrarily large
clock skews.
** Section 11.1. Editorial.
OLD
If jti is enforced unique for
the lifetime of the nonce, there is no additional risk of token
replay.
NEW
As long as the jti value is unique for the lifetime of the nonce, there is no
additional risk of token
replay.
** Section 11.2. Editorial. Wouldn't it be more precise to mention that the
DPoP proofs are tied to endpoints
OLD
An attacker in control of the client can pre-generate DPoP proofs for
use arbitrarily far into the future by choosing the iat value in the
DPoP proof to be signed by the proof-of-possession key.
NEW
An attacker in control of the client can pre-generate DPoP proofs for specific
end points to use arbitrarily far into the future by choosing the iat value in
the DPoP proof to be signed by the proof-of-possession key.
** Section 11.5. In additional saying that the typ field should be checked,
perhaps add that the value it must be set to -- "dpop+jwt"
** Section 12.17.1. I understand the intent of the change. I am going to need
to consult with IANA on how to handle a modification to a registry entry for
which the IETF is not the change controller.
** Section 12.8. Why is the "Permanent Message Header Field Name" registry
being used instead of the Hypertext Transfer Protocol (HTTP) Field Name
Registry" at https://www.iana.org/assignments/http-fields/http-fields.xhtml.
See Section 5.1 of RFC9110.
** Section 11.8. I'd like to have a deeper conversation about the algorithm
agility for the three data elements - ath, jkt and dpop_jkt - which have SHA256
hard-coded.
Regardless if more deeper changes are made, dpop_jkt needs to be mentioned in
this section. Additionally, the guidance of how to use a different algorithm
would required a bit more text. In addition to specifying and registering the
claim, some document would also have to update this document to allow it's use
and the explain the new guidance for ath and jkt.
Stepping back, given that ath and jkt are being invented by this spec, what is
the argument for hardcoding the hash algorithm? Why not future-proof them now
by redefining ath/jkt to be a more complicated data structure with a hash
algorithm identifier + the hash value? What would be the penalty for doing
that? I do see a long list of implementations in the shepherd report.
Thanks,
Roman
_______________________________________________
OAuth mailing list
[email protected]
https://www.ietf.org/mailman/listinfo/oauth
