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

Signed content not being synced #1759

Closed
git-hyagi opened this issue Sep 2, 2024 · 0 comments · Fixed by #1760
Closed

Signed content not being synced #1759

git-hyagi opened this issue Sep 2, 2024 · 0 comments · Fixed by #1760

Comments

@git-hyagi
Copy link
Contributor

Describe the bug
Trying to sync filtered content with cosign signature, from a remote without sigstore, and "signed_only": true can fail sometimes.

To Reproduce
Steps to reproduce the behavior:

pulp container repository create --name foo
pulp container remote create --name foo --url "https://quay.io" --upstream-name=curl/curl --include-tags='["8.9.1","sha256-7dd57efcae8c9c2a611816151d731a02a31fab5ab9fb5e0ff877f43009944a51.sig"]'
curl -H 'content-type:application/json' -u<user>:<pass> -X POST ${BASE_ADDR}$(pulp container repository show --name foo |jq .pulp_href -r)sync/ -d '{"signed_only": true, "remote": '$(pulp container remote show --name foo |jq .pulp_href)'}'

The above execution can download the signature tag, the '8.9.1' tag, or sometimes both.

Expected behavior
It should always sync the tag and the corresponding signature.

Additional context
I think this is happening because of 2 things:
1- random tag_name checking
Here

not (tag_name.endswith(".sig") and tag_name.startswith("sha256-"))

we are verifying the tag_name defined as the last element from loop:

for tag_name in tag_list:
relative_url = "/v2/{name}/manifests/{tag}".format(
name=self.remote.namespaced_upstream_name, tag=tag_name
)
tag_url = urljoin(self.remote.url, relative_url)
downloader = self.remote.get_downloader(url=tag_url)
to_download.append(
downloader.run(extra_data={"headers": V2_ACCEPT_HEADERS, "http_method": "head"})
)

which is updated in a later instruction:

tag_name = response.url.split("/")[-1]

which can cause problems because, depending on the order returned from to_download_artifact, we would be checking the tag_name from one artifact and the digest from another:
https://github.com/pulp/pulp_container/blob/0406fa242c07829d77064d4fe6843c04d5ea7cea/pulp_container/app/tasks/sync_stages.py#L146C1-L146C72
and mistakenly think that the tag has no corresponding signature. For example:

  • if the artifact for the current loop iteration is a signature and tag_name is still defined as '8.9.1', we will match the following condition
                    if (
                        not (tag_name.endswith(".sig") and tag_name.startswith("sha256-"))
                        and f"sha256-{digest.removeprefix('sha256:')}.sig" not in tag_list
                    ):

and the signature will not be downloaded.

  • if the artifact for the current loop iteration is the '8.9.1' manifest/tag and tag_name is still defined as 'sha-7dd5...sig' we will also match the above condition (the sha256-.sig does not exist) and the '8.9.1' tag will not be downloaded

2- wrong verification of digest
the return from calculate_digest is a string with format:

"sha256:<digest>"

when we look for cosign signatures, we verify if a string with pattern "sha256-{digest}.sig" is present in tag_list

digest = calculate_digest(raw_text_data)
# Look for cosign signatures
# cosign signature has a tag convention 'sha256-1234.sig'
if self.signed_only and not signature_source:
if (
not (tag_name.endswith(".sig") and tag_name.startswith("sha256-"))
and f"sha256-{digest}.sig" not in tag_list

so, we are verifying things like:

sha256-sha256:7dd57...sig

instead of

sha256-7dd57...sig
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Shipped
Development

Successfully merging a pull request may close this issue.

1 participant