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

Add warning text about empty permission sets #194

Closed
jricher opened this issue Sep 5, 2015 · 11 comments

Comments

Projects
None yet
2 participants
@jricher
Copy link

commented Sep 5, 2015

Suggest that we add the following warning text to §3.5.2 (Authorization assessment process):

The authorization server MUST NOT issue a token for a resource set for which
no active policies have been defined, either by the resource owner or by default
in the authorization server itself. In other words, in the absence of other, more
specific instructions, the authorization server MUST treat all resource sets as
inaccessible. Failure to do this can result in resources being made accessible to
any client or requesting party that asks, regardless of claims presented.
@jricher

This comment has been minimized.

Copy link
Author

commented Sep 5, 2015

Possibly add text to §2 indicating that the resource owner (or somebody) MUST set policies on the resources before they can be used.

@xmlgrrl xmlgrrl added core V1.0.1 and removed V1.0.1 labels Sep 5, 2015

@xmlgrrl

This comment has been minimized.

Copy link

commented Sep 7, 2015

There are a couple of things going on in these suggested text additions: One is requiring the authorization server to operate on a "default-deny" authorization basis, and the other is requiring positive action in order to configure policies that loosen this restriction.

Since we're using "policy" in this spec as shorthand for "the configuration parameters of an authorization server that effect resource access management", and we've put the setting of these parameters out of scope, it seems inappropriate to have any MUSTs around a process of "setting". (The only other instead of the phrase "set policies" in this spec is in 3.5.2, and it's in passing.) It could be an entirely passive process.

Similarly, I agree that default-deny is a good idea, but it's not up to us to mandate authorization policy strategy. (In fact, it's so widely agreed that default-deny is a good choice that I'm not aware of any modern systems that do the opposite.)

We could state that, in all likelihood given modern access control practices, resources put under AS protection are, well, protected from all access, barring some policy saying they're not. I'm not sure if saying this has any heft, though, because UMA is not (currently, anyway) in the business of prising open the AS/RO relationship.

@jricher

This comment has been minimized.

Copy link
Author

commented Sep 8, 2015

I would like to avoid developers making the same naive mistake that I did in implementation:

  • Resource sets have some set of claims that are required for access
  • Permission tickets have some set of claims that are supplied during the UMA process
  • If the supplied claims are equal to or greater than the required claims, issue a ticket

This was an obvious implementation to me and it works perfectly UNLESS the required claims are empty. I did not intend to have things be default-open, but when following the wording of the current spec to actual implementation I completely missed that corner case.

I don't care if the advice is normative or not, but it needs to be pointed out that a resource set with no policies set on it shouldn't be open to the world. The text above would have pointed out the corner case to me during implementation, I'm fairly certain.

Mandating or suggesting default-deny is good practice. Claiming that something is "so widely agreed" is dangerous territory for a spec, especially around a security issue like this. As a counter argument to that: it's also "so widely agreed" that you should use TLS, and yet we say all over the place that you have to.

Finally, you asked me to write up this issue when I first brought it up and now I am.

@xmlgrrl

This comment has been minimized.

Copy link

commented Sep 8, 2015

Oh, I see now. You're absolutely right that we did ask you to write it up! When we discussed it on the call, what I was imagining was more like a "health warning" about default-deny and an explanation of the corner case you discovered as an example of how an AS could get into trouble. Something like:

"It is widely considered proper practice for authorization assessment and policy decision-making to be conducted on a default-deny basis, that is, for all resources to be protected unless a policy is in place that unlocks this protection. It is RECOMMENDED to follow this practice unless an alternate model has been consciously chosen for a particular deployment scenario. Exercise caution in implementing default-deny in a "naive" manner; for example, it is insufficient simply to assume that all resource sets have some non-zero set of claims required for access, and then check that the number of supplied claims is greater than or equal to zero. [Optionally: See [UMA-Impl] for further discussion.]"

I'm reluctant to add normative requirements when they're entirely about internal server processes...

@jricher

This comment has been minimized.

Copy link
Author

commented Sep 8, 2015

But if the normative requirements prevent the authorization server from failing to fulfill its basic intended purpose and, as you say, it's "obvious anyway" to people trying to do the right thing, then it should not harm anyone. Anyone willingly running afoul of these requirements will have their own reasons for doing so, and they are likely well aware that they're going off the rails a bit.

Still, I don't care of it's actually normative or not. I just want a clear warning in the spec. If you lowercase all the normative words in my above statement then it should fit as well.

@xmlgrrl

This comment has been minimized.

Copy link

commented Sep 8, 2015

My challenge with your suggested wording is that it presumes a particular model of policy-setting that is "active" or has "instructions", and these are entirely out of band of UMA. We've defined policy fairly broadly as "The configuration parameters of an authorization server that effect resource access management." That could mean a lot of things: Getting individual policies from a policy store, having an admin turn settings on and off, having certain exceptions built in for a very specific deployment use case, etc. Let's see...

Okay, I can see using REQUIRE/MUST for default-deny. Even in really narrow ecosystem use cases (let's say UMA is used for a web-based file system behind a really secure firewall and you want to let everyone have "rwx" by default), I still don't think it's a good idea. :-) And in wide ecosystems, a resource owner will want to know that the AS works as it's supposed to, and there's some guarantee of the intent.

Then the challenge is to warn developers off of naive approaches to implementations. So, what if we strengthened my suggested wording to include a MUST?

"The authorization server MUST conduct all authorization assessment and policy decision-making on a default-deny basis, that is, to treat all registered resource sets as inaccessible by default unless explicitly made accessible through policy. Exercise caution in implementing default-deny in a "naive" manner; for example, it is insufficient simply to assume that all resource sets have some non-zero set of claims required for access, and then check that the number of supplied claims is greater than or equal to zero. [Optionally: See [UMA-Impl] for further discussion.]"

@jricher

This comment has been minimized.

Copy link
Author

commented Sep 8, 2015

With a couple of tweaks that works for me:

The authorization server MUST conduct all authorization assessment and policy decision-making on a 
default-deny basis. That is, it must treat all registered resource sets as inaccessible by default unless 
explicitly made accessible through policy. Exercise caution in implementing the default state of in a 
policy in a "naive" manner: for example, it is insufficient simply to assume that all resource sets have 
some non-zero set of claims required for access, and then accept an empty set of supplied claims as
sufficient for access. [Optionally: See [UMA-Impl] for further discussion.]
@xmlgrrl

This comment has been minimized.

Copy link

commented Sep 8, 2015

Hey @jricher: On the call that is still going on, this is the discussion and conclusion we reached; further comments?

Discussion: Is it proper to REQUIRE default-deny? Would adding “registered” to “resource set” sufficiently tweak Justin’s wording? Since saying anything about default-deny is perpetuating a certain trust model, and doesn’t tackle technical interop. However, in a wide ecosystem use case, Alice will tend to expect default-deny, and would be very disappointed if default-allow were the behavior. So maybe this comes down to our “…all parties need to document expected behaviors” language. Mike can see some fairly likely cases of default-permit in his environment. We do want to be sure it’s a conscious decision of the deployment and not a result of poor implementation. We concluded that default-deny on the AS side really does need to be required to give UMA as many teeth as possible.

Do we want to give this one example of a corner case? We have to assume there are others. Let’s shunt the entire discussion that is implementation-specific to the UIG.

“The authorization server MUST use a default-deny authorization assessment model in adding authorization data to RPTs, that is, “everything that is not expressly allowed is forbidden” for resource sets that resource servers have registered. Exercise caution in implementing default-deny because corner cases can inadvertently result in default-permit behavior; see [UMA-Impl] for further discussion.”

@jricher

This comment has been minimized.

Copy link
Author

commented Sep 8, 2015

If it's default permit because of an actual policy in place that's fine. If it's default permit because you didn't apply any policy, that's a problem.

@xmlgrrl

This comment has been minimized.

Copy link

commented Sep 8, 2015

The thinking among us talking here is that a "policy in place" (unless you mean something among humans) would be explicitly permitted access for whatever the resources are. So that still sounds like default-deny. Are we missing anything? And how would you change the wording if it doesn't suffice for you?

@xmlgrrl

This comment has been minimized.

Copy link

commented Sep 8, 2015

In UMA ad hoc telecon 2015-09-08 (which had quorum), this was the discussion and conclusion:

Change to:

“The authorization server MUST use a default-deny authorization assessment model in adding authorization data to RPTs, that is, “everything that is not expressly allowed is forbidden” for resource sets that resource servers have registered. Exercise caution in implementing default-deny because corner cases can inadvertently result in default-permit behavior. For example, it is insufficient simply to assume that all resource sets have some non-zero set of claims required for access, and then accept an empty set of supplied claims as sufficient for access. See [UMA-Impl] for further discussion.”

xmlgrrl added a commit that referenced this issue Sep 9, 2015

Completed #194
As agreed on 2015-09-08 call.

@xmlgrrl xmlgrrl closed this Sep 9, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.