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

"Created" date for signature is always Dec 31, 0 #522

Closed
jackie-kinsler opened this issue Aug 4, 2021 · 12 comments
Closed

"Created" date for signature is always Dec 31, 0 #522

jackie-kinsler opened this issue Aug 4, 2021 · 12 comments

Comments

@jackie-kinsler
Copy link

When signing images and pushing them to Google Cloud Registry, the signature's "Created" date is always Dec 31, 0.
I would have expected the "Created" date for the signature to be the date the container was signed.

printenv GCR_ACCOUNT | docker login -u _json_key --password-stdin gcr.io
cosign sign -key cosign.key $NAMESPACE:$version

@dekkagaijin
Copy link
Member

We generally avoid unnecessarily breaking reproducibility whenever possible, which means setting such timestamps to a constant value. If there's demand for it, we could add a flag to timestamp the signature images

@patrickdung
Copy link

On GitLab, the signature or attestation files would be created as a very old date.
If the user enabled house keeping of container register (by tags), I am not sure if the signature or attestation files would be removed by house keeping.

@dlorenc
Copy link
Member

dlorenc commented Dec 19, 2021

I'd be fine setting this to a real value. Reproduciblity is good, but the signatures are random anyway.

What do people think? Setting a real time could be useful, with a flag for reproducible? Time seems like it would be better to set by default TBH.

@joshuagl
Copy link
Member

I like the idea of using wall time by default and allowing clamping if an option is passed (maybe even using SOURCE_DATE_EPOCH from the environment?).

@dekkagaijin
Copy link
Member

What's the main goal of adding the creation timestamp to the sig images?

I'd be worried about potentially misleading interpretation when an image has multiple signatures, if the goal is to associate a signature with the timestamp.

@dekkagaijin
Copy link
Member

FWIW I'd also want to go through some formal design process and consider registry implementations if we just want to easily enable age-based garbage collection. Guess what contributed to "GKE's largest outage?"

@Lerentis
Copy link
Contributor

On GitLab, the signature or attestation files would be created as a very old date.
If the user enabled house keeping of container register (by tags), I am not sure if the signature or attestation files would be removed by house keeping.

I can confirm that they do get removed 😅
Any update on the decision to change this behavior or giving the user an option to use the timestamp of the signing date here? This is really frustrating to be forced to choose in between signing and housekeeping

@dlorenc
Copy link
Member

dlorenc commented Jul 26, 2022

Cc @imjasonh who has been talking about timestamps lately

@imjasonh
Copy link
Member

ref: docs for Gitlab Registry housekeeping: https://docs.gitlab.com/ee/user/packages/container_registry/reduce_container_registry_storage.html

I don't totally agree with Gitlab's decision to implement housekeeping in this way, but since they do, I think it makes sense to set the .config.created timestamp so these don't get reaped.

@Lerentis
Copy link
Contributor

I tried to have a look where in the code base this date is set but I could not find it. Using reflect to modify the signature does not seem like a good idea, so if one could drop me a hint where to set this date I could try to create a pr for this issue (:

@imjasonh
Copy link
Member

imjasonh commented Jul 27, 2022

The reason you can't find code for setting that date is because we don't set that date anywhere, and the zero date is the default. Sorry that's a bit confusing.

The place where we'd set the time is probably in https://github.com/sigstore/cosign/blob/main/pkg/oci/mutate/signatures.go when we append a signature to the existing set of signatures. When there aren't any previous signatures, we append to an empty.Image (whose created date is zero), and if there are signatures we just append a layer (and don't update the created date).

So where we mutate.Append(base, ...), we should also mutate.Time mutate.CreatedAt to either the new signature's time, or just time.Now(). This will have the effect of setting the created-time when we add the first signature, and update the time when we append to the list.

edit: it looks like we might have an issue calling mutate.Time when the layer contents aren't tar files, as with our signatures. If that's a problem I can make a change in go-containerregistry that lets you ignore non-tar layers when mutating image time. cc @jonjohnsonjr

edit again: Nevermind, there's mutate.CreatedAt which only updates the created-at date, and doesn't try to read layers. This is exactly what we need.

@Lerentis
Copy link
Contributor

Thanks @imjasonh , this seems longer than the implementation. highly appreciate your effort :)
PR here: #2108

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

No branches or pull requests

7 participants