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(aws): only check artifacts that can be scanned for vulnerabilities by ecr_repositories_scan_vulnerabilities_in_latest_image #4507

Merged
merged 11 commits into from
Jul 31, 2024

Conversation

kagahd
Copy link
Contributor

@kagahd kagahd commented Jul 22, 2024

Context

Until now, the check ecr_repositories_scan_vulnerabilities_in_latest_image just takes the newest pushed image from the ECR repository to check its scan report for vulnerabilities. However, the newest pushed image in a repository is not always an artifact that can be scanned, hence there is no report for it and the prowler check fails.

Description

AWS ECR is generally able to scan artifacts of the following artifact media types (not to confound with manifest media type):

  • application/vnd.docker.container.image.v1+json Docker image configuration
  • application/vnd.docker.image.rootfs.diff.tar Docker image layer as a tar archive
  • application/vnd.docker.image.rootfs.diff.tar.gzip Docker image layer that is compressed using gzip
  • application/vnd.oci.image.config.v1+json OCI image configuration
  • application/vnd.oci.image.layer.v1.tar Uncompressed OCI image layer
  • application/vnd.oci.image.layer.v1.tar+gzip Compressed OCI image layer

However, tools like Google container tool Jib, use application/vnd.oci.image.config.v1+json also for signatures, which are not scannable. Luckily, these are tagged with sha-<HASH-CODE>.sig, so that they can still be easily recognized.

In contrast, non-scannable artifacts are for example:

  • Image Indexes (Manifest Lists) application/vnd.docker.distribution.manifest.list.v2+json
    • An image index or manifest list groups multiple image versions for different platforms. The index itself does not contain layers and thus cannot be directly scanned. Instead, the individual images referenced by the index are scanned.
  • Notary Signatures application/vnd.cncf.notary.v2.signature
    • Notary signatures are used to verify the integrity and authenticity of container images but do not contain image layers that can be scanned for vulnerabilities.
  • OCI Artifact Types (such as Helm Charts) application/vnd.oci.artifact.v1+json
    • General OCI artifacts like Helm charts are not container images and do not contain layers in the traditional sense. They cannot be directly scanned for vulnerabilities, although other tools or processes might perform security checks on specific types of artifacts.

For example, if a signed container image was built with the Google container tool Jib, multiple artifacts are pushed to the registry.

AWS_ECR_signed_image

As you can see in the screenshot, the artifacts are ordered by Pushed at. The four first artifacts are not scannable because they are not Docker container images. The first three are signatures and the fourth is an "Image Index". All these four artifacts do not contain any layers that could be scanned.

Furthermore, as you can see in the screenshot, the fifth artifact, which is a scannable Docker container image, does not have any tags which is represented by the - sign. This can have several reasons as for example:

  • security considerations
    • In certain cases, images could be uploaded without tags to ensure that only the digest is used, which provides a higher level of security. The digest guarantees that the image is unchanged, while tags can be changed manually.
  • backwards compatibility and image promotion
    • Sometimes images are intentionally uploaded without tags to allow for later promotion. This may be part of a strategy to ensure backwards compatibility or to manage image versioning. For example, an image is initially uploaded without a tag and a tag is added after successful testing.

