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

[docs] add documentation regarding verify images signatures #8990

Merged
merged 4 commits into from
May 29, 2024

Conversation

cpanato
Copy link
Contributor

@cpanato cpanato commented Nov 24, 2023

Description: <Describe what has changed.

  • add documentation regarding verify images signatures

related to open-telemetry/opentelemetry-collector-releases#207

Fixes: #9610

/assign @jpkrohling

README.md Outdated Show resolved Hide resolved
Copy link

codecov bot commented Nov 24, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 92.47%. Comparing base (1d52fb9) to head (8bc3c48).
Report is 60 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #8990      +/-   ##
==========================================
+ Coverage   91.63%   92.47%   +0.84%     
==========================================
  Files         356      387      +31     
  Lines       16849    18246    +1397     
==========================================
+ Hits        15439    16873    +1434     
+ Misses       1068     1027      -41     
- Partials      342      346       +4     

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

@cpanato cpanato changed the title add documentation regarding verify images signatures [docs] add documentation regarding verify images signatures Nov 24, 2023
@cpanato cpanato force-pushed the doc-verify-images branch 2 times, most recently from 30e2eab to 2d09309 Compare November 24, 2023 12:07
Copy link
Member

@tigrannajaryan tigrannajaryan left a comment

Choose a reason for hiding this comment

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

I have a few questions which are about signing (not verifying) which would belong more to the PR that implements signing, but I am not sure which PR is that, so asking here to understand the signing story better.

README.md Outdated
Example:

