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

image-rs: fix duplicated image layer detective logic #842

Merged
merged 1 commit into from
Dec 12, 2024

Conversation

Xynnn007
Copy link
Member

@Xynnn007 Xynnn007 commented Dec 11, 2024

Before this commit, when we pull images who have two encrypted layers whose corresponding plaintext layers are same. For example quay.io/fidencio/prueba:encrypted, there will be an error like

index out of bounds: the len is 25 but the index is 25

This is caused by the deduplication logic inside image pull logic. On one hand, it will delete duplicated layers recorded inside image manifest, who reflectes the encrypted layers/blobs. On the other hand, it will delete duplicated layers recorded inside the config.json, who reflects the plaintext of the layers.

The image encryption logic will generate a random symmetric key for each layer. Thus even the same plaintext layer would be encrypted into different blobs/layers. Thus after deduplication, we might have more layers for image manifest.

This patch changes the deduplicating logic, by only check the layer digests inside image manifest, s.t. even if there are two same plaintext layers, we will pull and decrypt both of them. It's ok to do some optimization later if a fully analyzation is taken.

Fies #840

cc @fidencio @mkulke @arronwy

@Xynnn007 Xynnn007 requested a review from a team as a code owner December 11, 2024 08:08
@fidencio
Copy link
Member

Let me give it a quick try on my side and, as always, thanks @Xynnn007!

@fidencio
Copy link
Member

It works as expected, thanks @Xynnn007!

@Xynnn007
Copy link
Member Author

Fixed the lint error.

Copy link
Member

@fidencio fidencio left a comment

Choose a reason for hiding this comment

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

lgtm and works as expected, thanks @Xynnn007!

image-rs/src/pull.rs Outdated Show resolved Hide resolved
Before this commit, when we pull images who have two encrypted layers
whose corresponding plaintext layers are same like
`quay.io/fidencio/prueba:encrypted`, ther will be an error like

```
index out of bounds: the len is 25 but the index is 25
```

This is caused by the deduplication logic inside image pull logic. On
one hand, it will delete duplicated layers recorded inside image
manifest, who reflectes the encrypted layers/blobs. On the other hand,
it will delete duplicated layers recorded inside the config.json, who
reflects the plaintext of the layers.

The image encryption logic will generate a random symmetric key for each
layer. Thus even the same plaintext layer would be encrypted into
different blobs/layers. Thus after deduplication, we might have more
layers for image manifest.

This patch changes the deduplicating logic, by only check the layer
digests inside image manifest, s.t. even if there are two same plaintext
layers, we will pull and decrypt both of them. It's ok to do some
optimization later if a fully analyzation is token.

Fies confidential-containers#840

Signed-off-by: Xynnn007 <[email protected]>
@Xynnn007 Xynnn007 merged commit c90a166 into confidential-containers:main Dec 12, 2024
7 checks passed
@Xynnn007 Xynnn007 deleted the fix-image-rs branch December 12, 2024 02:12
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