Skip to content
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

fix: Upgraded docker distribution go package to v2.8.2 for fixing a high vulnerability #11554

Merged
merged 3 commits into from
Aug 10, 2023

Conversation

Jonsy13
Copy link
Contributor

@Jonsy13 Jonsy13 commented Aug 10, 2023

Motivation

Here from LitmusChaos Again!!

After our last PR - #11538 got merged, we scanned the latest images again, we found one more vulnerability in both workflow-controller and argoexec which may have been missed earlier -

Scan results for: image [quay.io/argoproj/argoexec:latest] sha256:ee7f51a0169c160f6c04e4a85d3fddae14b5ff640d0b2202a8e8fb7ffc4ddef4
Vulnerabilities
+---------------+----------+------+--------------------------------+---------+-----------------------+-----------+------------+----------------------------------------------------+
|      CVE      | SEVERITY | CVSS |            PACKAGE             | VERSION |        STATUS         | PUBLISHED | DISCOVERED |                    DESCRIPTION                     |
+---------------+----------+------+--------------------------------+---------+-----------------------+-----------+------------+----------------------------------------------------+
| CVE-2023-2253 | high     | 7.00 | [github.com/docker/distribution] | v2.8.1  | fixed in 2.8.2-beta.1 | 64 days   | < 1 hour   | A flaw was found in the `/v2/_catalog` endpoint    |
|               |          |      |                                |         | 90 days ago           |           |            | in distribution/distribution, which accepts a      |
|               |          |      |                                |         |                       |           |            | parameter to control the maximum number of records |
|               |          |      |                                |         |                       |           |            | retur...                                           |
+---------------+----------+------+--------------------------------+---------+-----------------------+-----------+------------+----------------------------------------------------+

Vulnerabilities found for image [quay.io/argoproj/argoexec:latest]: total - 1, critical - 0, high - 1, medium - 0, low - 0

and also there was one compliance issue (only in argoexec but not in workflow-controller), on checking Dockerfile, we found that argoexec container doesn't have a USER set like other containers (workflow-controller, UI,etc) -

Compliance Issues
+----------+------------------------------------------------------------------------------+
| SEVERITY |                                 DESCRIPTION                                  |
+----------+------------------------------------------------------------------------------+
| high     | (CIS_Docker_v1.5.0 - 4.1) Image should be created with a non-root user       |

Modifications

Have updated the docker distribution go package to 2.8.2 & also added USER instruction USER 8737 in argoexec build stage just like workflow-controller.

NOTE - Do let us know if USER instruction is not required, will remove it again! Thanks!!

Verification

Signed-off-by: Jonsy13 <vedant.shrotria@harness.io>
@gdsoumya gdsoumya requested a review from terrytangyuan August 10, 2023 14:28
Copy link
Member

@gdsoumya gdsoumya left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@terrytangyuan
Copy link
Member

Thanks. I'll include this in #11552

@terrytangyuan terrytangyuan enabled auto-merge (squash) August 10, 2023 14:33
@terrytangyuan
Copy link
Member

Could you do a scan of v3.4.9 and see if there are any important fixes that should go in v3.4.10 #11552?

@Jonsy13
Copy link
Contributor Author

Jonsy13 commented Aug 10, 2023

Sure @terrytangyuan Will do!! Thanks!

Signed-off-by: Jonsy13 <vedant.shrotria@harness.io>
auto-merge was automatically disabled August 10, 2023 14:39

Head branch was pushed to by a user without write access

Dockerfile Outdated
@@ -78,6 +78,8 @@ RUN --mount=type=cache,target=/go/pkg/mod --mount=type=cache,target=/root/.cache

FROM gcr.io/distroless/static as argoexec

USER 8737
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need some eyes on this

Copy link
Contributor Author

@Jonsy13 Jonsy13 Aug 10, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, let me know if you think this will take time, I can remove this change from this PR & have it in a separate one. Don't want to make it release blocker for you! 😄

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's remove it for now

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done thanks!!

@Jonsy13
Copy link
Contributor Author

Jonsy13 commented Aug 10, 2023

Scanned v3.4.9, There was only one more -

| PRISMA-2023-0056 | medium   | 6.20 | [github.com/sirupsen/logrus]       | v1.9.2  | fixed in v1.9.3          | > 4 months | < 1 hour   | The [github.com/sirupsen/logrus] module of all       |
|                  |          |      |                                   |         | > 5 months ago           |            |            | versions is vulnerable to denial of service.       |
|                  |          |      |                                   |         |                          |            |            | Logging more than 64kb of data in a single entry   |
|                  |          |      |                                   |         |                          |            |            | without new...

But this is already fixed in latest tag, master branch & release-3.5 branch as well -

github.com/sirupsen/logrus v1.9.3
.

Signed-off-by: Jonsy13 <vedant.shrotria@harness.io>
@terrytangyuan terrytangyuan enabled auto-merge (squash) August 10, 2023 15:04
@terrytangyuan terrytangyuan merged commit 43f15c6 into argoproj:master Aug 10, 2023
terrytangyuan pushed a commit that referenced this pull request Aug 11, 2023
…igh vulnerability (#11554)

Signed-off-by: Jonsy13 <vedant.shrotria@harness.io>
dpadhiar pushed a commit to dpadhiar/argo-workflows that referenced this pull request May 9, 2024
…igh vulnerability (argoproj#11554)

Signed-off-by: Jonsy13 <vedant.shrotria@harness.io>
Signed-off-by: Dillen Padhiar <dillen_padhiar@intuit.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants