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

Attacker goals #35

Closed
wants to merge 6 commits into from
Closed

Attacker goals #35

wants to merge 6 commits into from

Conversation

mnm678
Copy link
Contributor

@mnm678 mnm678 commented Aug 27, 2020

This reopens #20 on the new branch.

JustinCappos and others added 2 commits May 1, 2020 12:45
Signed-off-by: Justin Cappos <[email protected]>
Co-authored-by: Marina Moore <[email protected]>
Copy link
Contributor

@SteveLasker SteveLasker left a comment

Choose a reason for hiding this comment

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

General comment to refer to artifacts, and not just images as we'll want to secure Helm, WASM, Singularity and others.

Copy link
Contributor

@sudo-bmitch sudo-bmitch left a comment

Choose a reason for hiding this comment

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

Thanks for capturing these @mnm678. I've done the best I can to rephrase things to align with Notary/Container terms, but please feel free to improve on my wording.

threatmodel.md Outdated
@@ -10,3 +10,10 @@ It is assumed that an attacker may perform one or more the following actions:

While it is not always possible to protect against all scenarios, the system should to the extent possible mitigate and/or reduce the damage caused by a successful attack, detect the occurrence of an attack and notify appropriate parties, yet remain usable for parties operating the system. Furthermore, the system should recover from successful attacks in a way that presents low operational overhead and risk to users.

Attacker Goals:
1. Trying to have a party install a malicious image under the attackers control.
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd change "install" to "deploy" to better align with container terminology.

@@ -10,3 +10,10 @@ It is assumed that an attacker may perform one or more the following actions:

While it is not always possible to protect against all scenarios, the system should to the extent possible mitigate and/or reduce the damage caused by a successful attack, detect the occurrence of an attack and notify appropriate parties, yet remain usable for parties operating the system. Furthermore, the system should recover from successful attacks in a way that presents low operational overhead and risk to users.

Attacker Goals:
1. Trying to have a party install a malicious image under the attackers control.
2. Trying to have a party install an outdated image. For example, one with known security vulnerabilities.
Copy link
Contributor

Choose a reason for hiding this comment

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

Change "an outdated image" to "an image who's signature has been revoked by the signer" or something to that effect. That avoids us defining what outdated is, leaving the definition up to the signer to determine and implement according to their own policies.

Attacker Goals:
1. Trying to have a party install a malicious image under the attackers control.
2. Trying to have a party install an outdated image. For example, one with known security vulnerabilities.
3. Making images unavailable for installation.
Copy link
Contributor

Choose a reason for hiding this comment

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

For Notary, we could phrase this "Disrupt the verification of image signatures."

Copy link
Contributor

Choose a reason for hiding this comment

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

images - artifacts

threatmodel.md Outdated
2. Trying to have a party install an outdated image. For example, one with known security vulnerabilities.
3. Making images unavailable for installation.
4. Preventing a party from learning about updates to currently installed images.
5. Convincing a party to download large amounts of data that interfere with the party's system.
Copy link
Contributor

Choose a reason for hiding this comment

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

To scope this to Notary, we could say something like "Inject a malicious (oversized) signature that disrupts verification" (my phrasing could use improvement). The manifest itself will have a size limit imposed by many runtimes (like containerd), see this OCI issue for more context. The manifest includes descriptors that provide both the digest and size of the blob, so if the manifest digest is signed and verified, the only gap for us to cover is the signature artifact itself.

Copy link
Contributor

Choose a reason for hiding this comment

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

We had a good discussion about size attacks, and I thought we decided to scope this out of Notary. Registries already have mechanisms to cope with DOS attacks, and the descriptor already has enough info for artifact clients to gate these.
Suggest we remove # 5.

@sudo-bmitch sudo-bmitch mentioned this pull request Apr 10, 2021
Copy link
Contributor Author

@mnm678 mnm678 left a comment

Choose a reason for hiding this comment

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

Thanks for the suggestions @sudo-bmitch. I can't edit this directly, but I left some new wording for @JustinCappos to review, or I can re-open this pr from my fork.

threatmodel.md Outdated Show resolved Hide resolved
@@ -10,3 +10,10 @@ It is assumed that an attacker may perform one or more the following actions:

While it is not always possible to protect against all scenarios, the system should to the extent possible mitigate and/or reduce the damage caused by a successful attack, detect the occurrence of an attack and notify appropriate parties, yet remain usable for parties operating the system. Furthermore, the system should recover from successful attacks in a way that presents low operational overhead and risk to users.

Attacker Goals:
1. Trying to have a party install a malicious image under the attackers control.
2. Trying to have a party install an outdated image. For example, one with known security vulnerabilities.
Copy link
Contributor Author

@mnm678 mnm678 Apr 12, 2021

Choose a reason for hiding this comment

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

Suggested change
2. Trying to have a party install an outdated image. For example, one with known security vulnerabilities.
2. To have a party deploy an artifact using a digest or tag that does not have a currently valid signature. For example, a previous version of an artifact with a signature that was revoked by the signer, or an old version associated with a tag.

Choose a reason for hiding this comment

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

This changes the meaning a bit. The prior also would include artifacts that were not the latest (when the latest is requested), but this seems to imply those are fine.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I updated my change. In some cases, someone might pull an old image by digest on purpose, and as long as the signature is still valid, this should be accepted, which is the subtlety I was trying to add. But yes, it still needs to use the latest tag mapping when tags are used.

Attacker Goals:
1. Trying to have a party install a malicious image under the attackers control.
2. Trying to have a party install an outdated image. For example, one with known security vulnerabilities.
3. Making images unavailable for installation.
Copy link
Contributor Author

@mnm678 mnm678 Apr 12, 2021

Choose a reason for hiding this comment

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

Suggested change
3. Making images unavailable for installation.
3. Disrupt the verification of artifact signatures, for example by making the current version of metadata unavailable.

Copy link
Contributor

Choose a reason for hiding this comment

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

image - artifact

Choose a reason for hiding this comment

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

This is a bit more specific. I don't know if it matters, but it reads like your text is a special case of mine...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was trying to make it clear that DoS attacks are out of scope, but there might be a better way to word this.

threatmodel.md Outdated Show resolved Hide resolved
threatmodel.md Outdated Show resolved Hide resolved
threatmodel.md Outdated Show resolved Hide resolved
JustinCappos and others added 4 commits April 13, 2021 14:27
Co-authored-by: Marina Moore <[email protected]>
Co-authored-by: Marina Moore <[email protected]>
Co-authored-by: Marina Moore <[email protected]>
Co-authored-by: Marina Moore <[email protected]>
2. Trying to have a party install an outdated image. For example, one with known security vulnerabilities.
3. Making images unavailable for installation.
4. Prevent a party from learning about updates to currently installed artifacts.
5. Convince a party to download large amounts of data, such as signatures or metadata, that interfere with the party's system.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is covered by the size already listed in the registry, so is probably out of scope for Notary

@mnm678 mnm678 mentioned this pull request Jun 21, 2021
@mnm678
Copy link
Contributor Author

mnm678 commented Aug 3, 2021

superceded by #76

@mnm678 mnm678 closed this Aug 3, 2021
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.

4 participants