Fwd: Possible security bug in incubator-airflow OAuth GitHub Enterprise auth

[Maxime Beauchemin <ma...@gmail.com>]

Anyone on this mailing list using GH authentication in Airflow? I received
an email from GH security about a concern about our integration (bellow).

I commented on the original commit and tagged the author and someone else
who touched the file here:
https://github.com/apache/incubator-airflow/commit/4796245be517aed06df21a85c93a2b86a7f31939

I'd love if someone using GH authentication could take the lead on this.

Thanks,

Max

---------- Forwarded message ----------
From: Patrick Toomey <@github.com>
Date: Wed, Aug 17, 2016 at 8:25 AM
Subject: Possible security bug in incubator-airflow OAuth GitHub Enterprise
auth
To: max


Hi,
  I am a member of the GitHub application security team and  happened to
recently run across https://github.com/apache/incubator-airflow, as someone
was looking at using it internally. While they were pulling together a
proof of concept, I noticed that they had included the
“github_enterprise_auth.py” file. While browsing through this file I
noticed something that stuck out. In particular, this line looks like it
could be a security issue: https://github.com/apache/incubator-airflow/blob/
821bdb5310c6c21cd9c0b9d0797873ff6114179d/airflow/contrib/
auth/backends/github_enterprise_auth.py#L159.  That lines appears to look
for a response from the “user teams” API for a team slug that matches one
from the “allowed_teams” configuration option. However, this slug is just a
string, and doesn’t convey any information about a specific team. For
example, an enterprise instance might create an organization “Acme” and
also create a team in this organization called “designers”. But, there
could also be another organization on that same Enterprise instance,
“Evil”, that also creates a team called “designers”. I haven’t spun up a
working instance of this integration, so my apologies if I missed some
other bit of logic that would prevent this kind of attack. But, in general,
when you do this kind of validation, it is better to use an identifier that
is guaranteed to be globally unique across the GitHub instance. In the case
of a GitHub Enterprise instance, that would likely be the ID of the team,
which is also returned by the same API: https://developer.github.
com/v3/orgs/teams/#list-user-teams.

Thanks,
Patrick

Re: Possible security bug in incubator-airflow OAuth GitHub Enterprise auth

[Bolke de Bruin <bd...@gmail.com>]

I think it is smart to have this in jira and tagged with security. This makes tracking it easier. I'll create the issue for it. 

B. 

Sent from my iPhone

> On 19 aug. 2016, at 07:53, Maxime Beauchemin <ma...@gmail.com> wrote:
> 
> Anyone on this mailing list using GH authentication in Airflow? I received
> an email from GH security about a concern about our integration (bellow).
> 
> I commented on the original commit and tagged the author and someone else
> who touched the file here:
> https://github.com/apache/incubator-airflow/commit/4796245be517aed06df21a85c93a2b86a7f31939
> 
> I'd love if someone using GH authentication could take the lead on this.
> 
> Thanks,
> 
> Max
> 
> ---------- Forwarded message ----------
> From: Patrick Toomey <@github.com>
> Date: Wed, Aug 17, 2016 at 8:25 AM
> Subject: Possible security bug in incubator-airflow OAuth GitHub Enterprise
> auth
> To: max
> 
> 
> Hi,
>  I am a member of the GitHub application security team and  happened to
> recently run across https://github.com/apache/incubator-airflow, as someone
> was looking at using it internally. While they were pulling together a
> proof of concept, I noticed that they had included the
> “github_enterprise_auth.py” file. While browsing through this file I
> noticed something that stuck out. In particular, this line looks like it
> could be a security issue: https://github.com/apache/incubator-airflow/blob/
> 821bdb5310c6c21cd9c0b9d0797873ff6114179d/airflow/contrib/
> auth/backends/github_enterprise_auth.py#L159.  That lines appears to look
> for a response from the “user teams” API for a team slug that matches one
> from the “allowed_teams” configuration option. However, this slug is just a
> string, and doesn’t convey any information about a specific team. For
> example, an enterprise instance might create an organization “Acme” and
> also create a team in this organization called “designers”. But, there
> could also be another organization on that same Enterprise instance,
> “Evil”, that also creates a team called “designers”. I haven’t spun up a
> working instance of this integration, so my apologies if I missed some
> other bit of logic that would prevent this kind of attack. But, in general,
> when you do this kind of validation, it is better to use an identifier that
> is guaranteed to be globally unique across the GitHub instance. In the case
> of a GitHub Enterprise instance, that would likely be the ID of the team,
> which is also returned by the same API: https://developer.github.
> com/v3/orgs/teams/#list-user-teams.
> 
> Thanks,
> Patrick