CVE-2019-17495 for swagger-ui

[hughlunnon]

Apache Airflow version

2.5.0

What happened

this issue #18383 still isn't closed. It seems like the underlying swagger-ui bundle has been abandoned by its maintainer, and we should instead point swagger UI bundle to this version which is kept up-to-date

https://github.com/bartsanchez/swagger_ui_bundle

edit : it seems like this might not be coming from the swagger_ui_bundle any more but instead perhaps from connexion. I'm not familiar with python dependencies, so forgive me if I'm mis-reporting this.

There are CVE scanner tools that notifies GHSA-c427-hjc3-wrfw using the apache/airflow:2.1.4

The python deps include swagger-ui-2.2.10 and swagger-ui-3.30.0 as part of the bundle. It is already included at ~/.local/lib/python3.6/site-packages/swagger_ui_bundle

swagger-ui-2.2.10 swagger-ui-3.30.0

What you think should happen instead

No response

How to reproduce

No response

Operating System

any

Versions of Apache Airflow Providers

No response

Deployment

Docker-Compose

Deployment details

No response

Anything else

No response

Are you willing to submit PR?

• Yes I am willing to submit a PR!

Code of Conduct

• I agree to follow this project's Code of Conduct

[boring-cyborg]

Thanks for opening your first issue here! Be sure to follow the issue template!

[potiuk]

Any sucess on that @hughlunnon . It's not a high priority issue, I marked it as "good first issue" but since you've been assigned and wlling to make a PR, just wanted to check in.

[JGoldman110]

@potiuk & @hughlunnon we have just upgraded to 2.5.0 and this vulnubility is coming up from our scans. Happy to raise a PR to resolve.

[hughlunnon]

I'm sorry, I've been completely snowed under, and being a JVM guy I don't know (easily) how to work with the dependency management in python (if I did I figure it'd be a 5 min job to resolve). @JGoldman110 if you're able to easily raise a PR that'd be great, otherwise I'll have a play next week.

[JGoldman110]

The maintainer of swagger-ui-bundle has abandoned the project, and this is a dependency of the connexion[swagger-ui] extra. I don't see any alternative python package which bundles swagger-ui static files. One solution I see is that we can remove swagger-ui extra for connexion package, add the static swagger-ui files as a airflow vendor dependency or we can install swagger-ui-dist as an npm package. Then set the following connexion configuration to tell connexion where to find swagger static files.

@property
def openapi_console_ui_from_dir(self):
    # type: () -> str
    """
    Custom OpenAPI Console UI directory from where Connexion will serve
    the static files.
    Default: Connexion's vendored version of the OpenAPI Console UI.
    """
    return self._options.get('swagger_path', self.swagger_ui_local_path)

@potiuk any thoughts on this approach? I guess you could make the argument that connexion should make the fix, but maybe this could be a quick fix for the CVE?

[hughlunnon]

remove swagger-ui extra for connexion package, add the static swagger-ui files as a airflow vendor dependency or we can install swagger-ui-dist as an npm package

Looking at the directory structure, swagger 3.52.0 is also present (as well as 2.2.10), both in almost exactly the same location. If we just excluded the connexion swaggerUI dep, might connexion automatically pick up the newer version?

[JGoldman110]

Looking at the directory structure, swagger 3.52.0 is also present (as well as 2.2.10), both in almost exactly the same location. If we just excluded the connexion swaggerUI dep, might connexion automatically pick up the newer version?

connexion is already using 3.52.0 as we are using openapi version 3.0.3, so I am unsure if this vulnerability is still executable if we are using swagger-ui 3.52.0, but the 2.2.10 version is present?

https://github.com/spec-first/connexion/blob/2.14.1/connexion/options.py#L29-L31

[potiuk]

I think this is something that the person requesting it should determine (@hughlunnon). Usually when you raise a security related issue you should raise exploitation scenario and dependency chain. If you have reason to believe Airflow is impacted via dependency you should show how so this is the question to @hughlunnon - do you think this is issue is still worth to be open (and why?) or maybe it should be fixed because the latest version already fixes it

WDYT @hughlunnon ?

[hughlunnon]

@potiuk I don't personally have an issue with the risk inherant in the dependency - however in our environment (and it appears @JGoldman110 has the same issue) we are now blocked from using airflow in any form due to the automatic vulnerability scanners we have in place. I imagine this will also affect other consumers of the app.

Swagger UI 2.2.10 was last touched (by swagger) 6 years ago - there's no need for it as newer versions (the other bundled version is 3.52.0, but 4.15.5 is also available) also support OAS2.0 spec.

connexion is already using 3.52.0 as we are using openapi version 3.0.3, so I am unsure if this vulnerability is still executable if we are using swagger-ui 3.52.0, but the 2.2.10 version is present?

I think (from reading the connexion code) that 2.x is the default, and I can't find anywhere its being over-written, but I may be wrong?

https://github.com/spec-first/connexion/blob/cdc8af157dd55cd40b9d60643416ba168ca12b86/connexion/options.py#L26

[potiuk]

@potiuk I don't personally have an issue with the risk inherant in the dependency - however in our environment (and it appears @JGoldman110 has the same issue) we are now blocked from using airflow in any form due to the automatic vulnerability scanners we have in place. I imagine this will also affect other consumers of the app.

Sure, I am not sure if you've noticed, but I am absolutely for migrating. And I would love this to happen. And I think this is the least such companies (who care for security of the open source projects they use for free) is to help to upgrade such dependencies. I think your company would be a perfect candidate to either ask some of their employees to contribue a PR with migrating to newer/different swagger or to pay someone to do it. This is an absolute least such companies might do to both - help themselves and also give back to the community.

Especially if it is a blocker because of company security scanners - your company now has much bigger incentive to help fixing it because of those security scanning policies in place.

The issue is now marked as "good-first-issue" - because literally anyone (including - but not limited to - someone employed or paid by your company - can contribute a PR to update it.

Looking forward to it. And happy to review it when someone does it.

[potiuk]

BTW. This is how OSS development works - companies might use the open source, but also might contribute back. We have > 2300 contributors to Airflow and vast majority of those contributions are people/employees of companies who have certain need/requirement and contribute it. And we really, really, really encourage it.

[JGoldman110]

I am going to raise a PR, this will be my first contribution so may need some time to go through the contributing guide and get my local setup correctly, but will reach out if I get stuck 😃

[potiuk]

Happy to help

[hughlunnon]

So today the connexion guys also merged code to get rid of this version of swagger

spec-first/connexion#1619

Idk if the easiest thing is just to bump dependency version when they release? Thought I'd mention it either way

[JGoldman110]

We could wait for this, but not sure when their next release is, seems latest connexion 2.14.1 was released in August 2022.

Their forked swagger ui bundle includes two versions of the swagger UI as well 4.4.0 and 4.15.5. I think there is an argument to include/manage the swagger UI version we want on airflow side that way in future we can upgrade independently of connexion and not carry multiple versions of the ui in airflow.

The versions of swagger-ui 4.4.0 and 4.15.5 both support our current openapi spec version 3.0.3. So if we were to wait or switch to new version of connexion when it is available, bumping the version should be fine.

My vote would be to install on airflow side to resolve cve as soon as possible.

[potiuk]

Agree with @JGoldman110 I am also for separating managing those. There is no particular need we should link them together.

[potiuk]

I just merged the PR and marked it for 2.5.1 release.