However, the current prowler report refers to an image tag that not only may not be present, but is also used by different artifacts, so the user would have difficulty understanding which image is meant in the prowler report (the prowler report also didn't mention that the newest image of that tag is meant).

Implemented solution

  1. The check ecr_repositories_scan_vulnerabilities_in_latest_image verifies now the scan report of newest pushed scannable artifact.
  2. The prowler report refers now to the image digest. However, the image tag ist still mentioned in the report. The image digest is a cryptographic hash value that uniquely identifies the content of a Docker image. The digest changes when the content of the image changes, even if the image tag remains the same. The user can therefore easily identify the image to which the prowler report refers.

License

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@kagahd kagahd requested a review from a team as a code owner July 22, 2024 16:28
@github-actions github-actions bot added the provider/aws Issues/PRs related with the AWS provider label Jul 22, 2024
Copy link

codecov bot commented Jul 22, 2024

Codecov Report

Attention: Patch coverage is 81.25000% with 12 lines in your changes missing coverage. Please review.

Project coverage is 88.86%. Comparing base (aa44bde) to head (055b5b2).
Report is 51 commits behind head on master.

Files Patch % Lines
prowler/providers/aws/services/ecr/ecr_service.py 79.31% 12 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4507      +/-   ##
==========================================
+ Coverage   88.69%   88.86%   +0.16%     
==========================================
  Files         893      910      +17     
  Lines       27205    27693     +488     
==========================================
+ Hits        24130    24608     +478     
- Misses       3075     3085      +10     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@jfagoagas
Copy link
Member

WOW @kagahd this is impressive 🤯 Thank you for this contribution, we will review it and get back to you!

Thanks for contributing with Prowler always taking care of it ❤️

@sergargar sergargar changed the title fix(aws) only check artifacts that can be scanned for vulnerabilities by ecr_repositories_scan_vulnerabilities_in_latest_image fix(aws): only check artifacts that can be scanned for vulnerabilities by ecr_repositories_scan_vulnerabilities_in_latest_image Jul 23, 2024
@jfagoagas jfagoagas marked this pull request as draft July 23, 2024 20:51
@kagahd kagahd marked this pull request as ready for review July 24, 2024 18:16
@kagahd kagahd requested a review from a team as a code owner July 24, 2024 18:16
@jfagoagas jfagoagas self-requested a review July 26, 2024 12:19
@jfagoagas
Copy link
Member

Hello @kagahd I'm in the middle of the review but I'm not sure I'll be able to finish it today. If not I'll do that on Monday.

Thank you!

@kagahd
Copy link
Contributor Author

kagahd commented Jul 26, 2024

Hello @kagahd I'm in the middle of the review but I'm not sure I'll be able to finish it today. If not I'll do that on Monday.

Hello @jfagoagas, thanks for the info. I won't be at work for the next few weeks anyway, so there's no rush 😄

Copy link
Member

Choose a reason for hiding this comment

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

Please @kagahd, add a new test for a non scannable artifact.

try:
# use "image" for scan findings to get data the same way as for an image
image = (
client.describe_image_scan_findings(
Copy link
Member

Choose a reason for hiding this comment

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

Please, create a new function in the ECR class for this API Call.

@@ -121,6 +121,27 @@ def __get_repository_lifecycle_policy__(self, regional_client):

def __get_image_details__(self, regional_client):
logger.info("ECR - Getting images details...")

def is_artifact_scannable(artifact_media_type, tags):
Copy link
Member

Choose a reason for hiding this comment

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

Please, put this function outside of the parent function and use try/except.

Copy link
Member

@sergargar sergargar left a comment

Choose a reason for hiding this comment

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

Love this @kagahd ❤️ Please, review my comments.

@jfagoagas
Copy link
Member

No new permissions needed, describe:* covers it.

@sergargar sergargar self-requested a review July 31, 2024 12:48
@sergargar sergargar merged commit 26a5ffa into prowler-cloud:master Jul 31, 2024
10 of 11 checks passed
@jfagoagas jfagoagas added backport-to-v3 Backport PR to the v3 branch and removed backport-v3 backport-to-v3 Backport PR to the v3 branch labels Aug 7, 2024
github-actions bot pushed a commit that referenced this pull request Aug 7, 2024
…s by `ecr_repositories_scan_vulnerabilities_in_latest_image` (#4507)

Co-authored-by: Pepe Fagoaga <[email protected]>
(cherry picked from commit 26a5ffa)

# Conflicts:
#	tests/providers/aws/services/ecr/ecr_service_test.py
@github-actions github-actions bot added the was-backported The PR was successfully backported to the target branch label Aug 7, 2024
Copy link
Contributor

github-actions bot commented Aug 7, 2024

💚 All backports created successfully

Status Branch Result
v3

Questions ?

Please refer to the Backport tool documentation and see the Github Action logs for details

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-to-v3 Backport PR to the v3 branch documentation provider/aws Issues/PRs related with the AWS provider was-backported The PR was successfully backported to the target branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants