Early Review of draft-ietf-gnap-resource-servers-07
review-ietf-gnap-resource-servers-07-secdir-early-melnikov-2024-07-31-00
Request | Review of | draft-ietf-gnap-resource-servers |
---|---|---|
Requested revision | No specific revision (document currently at 09) | |
Type | Early Review | |
Team | Security Area Directorate (secdir) | |
Deadline | 2024-07-02 | |
Requested | 2024-06-11 | |
Requested by | Deb Cooley | |
Authors | Justin Richer , Fabien Imbault | |
I-D last updated | 2024-07-31 | |
Completed reviews |
Artart Early review of -05
by Rich Salz
(diff)
Intdir Early review of -05 by Tommy Pauly (diff) Secdir Early review of -07 by Alexey Melnikov (diff) Genart Early review of -06 by Lars Eggert (diff) Secdir Last Call review of -08 by Alexey Melnikov (diff) Tsvart Last Call review of -08 by Martin Duke (diff) |
|
Assignment | Reviewer | Alexey Melnikov |
State | Completed | |
Request | Early review on draft-ietf-gnap-resource-servers by Security Area Directorate Assigned | |
Posted at | https://mailarchive.ietf.org/arch/msg/secdir/7r1yFJQbX9WW0p9_MRFhMrj1mlw | |
Reviewed revision | 07 (document currently at 09) | |
Result | Has nits | |
Completed | 2024-07-31 |
review-ietf-gnap-resource-servers-07-secdir-early-melnikov-2024-07-31-00
I have reviewed this document as part of the security directorate's ongoing effort to review all IETF documents being processed by the IESG. These comments were written primarily for the benefit of the security area directors. Document editors and WG chairs should treat these comments just like any other early review comments. The document is well-written and seems basically in a good shape. I have some comments which are not purely editorial, but I don't think they would be hard to address. The document has extensive Security and Privacy Considerations, which seem to cover all things I can think of. Some specific comments on some of them below. 1) General comment: I don't expect anything to be done about this, but I think GNAP still has too much optionality and not enough mandatory to implement features. However creation of relevant registries is a big improvement. More specific comments, most of them are minor: 2) In several places in IANA Considerations: Terminology inconsistencies: claim vs parameter vs token. I suspect some of them are cut & paste issues, but it is hard to be sure. Also names or values of some registered fields are not properly constrained. Lack of such constraints affect interoperability and security. In particular: 5.3. GNAP Token Formats Registry 5.3.1. Registry Template Name The name of the format. Is there some formal definition of token format names? Do they allow spaces? Can I use non alpha-numeric characters? Are non ASCII (e.g. Unicode) characters allowed in them? 5.4. GNAP Token Introspection Request Registry 5.4.1. Registry Template Name The name of the claim. Type The JSON data type of the claim value. Reference The specification that defines the token. Did you mean "claim" in the last sentence? 5.5. GNAP Token Introspection Response Registry 5.5.1. Registry Template Name The name of the claim. Type The JSON data type of the claim value. Reference The specification that defines the token. Did you mean "claim" in the last sentence? 5.6. GNAP Resource Set Registration Request Parameters Registry This document defines a means to register a resource set for a GNAP AS, for which IANA is asked to create and maintain a new registry titled "GNAP Resource Set Registration Request Parameters". Initial values for this registry are given in Section 5.6.2. Future assignments and modifications to existing assignment are to be made through the Expert Review registration policy [RFC8126]. The Designated Expert (DE) is expected to ensure that: * all registrations follow the template presented in Section 5.6.1. * the parameter's definition is sufficiently orthogonal to other claims defined in the registry so as avoid overlapping s/claims/parameters ? functionality. * the parameter's definition specifies the syntax and semantics of the claim in sufficient detail to allow for the AS and RS to be s/claim/parameter ? able to communicate the resource set. 5.6.1. Registry Template Name The name of the parameter. Type The JSON data type of the parameter value. Reference The specification that defines the token. Did you mean "parameter" in the last sentence? 5.7. GNAP Resource Set Registration Response Parameters This document defines a means to register a resource set for a GNAP AS, for which IANA is asked to create and maintain a new registry titled "GNAP Resource Set Registration Response Parameters". Initial values for this registry are given in Section 5.7.2. Future assignments and modifications to existing assignment are to be made through the Expert Review registration policy [RFC8126]. The Designated Expert (DE) is expected to ensure that: * all registrations follow the template presented in Section 5.7.1. * the parameter's definition is sufficiently orthogonal to other claims defined in the registry so as avoid overlapping s/claims/parameters ? functionality. * the parameter's definition specifies the syntax and semantics of the claim in sufficient detail to allow for the AS and RS to be s/claim/parameter ? able to communicate the resource set. 5.7.1. Registry Template Name The name of the parameter. Type The JSON data type of the parameter value. Reference The specification that defines the token. Did you mean "parameter" in the last sentence? 5.8. GNAP RS-Facing Discovery Document Fields This document defines a means to for a GNAP AS to be discovered by a GNAP RS, for which IANA is asked to create and maintain a new registry titled "GNAP RS-Facing Discovery Document Fields". Initial values for this registry are given in Section 5.8.2. Future assignments and modifications to existing assignment are to be made through the Expert Review registration policy [RFC8126]. The Designated Expert (DE) is expected to ensure that: * all registrations follow the template presented in Section 5.8.1. * the claim's definition is sufficiently orthogonal to other claims s/claim/field ? s/claims/fields ? defined in the registry so as avoid overlapping functionality. * the claim's definition specifies the syntax and semantics of the claim in sufficient detail to allow for RS to be able to s/claim/field (twice)? communicate with the AS. 5.8.1. Registry Template Name The name of the parameter. Type The JSON data type of the parameter value. I think you mean "field" twice above? Reference The specification that defines the token. Did you mean "field" or "parameter" in the last sentence? 5.8.2. Initial Registry Contents The table below contains the initial contents of the GNAP RS-Facing Discovery Registry. +================================+==========+=============+ | Name | Type | Reference | +================================+==========+=============+ | introspection_endpoint | string | Section 3.1 | | | | of RFC xxxx | +--------------------------------+----------+-------------+ | token_formats_supported | array of | Section 3.1 | | | strings | of RFC xxxx | +--------------------------------+----------+-------------+ | resource_registration_endpoint | string | Section 3.1 | | | | of RFC xxxx | +--------------------------------+----------+-------------+ | grant_request_endpoint | string | Section 3.1 | | | | of RFC xxxx | +--------------------------------+----------+-------------+ | key_proofs_supported | array of | Section 3.1 | | | strings | of RFC xxxx | +--------------------------------+----------+-------------+ While you provide basic JSON types above, I think some of the fields register would benefit from ABNF or some other sort of formal definition of value format. Are "introspection_endpoint" and "resource_registration_endpoint" URIs? Are "token_formats_supported" values from the registry in 5.3? What about syntax of each individual "key_proofs_supported" element? 3) In 6.2: * Is presented using the appropriate key for the token (see also Section 6.4) Subject identification (the RS knows who authorized the token) Issuer restriction (the RS knows who created the token, including signing a structure or providing introspection to prove this) Are some comma(s) missing in the sentence above? In particular, if I remove (), this reads as "Is presented using the appropriate key for the token Subject identification Issuer restriction" 4) 6.5. Token Exfiltration Since the RS sees the token value, it is possible for a compromised RS to leak that value to an attacker. As such, the RS needs to protect token values as sensitive information and protect them from exfiltration. Any recommendation on how this can be done? This is especially problematic with bearer tokens and tokens bound to a shared key, since an RS has access to all information necessary to create a new, valid request using the token in question. Is this an obscure way of saying that bearer tokens must not be used? Best Regards, Alexey