-
-
Notifications
You must be signed in to change notification settings - Fork 50
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
20200916 token policy domains #456
Conversation
0c88a13
to
6c46235
Compare
6c46235
to
77fdcb9
Compare
ed41a2f
to
b8bd45a
Compare
b8bd45a
to
92421b8
Compare
c4c6bb8
to
8c37b2e
Compare
closed accidentally during master branch rename |
0cee6ca
to
c5789bb
Compare
c5789bb
to
abc6ecd
Compare
97262d4
to
4b5ca43
Compare
4b5ca43
to
0f7cd74
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
some questions, but this lgtm
## can't create policy for other user's domain | ||
data = dict(domain=self.other_domain.name, perm_dyndns=True, perm_rrsets=True) | ||
response = self.client.create_policy(self.token, using=self.token_manage, data=data) | ||
self.assertStatus(response, status.HTTP_400_BAD_REQUEST) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this leaking information?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, the error is that the domain does not exist in the user's domain set, and the serializer turns it into 400 (instead of 404) because it's only about a domain reference from a payload field (and not a URL resource).
The same error is returned when the domain does not exist at all, i.e. the error does not expose whether the domain is owned by another user. Further, you can always check whether a domain is owned by another user by checking whether deSEC nameservers have SOA records.
If you have a token that is restricted to example.com
, you can try to work on another domain's RRsets (of the same user), and you will get 403 instead of 400 or 404. That exposes to the token user that the other domain is in the same account. However, the token user could also just GET domains/
and look at the list of domains, so there's no additional exposure. (Unsafe methods on domains/
require a token without a domain policy, i.e. an unrestricted token.) One can argue that a domain-scoped token should not be able to access the domain list. However, if we block that, then the 403 side-channel occurs.
Do you think it's fine that a restricted token can read the list of domains in the account?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's fine, but I also don't see what could be the benefit. So maybe remove that permission?
a66706e
to
2df698e
Compare
As far as I can tell, the last remaining question is whether a scoped token should have the permission to list all domains belonging to user account. |
As discussed in person, we'll keep it as is for now, and may later implement something like |
816bd64
to
5ff138b
Compare
260382f
to
50edce9
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
endpoint reference is missing
662c1dd
to
66e32ab
Compare
No description provided.