Thank you very much for your detailed feedback Vittorio! ᐧ On Tue, Dec 8, 2020 at 3:22 PM <vittorio.berto...@auth0.com> wrote:
> Dear authors, > > It took ages but I finally managed to go thru a full review of the current > OAuth2.1 draft. Apologies for the delay. > > Metacomments: > > - The VAST majority of the comments are suggestions for improving > clarity, mostly on historical language coming from 2.0 that I found myself > having to clarify to customers and colleagues over and over thru the years. > None of those are critical. > - There are a few places where 2.1 requires a MUST I believe to be > unwarranted/too restrictive. For each of those I did my best to provide > context and concrete examples. > - A sizeable category of comments and disagreements on MUST come from > treating mobile and desktop apps as largely equivalent under the “native > app” umbrella, despite of the vast gulf that separates the two both in > terms of security posture and user experience. Again, I tried to be as > matter of fact as possible in there. > - The main reason for which I spoke up during the IETF interim on > oauth2.1 was the confusion the omission f the implicit grant caused among > the devs using implicit in OIDC for obtaining ID_tokens. I suggested some > language to pre-empt the issue, but I expect some iteration there. > > Thanks, > > V > > > > §1 > > I wonder whether we should take the opportunity offered by OAuth2.1 to > clarify frequent points of confusion about OAuth, by explicitly calling out > in the introduction what is out of scope. > > For example: OAuth is not an identity protocol, as it doesn’t concern > itself with how resource owners are authenticated; OAuth isn’t meant to > address 1st party scenarios, although the reader is free to use it in > that context as well; and so on. > > I believe there is value in adding this in the introduction rather than > relegating it in some later considerations section, as the people who need > this information the most rarely read past this point. > > > > §1.1 > > In the RS definition, wondering whether including the word “API” would > help to clarify what an RS is in practice. > > > > §1.2 > > I always found this part extraordinarily difficult to decipher. I get that > this is the first description and doesn’t have to be exhaustive and > consider all cases (eg it’s ok if step 3 claims that the client > authenticates w the AS even tho that’s only for confidential clients), but > I think it could be much clearer than it is today. > > Step 1 says > > The client requests authorization from the resource owner. The > authorization request can be made directly to the resource owner (as > shown), or preferably indirectly via the authorization server as an > intermediary. > > Besides the fact that “requests authorization” is a bit vague, this step > and the corresponding diagram leg does not correspond at all to what > normally happens- to get a code, the client does need to hit the AS and the > mention in passing in the text isn’t enough to figure that out. Also, with > the omission of ROPG there really isn’t any way of asking anything to the > RO directly (the client creds doesn’t involve the RO). > > I would recommend updating that diagram to be more descriptive of the > canonical scenario. > > Step 2 > > mentions the 2 grants defined in the spec, but only one of them represents > the RO’s authorization. Claiming that the client itself is the RO is a > formalism that doesn’t meet the reader’s intuition at this point. > > Step 5 > > The language here triggered multiple discussions, in particular on whether > the AT can actually be used to ascertain the identity of the client – that > isn’t the case for public clients, for example; besides, that’s not really > the highest order bit of the AT. If it is, it seems that the spec should be > more explicit about how client identification from the RS by means of an AT > works. If it isn’t, perhaps we should change the language to omit > authenticate. > > The last paragraph is emblematic IMO – if the preferred method is very > different from the diagram here, and if the abstraction presented here is > not terribly useful (given that we no longer have multiple RO based grants, > excluding the extension grants that are still too far at this point to > warrant a cognitive downpayment for the reader) I wonder whether we’d be > better off doing the authz code diagram directly (and mention that we also > have the client creds grant separately). > > > > §1.3 > > I understand that we can’t really change this because we inherit from > OAuth 2 but I’ll mention it anyway- modeling clients as ROs is problematic, > as it doesn’t often match what happens in practice. A confidential client > might batch-read a user’s inbox searching for ad words, but the resource > owner remains the user. > > I know we straighten things up in 1.3.2, but the positioning here is > confusing. > > Also: isn’t the refresh token grant a core-specified grant as well? I know > I am nitpicking. > > §1.3.1 > > We don’t say anywhere here that the authorization code can be exchanged > for an access token. It can be somewhat inferred from 1.2, but it’s a bit > of an intelligence test (one needs to infer from authorization grant). > > P2 > > “obtains authorization” could be more specific, to reinforce that we are > doing a delegated flow. “Obtains” seem to suggest that we are talking about > consent, rather than AS side rules. If that’s the case, calling it out > might make the scenario clearer. > > P3 > > Both the benefits listed apply to confidential clients only. Not sure > whether calling it out here would help prevent confusion later on (eg > people thinking that public clients can prove their identity) or would > bring confusion on now (given that we didn’t differentiate between client > types yet). Either ways, formally we are OK here; I am just thinking how to > make things clearer. Perhaps defining client types before grants might help > being clearer here. > > > > §1.3.2 > > A concrete example of credential (eg shared secret) might help clarify > things here. Also, the fact that client credentials indicate both a grant > in itself and an artifact (which participates in other grants) is a well > know source of confusion. Wondering if calling this out here might help. > > > > §1.4 > > In general, we use “access token” and “token” interchangeably- perhaps > pedantic, but I would suggest we always use “access token” to prevent > confusion with refresh tokens later on, and other token types in other > contexts (eg think ID tokens). > > P1 > > The client should treat the AT string as opaque, but that doesn’t > necessarily means it is: in some cases the client CAN see inside the token, > and with the current language they might interpret it as “in this case, > it’s OK to look- otherwise they would have made it opaque, per the spec”. > > > > §1.5 > > The first phrase of P1 is wonderfully clear. We should have the equivalent > in §1.3.1 > > Not having defined a mechanism for requesting a RT here, leaving it to > ASes to decide when and where, created the situation in which some AS only > issues RTs when they get the offline_access scope, with all the unfortunate > consequences about RT lifetime vs session lifetime etc… I know we can’t > really change this now as we don’t want to break existing AS > implementations, but wondering if there’s anything we can say to further > clarify/give readers a headsup about the ambiguity/diversity of behaviors > they’ll encounter here. > > P2 > > It’s odd that we say “usually opaque to the client” for the RT while we > decisively said opaque for the AT. Also, the client shouln’t do anything w > the RT content hence I think the same considerations done for §1.4P2 apply > here. > > “The token denotes an identifier used to retrieve the authorization > information” gets into the specifics of the implementation and it’s not > universally true (some AS encrypt/sign the authz info in the RT itself and > have no server state whatsoever. > > Step 3 > > Should we add a reference to RFC6750 here? > > > > §1.8 > > Should we say rich **delegated** authorization framework? > > > > §2 > > “end-user interaction with an HTML registration form” is oddly specific 😊 in > particular, I think “end user” might be misleading. We can either say > “interactive” or refer “the client app developer” or equivalent. > > Overkill but I’ll mention it anyway. Should we say that typically the client > registration in the non-dynamic scenario occurs in authenticated settings? > Not strictly necessary but might help the reader to tie what we say in this > section with their concrete experience. > > > > §2.1 > > P4 > > “Authorization servers SHOULD consider the level of confidence in a client's > identity when deciding whether they allow such a client access to more > critical functions, such as the Client Credentials grant type.” > > I don’t understand this sentence. Is the client credentials grant type a > “more critical function”? Or is it a level of confidence? Either ways, I > think it needs clarifying. > > P5 > > IMPORTANT: this is going to break many OAuth implementations with significant > adoption. Auth0 is fine (each client_id is tied to a single client type) but > I know of others that will break. > > I suggest softening to a SHOULD NOT. > > “browser-based application” > > I am not convinced this is so much easier than the original > “user-agent-based”. I understand the advantages (dovetails w the BCP, more > precise given that apps can be user agent as wells nowadays, more familiar) > however the break w 2.0 terminology is jarring. I don’t feel very strongly > about it but enough to type it. > > > > §2.2 > > It’s a bit odd to define the client identifier like it’s something brand new > when §2.1 already introduced it. This language from the original 2.0 might > need to be revised to accommodate that change. > > Wondering whether a warning against structured client_ids (eg identifiers > assembled thru some string template, like developer name+region+serial) would > be in order. Perhaps in the security considerations? > > > > §2.3 > > P1 > > That sounds vaguely circular, given that being assigned credentials might be > considered part of the “establish a client authentication method” task listed > there. I’d simply say “if the client is confidential or credentialed”. > > P2 > > I’d add “by the authorization server” for good measure. > > P3 > > That sounds vague. Shouldn’t it be mandatory for the AS to require client > auth for the client types who have creds? “if possible” seem to open the > possibility of circumstances where that’s not he case. > > P5 > > I think that this idea of identifying the client will need to be fleshed out > more for people to fully understand it. Credentialed clients can prove that > they are the same client instance across multiple transactions, which some > might consider a weak form of identification. To rule that out, it has to be > mentioned upfront IMO. If not here, in some of the considerations section… > with a forward reference here. > > P6 > > Do we say why anywhere? If yes, we should reference it. If not, perhaps we > should. > > > > §2.3.1 > > We no longer mentioned the empty client secret, but we don’t forbid it > either. What’s our stance? > > > > §2.3.2 > > In §2.3 we mention MTLS, private_key_jwt, but here we just point the reader > to IANA. It looks like echoing those methods here might help clarity. > > > > §3.1 > > Last paragraph > > I have been in discussions where readers interpreted this as “you cannot send > custom parameters to the authorization server”. To preempt that mistake, we > mighr consider calling out that custom extensions _are_ permitted as long as > the AS supports them. I know that’s what the current language says already. > > > > §3.1.1 > > Wondering if referring to some specific, well known extensions (like OIDC) > might help readers to better understand this point. > > > > §3.1.2 > > RFC3986 6.2.1 talks about character by character comparison, but doesn’t > mention case sensitivity. I am sure it does elsewhere in the spec, but for > clarify and readability I recommend specifying the desired behavior directly > here. > > > > §3.1.2.1 > > Personally, I would advocate for a MUST here. True, lots of people won’t > comply at development time, but I think that’s OK as long as they do use TLS > when going in production. > > Also, SameSite changes are making the use of HTTPS at dev time more and more > common. If OAuth2.1 is about picking the best of the security practices, this > seems like a an obvious candidate. > > > > §3.1.2.2 > > P3 > > “lack of requiring” doesn’t sound proper. > > > > §3.2 > > P2 > > Should we also say that the spec doesn’t care about _when_ the client obtains > the endpoint? > > Last P > > Same considerations as §3.1 > > > > §3.2.1 > > P1 > > That’s stricter than §2.3P3 – I think the language there should be tweaked to > be coherent with the one here. > > > > §3.3 > > Wondering if the “scope strings order does not matter” point should be > somehow emphasized or clarified. I know of implementations who considered > heuristics such as “if the scopes requested correspond to multiple resources, > I’ll show consent for all byt the token eventually issued when redeeming the > code will have as audience the resource corresponding to the FIRST requested > scope”, which would violate the order invariant requirement. > > > > §4 > > Potentially VERY confusing. I would recommend to be more specific and state > that “OAuth *2.1* defines two grant types”. > > > > §4.1 > > Diagram > > Not critical. But I want to point it out. The first time I saw this diagram I > found it confusing. The fact that the same numeral is assigned to multiple > legs is just odd for anyone not already familiar with the flow, possibly > still struggling to understand the client as a service side component. > > Also, now that we have mighty SVG support, I would strongly advocate for a > modern version of this diagram (there lines perhaps don’t need to be broken > into segments). > > Step 5 > > “optionally, a refresh token” is too vague IMO. I will look for opportunities > to clarify later in the spec, given that this might not be the best place to > go in details. > > > > §4.1.1 > > Overall: a high summary of the steps in this preamble might help. The current > denormalization in subsection can be pretty hard to follow for someone seeing > this for the first time. > > Also: creating challenge and verifier BEFORE assembling the request seems > profoundly counterintuitive to me, as it emphasizes a security measure over > the core function of this leg of the flow. Unless there’s a crypto reason for > this current sequencing that I can’t see, I recommend first creating the core > request (what’s now 4.1.1.3) and then attaching challenge and verifier. Also, > sending the message can be its own subsection rathe than being conflated with > the last message composition subsection. > > P1 > > “to begin” remains a bit suspended, given that there’s no obvious segue on > what constitutes the steps after the beginning. > > P2 > > “later use with the authorization code” could be clearer, e.g. “at > authorization code redemption time”. At this point that might still not be > obvious for the reader. > > Mentioning the provenance of properties (parameters?) code_challenge and > code_verifier without first having introduced them might confuse people not > already familiar with them and the request process in general, as their > function will not be obvious not naturally map with the preceding sentencer. > > P3 > > Imposing a MUST before knowing what those this are yet is not as clear as it > would be if this would be stated after their use and function has been > explained. > > > > §4.1.1.3 > > On state. Given the change vs OAuth2, I think it might be helpful to call out > the relevant section on the appendix about differences to help people > familiar w 2.0 not to miss this important change and avoid doing work twice. > > > > §4.1.2 > > P2 > > Should we say that the code should be opaque to the client, to discourage the > use of structured code templates that can be partially manufactured? > > P8 > > “he server MUST NOT include the "code_challenge" value in client requests”, > was that meant to be “responses”? > > Qualifying “other entities” (anyone but the AS?) might make this point > clearer. > > > > §4.3 > > We mentioned extension grants in passing, but I don’t recall seeing a > definition/description of their function in the context of the framework. > Even a short sentence to that effect here would help, given that the section > title names them explicitly. Also, stressing that the device flow is just one > example and other extensions might differ (for example in their logic to > establish whether an access token request is valid and authorized) would go a > long way in helping the reader put this section in better focus. > > > > §5.1 > > On the access_token parameter. Given the discussions we had for the JWT AT > profile draft, I am wondering whether it should be called out here that the > AT recipient is the RS, that the client should not expect to be able to parse > the access_token, and that the AS is under no obligation to use a consistent > AT encoding outside of what is negotiated with the RS. I don’t feel very > strongly about this, or about where in the spec this should be called out, > but it sure would have made life easier in those discussions- hence the > comment. > > On the refresh_token parameter. The lack of details in how OAuth2 describes > how/when an AS returns refresh tokens led to today’s complicated situation in > which many implementations issue RTs only when OIDC’s offline_access is > received in the scopes, as it was the only mention in public specs describing > a concrete behavior. See the associated online_access discussion on the OIDC > list, as RTs gain importance as session artifacts of sort for SPAs now that > implicit is dead and ITP makes iframe renewals problematic. > > Unfortunately it is too late to be prescriptive here, as we cannot break > compatibility with whatever choices existing AS implementations made. However > we can be more descriptive and give the reader a better idea of what’s the > range of possibilities. Some nonnormative examples of how existing AS > determine whether to issue an RT or not (eg as an option determined at client > registration time, or any other heuristic you guys encountered in the wild) > might help people to better understand their options and the intent of the > specification here. > > §5.2 > > It might help to remind the reader here that extensions to the core spec > might specify or further specialize circumstances in which the errors > mentioned here are returned (for example, see the validation errors in the > JWT AT profile). There’s a mention of that in §7.3.1 but that’s pretty far, > and having even brief language here might be handy for people reading the > spec for reference rather than cover to cover. > > > > §6 > > P1 > > I think the risk assessment is just one of the factors an AS might use to > decide whether to issue an RT or not. The current language suggests risk is > the only determinant in that decision and that doesn’t seem right. > > Saying that one might refresh tokens using other grants seems odd. A new > authorization code grant gets me a new token and offers me the opportunity to > describe what token I want (scopes etc), the fact that I might choose to ask > the exact same things I asked in the original request is expediency. I would > rather phrase this as the fact that the client can simply repeat the original > request, and external factors such as cookies, sessions and other auth method > specific options may allow the client to do so without prompting the user. > > P2 > > We might need to be more precise here. Do we mean the scopes consented by the > RO in the request that led to the issuance of the RT being used? Just saying > consented by the RO for the client does not exclude cases in which there are > more instances of the client in operation. Say that I am running uber on > phone 1 and I consent to read my google calendar, getting AT1 and RT1. Say > that on phone 2 I also run the uber app, and this time I consent to write my > google calendar, obtaining AT2 and RT2 on this new device. Now consider the > various combinations here. Should RT2 allow me to get calendar:read too, > given that it was already consented by RO for this client? Should RT1 allow > me to get AT1’ containing calendar:write, given that RO consented for it when > using a different instance of the same client? Whatever is the answer you > want to the questions above, I think the spec should have language clear > enough to unambiguously determine the desired behavior. > > > > §6.1 > > It’s a bit confusing that half of the RT use requirements is in §6 (the > requirement to authenticate confidential/credentialed clients) and half is in > here, with the only differentiator being the nature of the client. This is > pretty minor, but I think I would personally find clearer if all the > requirements for the use of an RT would be consolidated in a single place. > It’s true that the public client reqs are a SHOULD, but still. > > Rotation. > > Wondering whether it would be wise to advise the reader to have their AS > revoke all the still valid ATs issued by the AS from the same session/family > of RTs upon detection of a RT reuse. It is not uncommon for clients to > request new ATs before their projected expiration. > > P6 > > I think the “MAY” here might be confusing when applied to the rotation, as in > either the AS does it, or the scenario won’t work. I understand this is > formally correct, but perhaps explicitly calling out some cases in which the > AS might decide to do otherwise and acknowledge that in that case the client > will be stuck might help clarify. Also, if the public client protection > measures were in §6 instead of here, there would be less opportunities for > confusion as it would be easier to grok that this doesn’t apply to the > rotation case only (now adjacent) but to other RT reissuance cases as well > (eg sliding expiration). > > On the identical scopes requirement. Say that after obtaining RT1, which > includes scopes s1 and s2 for client c1, the RO revokes authorization for c1 > to use s2. Should the AS fail the RT redemption, or return an AT with only s1 > and a scopes parameter informing the client of the change? As developer I > would prefer the latter, to preserve the experience: but if we are adamant > about the current language, I think it might be useful to explicitly call out > that any changes to the grant on the AS side should result in failure of the > RT redemption. > > P7 > > Calling out deprovisioning of RO might be useful as well. > > On “the logout at the AS” event, that’s definitely a valid case but I worry > about how presenting that alone might reinforce misunderstandings that equate > RTs with sessions. There are certainly times where we want that (see mentions > earlier of the online_access discussions) but there are also cases where the > ability of a client to refresh ATs needs to survive session boundaries > (offline_access) and confusing the two are problematic. I don’t have a clear > solution here, just pointing out a potential point of confusion. Maybe there > will be more opportunities to clarify later in the spec. > > > > §7 > > P1 > > An important case to call out is the AS-RS colocation, where neither > introspection nor token format agreements are necessary. I suggest mentioning > it openly. > > > > §7.2 > > “bearer tokens may be extended to include proof-of-possession techniques by > other specifications” sounds like an oxymoron. Wouldn’t PoP make the token no > longer bearer by the very definition above? > > It looks like we might need a term for simply “token”. > > > > §7.2.1 > > Do we also want to forbid two tokens in the same request, using different > methods? Current language only constraints the behavior of one token. > > > > §7.2.3 > > The “insufficient_scope” description here is problematic. The privileges the > AT carries/points to are not necessarily (or exclusively) represented by the > included scopes (eg the RO might have granted document:read to the client, > but RO might have no privileges for the particular document being requested > in this particular call). It might be useful to specify that “invalid_scope” > should be used for authorization errors that can be actually expressed in > terms of delegated authorization, leaving to RS implementers the freedom to > handle other authorization issues (eg user privileges, RBAC, etc) with a > different error code. Or at least, we should be clear that authorization > logic not expressed via scopes is out of scope (pun not intended) for this > specification. > > Note, this isn’t an abstract problem: there are SDKs out there that use > “invalid_scope” for every permission issues. Very confusing. > > > > §7.3.1 > > We rewrite portions of 6750 in oauth2.1, but here we refer to it as its own > spec… that could be confusing as it’s unclear which parts of 6750 are > overridden by oauth2.1 (eg no more querystring) and what parts remain > normative. Perhaps we can call those things out in the sections meant to > replace the corresponding sections in 6750. > > > > §7.4.2 > > Pedantic: although the title of the section states it, wondering whether > every instance of “token” here should be “access token” instead. Think of > cases in which the spec is quoted in discussions and disputes, where snippets > can be pasted and mentioned outside of the context of this section. > > P2 > > Referencing the JWT AT profile as an example of extension providing the info > out of scope for the core might help the reader grok a concrete example. > > > > §7.4.3.5 > > “one hour or less” seems very arbitrary, and breaks step in respect to what > the spec does elsewhere (eg we don’t give any indication of how long an AS > should wait to invalidate an RT for inactivity, but we do say the AS should > do so). I would actually not provide any reference value here. > > > > §7.4.3.6 > > Another opportunity of referencing the JWT AT profile for a concrete example > of detailed audience restriction guidance in ATs. > > > > §7.4.3.7 > > Besides the indications given to clients here, should we also give guidance > to an RS to ignore tokens passed that way? > > > > §7.4.5 > > Along the same lines of the comments about delegated authorization earlier > for §7.2.3. I think it would be useful to acknowledge here that ATs might > carry, and RSs might expect, authorization information that go beyond the > delegated authorization for 3rd party API case that is core to OAuth- and > remind the reader that those mechanisms are out of scope for oauth hence they > shouldn’t expect those aspects to be addressed/handled/regulated by this > specification. > > > > §8 > > As mentioned earlier, it seems a potentially confusing to reference the > section of a document being superseded. I do see an issue in redefining here > something already established and in use, hence I am not expecting this to > change. Just wondering whether we need to provide a more explicit map of the > sections in 6749 that are being updated by oauth2.1. > > > > §9 > > Should we also say something about the scenario being chiefly 3rd party > clients? We know lots of people use OAuth2 for 1st party scenarios and some > considerations might differ. This might be an opportunity to finally make > that clear. > > > > §9.1 > > P1 > > Well, the AS doesn’t really use client auth... the active part here is the > client itself. Perhaps the AS can require it, when possible. > > P2 > > Unclear. Are we saying that if it is possible to safely distribute keys to > client, the AS MUST use client auth? That seems odd, there might be other > reasons coming into play (cost, security posture) making that choice not > viable. Or is the intent to say that the AS should not use client auth if the > key distribution cannot be trusted? That sounds far more realistic, but then > the language should be tweaked or the reader might pick the former > interpretation. > > P3 > > The AS can’t really PREVENT creds forwarding as the RO machine might still > have funny business going on (eg DNS attacks). Some softer language might be > more accurate there. > > P4 > > That sounds very abstract. What does it mean? That the AS should consider to > issuing RTs to public clients? If that’s the case, we should just say so… tho > without more details, I don’t know how actionable the guidance here will be. > I can see BoFa requiring a user to reauth in their iPhone app after > inactivity, but I don’t see Uber doing so for their app.. unless they produce > long lived ATs, which isn’t what we want either. > > P5 > > This could be clearer. The dynamic client registration case just flesh out > the confidence level an AS can have in its identity, but does not offer a > corresponding privilege level to it – whereas the second case does mention > assigned privileges explicitly. Also, pitting “dynamically registered client” > vs “web application” might suggest the app type is a factor, whereas AFAIK > 7591 can be used for registering web apps too (whether that s wise or not is > immaterial here). > > §9.2 > > Should we say something about whether native clients should be allowed to be > “upgraded” as confidential clients in the future, or the other way round? I > know of scenarios where people did that to preserve consent info, but that > seem sketchy security wise. > > P3 > > The SHOULD here refers to a requirement that in 10.3.1 is a MUST. I don’t > think the MUST is warranted (more about that in the 10.3.1 comments) but if > we do keep it, it looks like the level should be coherent here. > > P4 > > That example is compatible with a SHOULD- works now, but would look odd if > we’d upgrade P3 with a MUST for coherence with 10.3.1. > > P5 > > Inclusion where/how? We should be precise IMO. If it’s just registration > material (eg not part of the redirect URI), we should mention how we expect > it to be used in the context of OAuth- and if we don’t know, perhaps we > should not mention it here. > > > > §9.3 > > P1 > > I know it becomes clearer later on, but I think it would help here to > explicitly call out confidential and credentialed client as the subject of > this sentence. Those are the only client types with credentials, hence the > current language is formally correct. This is just for clarity. > > P2 > > I thought we require redirect URI registration in all cases? This makes it > sound it’s only for public clients. > > P3 > > I have been in various discussions where people were attempting to interpret > what “explicit RO authentication” means in practice. Is it a full credential > prompt regardless of whether one session already exists? A selection between > existing sessions, if present? > > P4 > > This is unclear. As it currently reads it seems to prohibit things like > getting a new authz code silently via iframe (and prompt=none or equivalent > UX suppressing mechanism, please ignore the ITP complications for the sake of > argument). > > > > §9.3.1 > > P1 > > I don’t follow this sentence. The client identity cannot be proved for public > clients (see also next P), and the native apps are public clients unless > otherwise specified (eg credentialed). > > P2 > > I find this misleading. Client side measures such as claimed schemes, domains > etc might work to prevent an app impersonating another app on the same > device/OS, but they aren’t guaranteed to be honored on other operating > systems. The AS has no way of knowing whether those measures have been > enforced on the client, hence it should not accept them as proof. > > > > §9.4.1 > > This is another place where a reference to the JWT AT profile would provide a > concrete example of the conditions set forth here (eg RS guidance for > audience validation). > > Also, as mentioned earlier, it might be useful to remind the reader that the > AS might include in the token authorization information that go beyond the > delegated authz scenario OAuth2.x concerns itself with, and that those > aspects are beyond the scope of this specification. That would truly go a > long way preventing people from abusing and overextending the spec on > scenarios it is not meant to address. > > And even for the canonical scenarios, it might be useful to remind the reader > that the RS might have extra logic not described in this spec that determines > whether the call will be authorized or otherwise- to dispel the notion that > the AS is always the sole source of truth for authorization. > > > > §9.4.2 > > P2 > > Clarifying that MTLS is one instance of the sender constraint methods just > mentioned would prevent some readers considering that an independent, > additional constraint. > > > > §9.5 > > In §6.1 we give more details about protection, should we have a backward > reference to that section here? > > P2 > > Is it worth to specify that it doesn’t matter how the AS tracks the binding? > (eg server side or embedded in the RT bits themselves). > > > > §9.6 > > P1 > > This language makes the assumption that for client cred grants, the sub will > take on the client_id value. That it certainly possible, but not a given. As > such, the language here should be explicit abouty the fact that it’s what we > are expecting to happen in this particular scenario. > > P2 > > Referring to the client as the actor here is confusing as soon as you move > beyond client_id. When you talk about rhe sub or any other value, it’s not as > much the client as it is the developer who owns the client. The difference is > subtle but might be a source of confusion. > > §9.7 > > P3 > > I thought that we were going to make PKCE or nonce the only two mandatory > alternatives? Nothing against supporting state as an accepted way to achieve > this, just surprised as I recall people being quite adamant about pushing > PKCE. > > P4 > > Looks like the iss response parameter might make the distinct redirect URIs > unnecessary? > > > > §9.7.1 > > The guidance in this section isn’t likely to be widely followed, but I > understand the rationale behind it. > > > > §9.8 > > P2 > > That’s probably obvious, but I think we should specify that those multiple > attempts should come with the expected code verifier as well for the > revocation logic to be triggered- if it’s just the code without verifier, it > doesn’t look like the leak went far enough to warrant that intervention. > > P4 > > If this holds, then the remark about the use of state for CSRF in §9.7 P3 > seems unnecessary. > > P9 > > The use of MUST here seems incompatible with the concession of using nonce > instead of PKCE. Either we allow it or we don’t… > > §9.11 > > P1 > > Now that we no longer trade in RO passwords, should we mention them here? > Sure, if the AS uses passwords they do need protection, but so do lots of > other things the AS might use as part of auth that we don’t mention here for > Occam’s razor. > > P3 > > What other means are we referring to? > > > > §9.12 > > “service providers” occurs for the first time in the document here. Tying the > guidance to entities already mentioned in the doc (clients, AS) might make > things clearer/more actionable. > > > > §9.13 > > I understand that some of the comments in this area should be done on the BCP > rather than here, but here we are. That said… > > Blanket comment: this section seems to assume native app == mobile app, and > that’s not strictly the case. Desktop apps have different characteristics and > desktop OSes different capabilities. Would be clearer to make a distinction > when relying on mobile OS capabilitesas we appear to do here. > > P2 > > Embedded user-agent != fake external user agent, but P2 uses that > interchangeably. A desktop app might use an embedded browser for UX reasons > and making no attempt whatsoever to disguise that as an external agent. And I > would be pretty surprised to see a malware remover get rid of google drive > for Mac, or Adobe products, or Office- all native apps using embedded > user-agents for authentication. > > P3 > > It’s unclear how the AS would do that, given that user agent strings can be > faked. Google has its ML based secret sauce, but that might not be accessible > by everyone. > > P4 > > This is mobile specific, doesn’t apply to desktop apps as readily. Also > unclear how that would change for the end user, given that pixel perfect > replicas can easily include fake address bars. More details on how a user > would detect an actual browser (presence of an existing session, access to > bookmarks etc) might add enough color to help the reader truly understand the > extent of the remedy power (or its true limits). > > > > §9.14 > > There’s no action for the reader here. If that reflects the actual situation > (none of the roles described in this spec can do anything to mitigate or > contain the damage, and solutions to prevent the situation lie outside its > purview eg use MDM software on your corporate devices) we should explicitly > say so. > > > > §9.15 > > A bit odd that we used CSRF throughout the document assuming the reader was > familiar with it, but here we attempt a definition. > > Still confused on how we admit state as a valid mechanism, as opposed to > limiting to nonce and PKCE. Also note the potential discrepancy called out > earlier in which at code redemption time we appear to require PKCE, in > contrast to admitting nonce/state here. > > > > §9.18.2 > > Wondering if here we should go as far as recommending the AS keeps dynamic > client registration OFF when it’s not needed. That might provide good secure > by default guidance for AS SDKs and product developers. > > > > §9.19 > > I assume we’d add the “iss” response parameter as alternative to the > requirements here? > > > > §9.20 > > P1, P2, P3 > > The security properties described here do not apply as is to desktop apps > using embedded browsers. > > As mentioned earlier, on the desktop the separation between apps isn’t as > stark as on mobile- the keystrokes message pump is potentially accessible at > the user session level, encompassing multiple applications. Although process > separation mechanisms are in place, the circumstances of the execution and > the OS specific features in this respect determine the degree to which > entering credentials in one process makes the data inaccessible by another. > We should not claim advantages we cannot guarantee, and we should be explicit > about what measures specific to dekstops should be considered to mitigate > risks eg not executing apps as admins, enable and use UAC at the right level > on Windows or equivalent on other OSes, use app sandboxing when the OS > supports it, etc. Not suggesting we call out specific technologies like UAC, > but at least point at the category of security measures. > > P4 > > As mentioned above, the presence of the address bar can be easily faked by > pixel perfect replicas- relying on those is security theater. > > P5 > > This is a valid concern, but if we want to frame it in the context of the > security considerations section we need to add more color (eg minimizing the > situations in which one needs to enter creds is good security practice). > > > > Another thing we might add as lowlight for embedded browsers: password > managers might not work, making the user’s life difficult and possibly > promoting insecure situations (eg placing a random pwd in the clipboard, > where other apps might later steal it from). > > §9.21 > > Do we need this here, given that we cover this in depth earlier? > > > > §10 > > I find this subdivision confusing. We have native clients considerations > scattered throughout the entire specification, but now we have a dedicated > section that somewhat repeats some of the points already made while > interleaving new ones. Perhaps having a more specific section title, one that > better characterizes the content and intent of this section, would avoid > giving the impression that if the reader is interested in native clients, > this is the only section they need to read. > > P2 > > See discussion so far on native vs desktop. > > Also: I have been in ferocious discussions where people thought the external > user agent HAD to be the browser, which we know isn’t the case (hero apps > like the FB SDK or OS features like sign in w Apple work just as well). Some > language here giving a nod to those non-browser external user agents might > help clarify. > > P4 > > See previous discussion on mobile!=desktop. > > P5 > > I’d always qualify the “browser” with system, external, device etc given that > some reader calls the embedded user-agent “embedded browser”, hence just > saying “browser” without modifiers is ambiguous. > > User authentication state.. on the device. I think specifying it will clarify > what this really means. > > > > §10.1 > > Unless you scope this statement to MOBILE apps, I think the MUST here should > be a SHOULD. > > The security posture of the desktop is different enough, and the quality of > the user experience when using a system browser bad and disomogeneous enough, > that a MUST isn’t justified here. > > > > §10.2 > > Beware of the native-mobile false equivalence here. > > P3 > > One or two examples of non-browser external user agents might help. > > P4 > > Text pasted here is technically not a best practice but core. > > I see there are examples here, so no need to add them in P3. > > It’s unclear why the external browser is RECOMMENDED here- if it’s because we > can’t go in details of the behavior of non browser apps, it seems like we > should say that much rather than making a recommendation. In other words, > RECOMMENDED expresses a strong preference for it, but if I am on iOS and I > want to sign in with Facebook I am actually better off using their SDK both > for security and user experience reasons than to use the system browser. > > > > §10.3 > > Requesting a MUST for all 3 methods seems restrictive… why not requesting at > least one? > > > > §10.3.1 > > P3 > > This seems unnecessarily restrictive. 3.8 in 7595 is mostly SHOULDs, and it > refers to organizations rather than individuals. Not every developer owns a > domain, not every app goes on an app store and is meant for general public > consumption. An organization sideloading apps on managed devices should not > be forced to follow those constraints if they control the environment and > aren’t worried about other apps competing for the same schema, but placing a > MUST here might compel SDK developers to embed validation checks that would > make developers on those circumstances deal with complexity without any > security upside. In fact, apps should not even be required to have internet > access in the general case but this requirement does impose that. > > I think this is an important best practice that should be encouraged, > RECOMMENDED or even SHOULDed , but it shouldn’t be a MUST in the core > specification. > > > > §10.3.2 > > P4 > > Like everything happening on the client, the AS cannot really take that as > guarantee. What might be true for an app running on iOS might not apply if > the requests are all manufactured via cURL on a Linux box. The scope of those > measures is really limited to one particular device or devices sharing an OS, > outside of that cohort there are no guarantees. > > > > §10.3.3 > > This section should come with glaring warnings, anything actively listening > on the client extends the attack surface- an app listening on the loopback > might now get affected by exploits in the local HTTP driver/local network > stack and taken over, executing with the same privileges as the user > (remember > https://www.rapid7.com/db/vulnerabilities/WINDOWS-HOTFIX-MS15-034/). The fact > that the loopback is only accessible locally only reduces the risk but > doesn’t eliminate it, a local process with lower privileges might use that > exploit for elevation of privileges for example. We should at least recommend > that the app hosting the loopback adapter runs with low privileges, reiterate > the “listen only when you must” and generally warn about the extra attack > surface. > > As AS I might not want to support this method at all, both for security > reasons and for the extra logic the wildcard port entails at registration and > request serving times, but right now the spec forces me to- more reasons for > relaxing the MUST for all 3 methods as mentioned earlier.. > > > > §12 > > I am not exactly sure where to place the following- this section (or a > subsection) might be the best fit. > > As mentioned during the interim meeting, the omission of the implicit flow in > OAuth2.1 has already caused a lot of people to interpret this as an indirect > deprecation of the use of implicit flow by OpenID Connect for obtaining > ID_tokens, either via traditional response_modes or via form_post. > > We already debated and concluded that the reasons that led to the omission of > the implicit grant in 2.1 do not apply to ID_tokens, hence there’s no reason > for people to stop using OpenID Connect that way. > > Formally we are in the clear, as OIDC is vased on 2.0 the omission of > implicit here does not prevent it as extension grant in OIDC anyway- but the > formal stance doesn’t help in preventing the confusion and assuaging the > concerns of the developers who aren’t as well versed as the people on this > list in all things specifications. > > Because of this, I recommend we add some language here that prevents that > confusion. Something like > > > > * The Implicit grant ("response_type=token") is omitted from this > > specification as per Section 2.1.2 of[I-D.ietf-oauth-security-topics > <https://tools.ietf.org/html/draft-ietf-oauth-v2-1-00#ref-I-D.ietf-oauth-security-topics>] > > Please note: the omission of the implicit grant from this specification does > not automatically imply that other extension grants obtaining credentials > directly from the authorization endpoint should also be discarded. For > example, the implicit flow defined in Section 3.2 of [OIDC > <https://openid.net/specs/openid-connect-core-1_0.html#ImplicitFlowAuth>] > remains valid for all the response_type values not including “token”. > > > > Although it might be a bit unusual to refer to details of specs from other > entities, this spec already mentions OpenID 9 times even excluding §14.2- and > as soon as the browser apps section will be included, that number is certain > to rise. And the confusion on this point is truly widespread- adding language > along the lines of the above directly in the core would go a long way to save > a lot of grief. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >
_______________________________________________ OAuth mailing list OAuth@ietf.org https://www.ietf.org/mailman/listinfo/oauth
