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 threat model. #9

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Add threat model. #9

wants to merge 1 commit into from

Conversation

aekblad
Copy link
Contributor

@aekblad aekblad commented Oct 15, 2024

What changes are made in this PR?

This commit adds a formal threat model for the system.

Why are these changes needed?

We need a threat model if we're serious about security hardening this backend. Once we've done that, having one will increase people's confidence in it.

Related issues:

Closes #4.

@aekblad aekblad requested a review from a team as a code owner October 15, 2024 13:54

## Assumptions
- Malicious TF state can cause information leaks when used by TF client to deploy an application.
- TVB and Vault are deployed on a local network separated from the Internet.
Copy link
Contributor

Choose a reason for hiding this comment

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

As this is a public (open source) project, there is no limitation or recommendation to keep it hidden behind a corp. firewall. Lets assume the opposite in or threat model (TVB and Vault are deployed on the Internet) and adjust exposure to UIA accordingly.

We can use LAN deployment only as a Countermeasure, if there are no better options.

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 guess we just replace LNA with UIA then, and update exposure accordingly.

| UC1:3 | LNA | Return malicious state to user which steals data from deployment if deployed | High | Low | High | Protect traffic with TLS |
| UC1:3 | LNA | Eavesdrop on returned TF state to obtain secrets | High | Low | High | Protect traffic with TLS |
| UC1:3 | CTA | Leak secrets obtained from Vault | High | Low | High | Filter outbound traffic to prevent exfiltration; ensure dependencies are minimal, trustworthy, and up to date |
| UC1:3 | CVA | Return malicious state to user which DoSes deployment if deployed | Medium | Low | Low | Deploy IDS to increase chance of detection; implement gradual rollout and rollback strategy | UC1:3 | CVA | Return malicious state to user which steals data from deployment if deployed | High | Low | High | Deploy IDS to increase chance of detection; ensure Vault is kept up to date |
Copy link
Contributor

Choose a reason for hiding this comment

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

missing newline

| UC1:4 | UIA | Impersonate user to obtain secrets from TF state | High | High | High | Authenticate using Vault API key |
| UC1:4 | UIA | Return malicious state to user which steals data from deployment if deployed | High | Low | Low | Protect traffic with TLS |
| UC1:4 | UIA | Eavesdrop on TF state as it is sent to user to obtain secrets | High | Low | Medium | Protect traffic with TLS |
| UC1:1 | MUA | Impersonate another user | Low | Medium | Low | Use mTLS with individual certificates, log accesses |
Copy link
Contributor

Choose a reason for hiding this comment

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

Surely there are other ways (countermeasures) to prove your identity as well (not just client certificates)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, but I didn't want to recommend implementing full-blown user-management or OAuth integration for this relatively simple tool, since both would require significantly more work (and complexity) than just verifying that a client cert was signed by the right authority. Should I include them anyway?

Copy link
Contributor

Choose a reason for hiding this comment

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

I might reading this wrong ...

MUA == "Malicious User"

Is the user authenticated of or not ?

If the user is:

  1. Authenticated: it cannot pretend to be somebody else because the VAULT_TOKEN in his/her possession is personal (bound to the user when logging in through i.e. OIDC).
  2. Not authenticated: No valid VAULT_TOKEN => cannot access TVB or Vault at all.

Imo, the countermeasure is to log the identity behind the VAULT_TOKEN. That we it will be possible what TF state the MUA has gotten access to and/or modified.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, then I get it! I misunderstood the vault token; I thought it was a shared, long-lived one. That makes a lot more sense, and necessitates changes in a few more places as well.

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.

[Feature Request] Formal Threat Model for TVB
2 participants