Secure-by-default APIs

[Frédéric Buclin]

Le 24. 10. 12 17:41, Stephanie Daugherty a écrit :
>> I would like to suggest that this is an anti-pattern. We should instead be
>> creating APIs which are "secure by default", i.e. they return only data the
>> currently-logged-in user is permitted to see.

This is what we do already! The Bugzilla API is the WebServices
interface, not internal methods used by Bugzilla itself. This internal
part is not an API. Our API (WebServices) is secure by default.

If you read the documentation related to the bug you are talking about
correctly, it says "Returns *all* products of the classification." This
is very clear and unambiguous.

I will veto the kind of changes you have in mind. We already have very
explicit methods like $user->get_accessible_products to return exactly
what you are asking for. If some methods need more detailed
documentation, then fix the documentation. Renaming methods used for
ages is just going to break existing extensions and confuse developers
used to them.


>> straw man, the pattern that e.g. "object.things" be the secure version, and
>> "object.all_things" be the insecure version.

I don't want all_foo() methods appearing when our documentation is
explicit enough. Moreover, your naming isn't better at all. When sending
data by email (bugmails, flagmails, whinemails), the credentials of the
logged in user is irrelevant. What matters are credentials of the one
getting the email. We are not going to add a 3rd method
object.things_for_user_going_to_get_the_email.


>> seems to me that sticking with the current system is going to lead to
>> people - both Bugzilla devs and those making custom templates for their own
>> systems - to create information leak bugs again and again. People cannot be
>> expected to know that when you call object.things, you then need to
>> manually remove all things which are in group 23 (or whatever) before
>> displaying the list to the user.

I disagree. They are supposed to know. Methods are documented. If you
want a safe interface, use our WebServices interface. But even in this
case, your statement may fail if one user is getting data for another
user with different credentials.


>> Comments?

I think I have been very explicit above. :)


>> This sounds like very much the right approach.

I disagree totally!


> ie, hypothetically, get_all_bugs() might become
> unsantitized_get_all_bugs(), and get_all_bugs() would be replaced by a stub
> that logs a debug message so as not to break existing code.

This doesn't make sense. get_all_bugs() means get *ALL* bugs.


LpSolit