```console
$ cosign verify --certificate-identity=https://github.com/open-telemetry/opentelemetry-collector-releases/.github/workflows/release.yaml@refs/tags/v99.99.01 --certificate-oidc-issuer=https://token.actions.githubusercontent.com ghcr.io/open-telemetry/opentelemetry-collector-releases/opentelemetry-collector:99.99.01
Copy link
Member

Choose a reason for hiding this comment

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

Please add some explanation about what exactly is being verified here and how to spot the expected valid output.

Copy link
Member

Choose a reason for hiding this comment

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

@cpanato, can you provide this?


> **Note**: To verify a signed artifact or blob, first [install Cosign](https://docs.sigstore.dev/system_config/installation/), then follow the instructions below.

We are signing the images `otel/opentelemetry-collector` and `otel/opentelemetry-collector-contrib` using [sigstore cosign](https://github.com/sigstore/cosign) tool and to verify the signatures you can run the following command:
Copy link
Member

Choose a reason for hiding this comment

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

Sigstore docs say that when signing:

A verifiable OpenID Connect identity token, which contains a user’s email address or service account, is provided in the request.

What identity token do we provide for signing the Collector? Is it an email address or some other identity? Where can this identity token be seen?

Copy link
Member

Choose a reason for hiding this comment

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

Sigstore docs say:

All signers should monitor the Fulcio CT log for unauthorized use of their identity, and all verifiers should look for alerts from consistency monitors.

What do we (Otel) do to monitor the CT log?

Copy link
Member

Choose a reason for hiding this comment

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

Right now, nothing. I just created open-telemetry/sig-security#45 to track this.

Copy link
Member

Choose a reason for hiding this comment

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

The identity is the JSON that is provided above in the readme and includes things like the github repo name and the workflow that generated it. The actual "Subject" of the token would be https://github.com/cpanato/opentelemetry-collector-releases/.github/workflows/release.yaml@refs/tags/v99.99.01 , in the readme's example.

Copy link
Contributor

This PR was marked stale due to lack of activity. It will be closed in 14 days.

@github-actions github-actions bot added Stale and removed Stale labels Dec 20, 2023
Copy link
Contributor

github-actions bot commented Jan 9, 2024

This PR was marked stale due to lack of activity. It will be closed in 14 days.

@github-actions github-actions bot added Stale and removed Stale labels Jan 9, 2024
Copy link
Contributor

This PR was marked stale due to lack of activity. It will be closed in 14 days.

Copy link
Contributor

github-actions bot commented Feb 9, 2024

This PR was marked stale due to lack of activity. It will be closed in 14 days.

@github-actions github-actions bot added the Stale label Feb 9, 2024
@cpanato
Copy link
Contributor Author

cpanato commented Feb 13, 2024

@jpkrohling let me know when is good to update this PR to merge, to avoid keeping the updating this.

@github-actions github-actions bot removed the Stale label Feb 14, 2024
@jpkrohling
Copy link
Member

Given that the -releases PR is merged, this one can be rebased and marked as ready!

@cpanato cpanato marked this pull request as ready for review February 20, 2024 12:38
@cpanato cpanato requested a review from a team as a code owner February 20, 2024 12:38
@cpanato
Copy link
Contributor Author

cpanato commented Feb 20, 2024

@jpkrohling rebased, thanks!

README.md Show resolved Hide resolved
Copy link
Contributor

github-actions bot commented Mar 7, 2024

This PR was marked stale due to lack of activity. It will be closed in 14 days.

@github-actions github-actions bot added the Stale label Apr 11, 2024
@tigrannajaryan
Copy link
Member

Folks, what's the status of this PR? It would be great to finalize and merge it.

@cpanato
Copy link
Contributor Author

cpanato commented Apr 11, 2024

@tigrannajaryan i will do that tomorrow, sorry for the delay

Signed-off-by: cpanato <ctadeu@gmail.com>
@cpanato
Copy link
Contributor Author

cpanato commented Apr 12, 2024

@jpkrohling @tigrannajaryan made some changes and update the image reference PTAL

Copy link
Member

Choose a reason for hiding this comment

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

We can omit a changelog for this

Copy link
Member

Choose a reason for hiding this comment

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

I'm OK keeping this though, as it can help spread the word about our images being signed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

will wait the decision to remove or keep :)

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 keep this and merge the PR, if that's the only thing holding this PR

@TylerHelmuth TylerHelmuth added the Skip Changelog PRs that do not require a CHANGELOG.md entry label Apr 16, 2024
Copy link
Member

Choose a reason for hiding this comment

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

I'm OK keeping this though, as it can help spread the word about our images being signed.

README.md Show resolved Hide resolved
Copy link
Contributor

github-actions bot commented May 9, 2024

This PR was marked stale due to lack of activity. It will be closed in 14 days.

@cpanato
Copy link
Contributor Author

cpanato commented May 13, 2024

anything that i need to do?

@jpkrohling
Copy link
Member

anything that i need to do?

I don't think so, I just pinged @open-telemetry/collector-maintainers to see if this can be merged as is.

README.md Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
Copy link
Contributor

This PR was marked stale due to lack of activity. It will be closed in 14 days.

@github-actions github-actions bot added the Stale label May 29, 2024
README.md Outdated Show resolved Hide resolved
@mx-psi mx-psi merged commit 40f5a4e into open-telemetry:main May 29, 2024
48 checks passed
@github-actions github-actions bot added this to the next release milestone May 29, 2024
@cpanato cpanato deleted the doc-verify-images branch May 29, 2024 10:12
steves-canva pushed a commit to Canva/opentelemetry-collector that referenced this pull request Jun 14, 2024
…emetry#8990)

**Description:** <Describe what has changed. 
- add documentation regarding verify images signatures

related to
open-telemetry/opentelemetry-collector-releases#207

Fixes:
open-telemetry#9610

/assign @jpkrohling

---------

Signed-off-by: cpanato <ctadeu@gmail.com>
Co-authored-by: Juraci Paixão Kröhling <juraci@kroehling.de>
Co-authored-by: Pablo Baeyens <pbaeyens31+github@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Skip Changelog PRs that do not require a CHANGELOG.md entry Stale
Projects
None yet
Development

Successfully merging this pull request may close these issues.

add documentation regarding verify images signatures
5 participants