For the records - I also looked up the discussion we had about the Github Action issues
That was a discussion on 7th of October on priv...@airflow.apache.org (for those who have access there - here is the link: https://lists.apache.org/thread.html/re92b77f64e5923ec0044edf3a060339b9e170f9438e2fde0811bf6d2%40%3Cprivate.airflow.apache.org%3E The title was "Potential Security issues with custom GitHub Actions". We came to the conclusion that just following the official recommendations from GitHub was enough for us: https://docs.github.com/en/free-pro-team@latest/actions/learn-github-actions/security-hardening-for-github-actions#using-third-party-actions But we have not thought it might be un-enforceable for the overall Apache community. It looks like then it was me, who have not "bubble" it up. I should have escalated it to infra/security (and I will for sure do it next time we stumble on smth like this). For the future, I'd love to know what is the best way to do so? J. On Sun, Dec 27, 2020 at 5:01 PM Jarek Potiuk <jarek.pot...@polidea.com> wrote: > | While pinning to a commit is a good workaround, it doesn’t substitute > for a security review. > > Absolutely. And we do review the particular commit we want to switch to. > We do it since this commit as of October 7th in Apache Airflow when we > discussed and realized the threat: > https://github.com/apache/airflow/commit/fe59f2622337616fe1902bb6d7e1bce6f002ffa8 > > But we do not review anything else - just this single commit we are using. > This is not stated anywhere but the .yml filles in our project, so if some > other project, takes the cloned repo as granted, they might use different > commit from that repo which we have not reviewed. > > So there is no higher security here IMHO - just the location of the repo > is different. Anyone can still any commit there. The fact that the clone > code is in "apache" does not mean that it has been reviewed by anyone @ > ASF. > On the other hand, we do want to have the history and bring new commits > when someone releases a new version of their action we want to use. So > "cloning" the repo is the way forward for airflow. > > J. > > > On Sun, Dec 27, 2020 at 4:52 PM Matt Sicker <boa...@gmail.com> wrote: > >> Manually reviewing third party actions would be comparable to reviewing >> shared libraries in Jenkins or Maven plugins or similar. Since the code >> runs in a privileged context, allowing arbitrary git repositories to take >> part would allow for supply chain attacks. While pinning to a commit is a >> good workaround, it doesn’t substitute for a security review. >> >> On Sun, Dec 27, 2020 at 09:05 Jarek Potiuk <jarek.pot...@polidea.com> >> wrote: >> >> > I have now clones of the actions we've been using in Airflow but I know >> few >> > other projects are using it. >> > >> > I think I should NOT encourage others to use it. Not at all. If you >> want to >> > use them you should create your own clones. >> > The skywalking project switched to those but I am going to ask them to >> make >> > their own clone. >> > >> > Greg, others, >> > >> > Is there any way we can add it to the policy? I think it should be >> strongly >> > discouraged to use cross-project clones of the actions because of the >> > reasons explained below: >> > >> > Those are our clones: >> > >> > * potiuk/cancel-workflow-runs -> apache/airflow-cancel-workflow-runs : >> > https://github.com/apache/airflow-cancel-workflow-runs >> > * potiuk/get-workflow-origin -> apache/airflow-get-workflow-origin : >> > https://github.com/apache/airflow-get-workflow-origin >> > * TobKed/label-when-approved-action -> >> apache/airflow-label-when-apprved: >> > https://github.com/apache/airflow-label-when-approved >> > * LouisBrunner/checks-action -> apache/airflow-checks-action: >> > https://github.com/apache/airflow-checks-action >> > * ad-m/github-push-action -> apache/airflow-github-push-action: >> > https://github.com/apache/airflow-github-push-action >> > * aws-actions/configure-aws-credentials/ >> > -> apache/airflow-configure-aws-credentials: >> > https://github.com/apache/airflow-configure-aws-credentials >> > >> > But when I am thinking about it, I do not want to take any >> responsibility >> > for it for other projects to use it without them reviewing it. This is a >> > really bad idea. >> > >> > This basically shifts the trust from a 3rd-party (for example me >> personally >> > on potiuk account), to apache-airflow PMCs for any other projects using >> it. >> > And I do not think as a PMC of Airflow I want to take responsibility for >> > all the commits in those repos. >> > >> > Those repos are clones of someone else's code. And we will likely >> continue >> > simply pushing stuff there from the original repositories (after >> reviewing >> > the exact version that we are planning to use but nothing else). >> > >> > So I am not sure if this actually solves the problem that infra wants to >> > deal with. >> > >> > Of course, I cannot prevent anyone, if anybody wants to use those >> > repositories. But you are basically on your own if you want to use any >> of >> > those: >> > >> > * make sure you review if the exact version you want to use is >> > * Make sure you use commit hash as specified here: >> > >> > >> https://docs.github.com/en/free-pro-team@latest/actions/learn-github-actions/security-hardening-for-github-actions#using-third-party-action >> > >> > J. >> > >> > >> > >> > On Sun, Dec 27, 2020 at 2:30 PM Jarek Potiuk <jarek.pot...@polidea.com> >> > wrote: >> > >> > > Sure. I perfectly understand, no problem and no hard feelings. I am >> not >> > at >> > > all against such immediate actions, just to message >> > > >> > > I perfectly understand why it happened and I'd probably do the same. >> > > >> > > But I would try to at least warn everyone involved immediately after, >> > > explaining it was a necessity). >> > > >> > > And since the subject was raised and discussed before (and I was well >> > > aware of the potential threat), I would love to know what we can do >> > > to prevent things like this from happening in the future. After I deal >> > with >> > > it and restore service for our users, I will try to find all the >> > discussion >> > > threads about it, and maybe we can analyze why this was not picked up >> > > before and acted before? I think we could have prevented it. Maybe >> it's >> > > simply us @Airflow that we knew about it and did not raise it to the >> > > security team? Or maybe it was just missed among other problems >> raised in >> > > the same discussion? I think we should treat it as a learning >> > experience. >> > > >> > > It would be great to find out how in the future we could help infra to >> > act >> > > proactively rather than reactively in such cases. >> > > >> > > J. >> > > >> > > On Sun, Dec 27, 2020 at 2:26 PM Greg Stein <gst...@gmail.com> wrote: >> > > >> > >> We are all human, and fallible. Could we have resolved this earlier, >> and >> > >> rolled it out in a controlled manner? Yes. But we did not understand >> the >> > >> issue, and (thus) did not apply the restriction sooner. >> > >> >> > >> Volunteers help out Infra with alerts on JDK releases, about email >> > >> issues, etc ... We would certainly welcome more alerts/contributions >> > when >> > >> these kinds of issues arise. >> > >> >> > >> And yes: DGruno noted in Slack that you were using a pinned version >> > >> (yay!). This isn't about Airflow, but about protecting the overall >> > >> Foundation attack surface. >> > >> >> > >> Thx, >> > >> -g >> > >> >> > >> >> > >> On Sun, Dec 27, 2020 at 7:04 AM Jarek Potiuk < >> jarek.pot...@polidea.com> >> > >> wrote: >> > >> >> > >>> Ok. Thanks. I understand now. >> > >>> >> > >>> But it would be great if you would explain the context in the >> > >>> announcement and try to be a bit more vocal (builds@ seems like a >> > great >> > >>> place to announce such changes). >> > >>> >> > >>> It caught everyone by surprise. >> > >>> >> > >>> Again - the scenarios you explain were already discussed before (I >> have >> > >>> to now rush and fix stuff for our committers so I cannot find it >> > quickly). >> > >>> I raised it a few months ago and it was a consensus to use >> > >>> >> > >> https://docs.github.com/en/free-pro-team@latest/actions/learn-github-actions/security-hardening-for-github-actions#using-third-party-actions >> > >>> explains exactly what to do (using pinned hashes) which we >> rigorously >> > >>> follow and recommended everyone to do - to prevent exactly the >> > scenarios >> > >>> you described. >> > >>> >> > >>> While I understand this approach can't be enforced (and the policy >> is >> > >>> reasonable), it could have been acted on before I think, and without >> > such >> > >>> rush. >> > >>> >> > >>> J. >> > >>> >> > >>> >> > >>> On Sun, Dec 27, 2020 at 1:57 PM Greg Stein <gst...@gmail.com> >> wrote: >> > >>> >> > >>>> On Sun, Dec 27, 2020 at 6:54 AM Jarek Potiuk < >> > jarek.pot...@polidea.com> >> > >>>> wrote: >> > >>>> >... >> > >>>> >> > >>>>> Was it as a response to some security incident that would justify >> > such >> > >>>>> immediate and disruptive action without an earlier warning? What >> was >> > the >> > >>>>> reasoning behind this? >> > >>>>> >> > >>>> >> > >>>> Yes. >> > >>>> >> > >>>> >> > >>> >> > >>> -- >> > >>> >> > >>> Jarek Potiuk >> > >>> Polidea <https://www.polidea.com/> | Principal Software Engineer >> > >>> >> > >>> M: +48 660 796 129 <+48660796129> >> > >>> [image: Polidea] <https://www.polidea.com/> >> > >>> >> > >>> >> > > >> > > -- >> > > >> > > Jarek Potiuk >> > > Polidea <https://www.polidea.com/> | Principal Software Engineer >> > > >> > > M: +48 660 796 129 <+48660796129> >> > > [image: Polidea] <https://www.polidea.com/> >> > > >> > > >> > >> > -- >> > >> > Jarek Potiuk >> > Polidea <https://www.polidea.com/> | Principal Software Engineer >> > >> > M: +48 660 796 129 <+48660796129> >> > [image: Polidea] <https://www.polidea.com/> >> > >> > > > -- > > Jarek Potiuk > Polidea <https://www.polidea.com/> | Principal Software Engineer > > M: +48 660 796 129 <+48660796129> > [image: Polidea] <https://www.polidea.com/> > > -- Jarek Potiuk Polidea <https://www.polidea.com/> | Principal Software Engineer M: +48 660 796 129 <+48660796129> [image: Polidea] <https://www.polidea.com/>
