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

Add summaries for the security models of attestations and trusted publishing #17242

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

mac-chaffee
Copy link

Hello!

I recently wanted to learn more about trusted publishing and attestations but I struggled initially to understand the big-picture of the security goals and non-goals, and I had a lot of misconceptions initially.

So I added some summaries with my goal being to surface the answers to the questions I had initially, while trying to keep it simple. But that does mean I possibly over-simplified or glossed over important details, so feedback is welcome!

@mac-chaffee mac-chaffee requested a review from a team as a code owner December 7, 2024 16:09
@mac-chaffee mac-chaffee changed the title Add summaries for the security models of attestations and trusted publishers Add summaries for the security models of attestations and trusted publishing Dec 7, 2024
@miketheman miketheman requested a review from woodruffw December 7, 2024 22:13
@woodruffw
Copy link
Member

Hey @mac-chaffee! Sorry for the delay here -- I had this flagged for myself, but it looks like I lost it in the stacks. I'll do a review of these changes today!

@mac-chaffee mac-chaffee force-pushed the security-model-summaries branch from 647b2bb to a2797aa Compare January 29, 2025 17:34
Copy link
Member

@woodruffw woodruffw left a comment

Choose a reason for hiding this comment

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

Thanks @mac-chaffee! These improvements look great to me.

@mac-chaffee mac-chaffee force-pushed the security-model-summaries branch from a2797aa to d1b2433 Compare January 29, 2025 18:06
@mac-chaffee
Copy link
Author

Thanks for the suggestions! I rebased and edited the author to remove the unverified commit in case that causes issues

Copy link
Member

@di di left a comment

Choose a reason for hiding this comment

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

Looks great, thanks for improving these! Just one note from me.

Comment on lines 14 to 16
this risk by using short-lived tokens instead of long-lived tokens that can
easily get misplaced, leaked in logs, stolen by malware, or any number of other
ways.
Copy link
Member

@di di Jan 29, 2025

Choose a reason for hiding this comment

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

I think the key distinction we should make here is that long-lived tokens need to be stored and do not quickly expire. Short-lived tokens somewhat reduces the likelihood that they will "misplaced, leaked in logs, stolen by malware" (although it's still possible) but also reduces the impact if they are compromised.

Copy link
Author

Choose a reason for hiding this comment

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

Good point, let me know what you think of the new version I just pushed 👍

Comment on lines +11 to +17
In recent years, theft of credentials such as API tokens has [played a major
role in cyber attacks]. The reason for this is the unfortunate reality that
managing credentials can be complicated and risky. Trusted Publishing reduces
this risk by using short-lived tokens instead of long-lived tokens. Short-lived
tokens are less likely to be misplaced, leaked in logs, or stolen by malware.
If short-lived tokens are leaked, they only give attackers a narrow time window
to exploit the leaked token, which minimizes the potential damage.
Copy link
Member

Choose a reason for hiding this comment

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

I think this is still missing the distinction that when used with Trusted Publishing short-lived tokens are ephemeral and don't need to be stored.

Copy link
Author

Choose a reason for hiding this comment

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

Oh I thought you wanted emphasis on the fact that short-lived tokens only reduce but don't eliminate the risk. How about this?

Short-lived tokens are less likely to be misplaced, leaked in logs, or stolen by malware since they don't have to be stored.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants