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 Mithril Threat Model page on website #1726

Merged
merged 10 commits into from
Jun 19, 2024
Merged

Conversation

jpraynaud
Copy link
Member

@jpraynaud jpraynaud commented Jun 4, 2024

Content

This PR includes a new Mithril Threat Model page on the website based on the original Hackmd document:

Screenshot from 2024-06-04 11-55-04

Tip

In order to run locally the website, please run the following steps:

  • Pull this branch
  • Run cd docs/website from the root of the repository
  • Run npm install
  • Run make dev
  • Open your browser at http://localhost:3000/doc/next/mithril/threat-model
  • Pages are automatically refreshed in the browser when they markdown files are modified

Pre-submit checklist

  • Branch
    • Commit sequence broadly makes sense
    • Key commits have useful messages
  • PR
    • No clippy warnings in the CI
    • Self-reviewed the diff
    • Useful pull request description
    • Reviewer requested
  • Documentation
    • Update documentation website (if relevant)
    • Rotate documentation before merging

Issue(s)

Closes #1350

Copy link

github-actions bot commented Jun 4, 2024

Test Results

    4 files  ±0     51 suites  ±0   8m 45s ⏱️ -1s
1 073 tests ±0  1 073 ✅ ±0  0 💤 ±0  0 ❌ ±0 
1 221 runs  ±0  1 221 ✅ ±0  0 💤 ±0  0 ❌ ±0 

Results for commit 8582843. ± Comparison against base commit 56aa23e.

♻️ This comment has been updated with latest results.

@jpraynaud jpraynaud temporarily deployed to testing-preview June 4, 2024 10:09 — with GitHub Actions Inactive
@jpraynaud jpraynaud temporarily deployed to testing-sanchonet June 4, 2024 10:09 — with GitHub Actions Inactive
@ch1bo ch1bo self-requested a review June 10, 2024 07:46
docs/website/root/mithril/threat-model.md Outdated Show resolved Hide resolved
docs/website/root/mithril/threat-model.md Outdated Show resolved Hide resolved
docs/website/root/mithril/threat-model.md Outdated Show resolved Hide resolved
docs/website/root/mithril/threat-model.md Outdated Show resolved Hide resolved
docs/website/root/mithril/threat-model.md Outdated Show resolved Hide resolved
docs/website/root/mithril/threat-model.md Outdated Show resolved Hide resolved
docs/website/root/mithril/threat-model.md Outdated Show resolved Hide resolved
@jpraynaud jpraynaud marked this pull request as ready for review June 10, 2024 08:59
@jpraynaud jpraynaud temporarily deployed to testing-preview June 10, 2024 09:04 — with GitHub Actions Inactive
@jpraynaud jpraynaud temporarily deployed to testing-sanchonet June 10, 2024 09:04 — with GitHub Actions Inactive
@jpraynaud jpraynaud temporarily deployed to testing-preview June 10, 2024 09:41 — with GitHub Actions Inactive
@jpraynaud jpraynaud temporarily deployed to testing-sanchonet June 10, 2024 09:41 — with GitHub Actions Inactive
docs/website/root/mithril/threat-model.md Outdated Show resolved Hide resolved
Copy link
Collaborator

@sfauvel sfauvel left a comment

Choose a reason for hiding this comment

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

Just some typo issues on my side

docs/website/root/mithril/threat-model.md Outdated Show resolved Hide resolved
docs/website/root/mithril/threat-model.md Outdated Show resolved Hide resolved
docs/website/root/mithril/threat-model.md Outdated Show resolved Hide resolved
docs/website/root/mithril/threat-model.md Outdated Show resolved Hide resolved
docs/website/root/mithril/threat-model.md Outdated Show resolved Hide resolved
docs/website/root/mithril/threat-model.md Outdated Show resolved Hide resolved
Copy link
Collaborator

@rezabaram rezabaram left a comment

Choose a reason for hiding this comment

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

Here the assets are enumerated, but there is no analysis of the associated risks, neither the mitigations. Am I misunderstanding the scope of the document?

docs/website/root/mithril/threat-model.md Outdated Show resolved Hide resolved
docs/website/root/mithril/threat-model.md Outdated Show resolved Hide resolved
docs/website/root/mithril/threat-model.md Show resolved Hide resolved
docs/website/root/mithril/threat-model.md Show resolved Hide resolved
docs/website/root/mithril/threat-model.md Outdated Show resolved Hide resolved

* **availability**: No (? The key is not needed unless a re-genesis process is required, but then a new key could be used instead?)
* **confidentiality**: Yes
* **integrity**: Yes (?)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we say Genesis signing keys are tamper-proof?

Copy link
Member Author

Choose a reason for hiding this comment

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

@curiecrypt could you make a suggestion for this comment?
(I think that it will also apply to the "Era signing key" section)

docs/website/root/mithril/threat-model.md Outdated Show resolved Hide resolved
docs/website/root/mithril/threat-model.md Outdated Show resolved Hide resolved
Copy link
Member

@ch1bo ch1bo left a comment

Choose a reason for hiding this comment

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

System description contains most important things, but is a bit inconsistent on libp2p or not.

Assets already a good collection. We should order the CIA items consistently though.

Threats are clearly not complete. Probably can't hurt to also mention there that this is incomplete.

docs/website/root/mithril/threat-model.md Outdated Show resolved Hide resolved
docs/website/root/mithril/threat-model.md Outdated Show resolved Hide resolved
docs/website/root/mithril/threat-model.md Outdated Show resolved Hide resolved
docs/website/root/mithril/threat-model.md Outdated Show resolved Hide resolved
docs/website/root/mithril/threat-model.md Outdated Show resolved Hide resolved
docs/website/root/mithril/threat-model.md Outdated Show resolved Hide resolved
docs/website/root/mithril/threat-model.md Show resolved Hide resolved
docs/website/root/mithril/threat-model.md Show resolved Hide resolved
docs/website/root/mithril/threat-model.md Outdated Show resolved Hide resolved
docs/website/root/mithril/threat-model.md Outdated Show resolved Hide resolved
Copy link
Member

@ch1bo ch1bo left a comment

Choose a reason for hiding this comment

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

Just to be clear: I do approve of this document. "Done is better than perfect" and anything we can get published about this is good.

There are many good comments by reviewers which, when incorporated, makes this even better.

@jpraynaud jpraynaud temporarily deployed to testing-preview June 18, 2024 10:39 — with GitHub Actions Inactive
@jpraynaud jpraynaud temporarily deployed to testing-sanchonet June 18, 2024 10:39 — with GitHub Actions Inactive
@jpraynaud jpraynaud temporarily deployed to testing-preview June 18, 2024 14:45 — with GitHub Actions Inactive
@jpraynaud jpraynaud temporarily deployed to testing-sanchonet June 18, 2024 14:45 — with GitHub Actions Inactive
docs/website/root/mithril/threat-model.md Outdated Show resolved Hide resolved
docs/website/root/mithril/threat-model.md Outdated Show resolved Hide resolved
@jpraynaud jpraynaud force-pushed the ensemble/1350-threat-model-doc branch from f051325 to 8582843 Compare June 19, 2024 15:20
@jpraynaud jpraynaud temporarily deployed to testing-preview June 19, 2024 15:40 — with GitHub Actions Inactive
@jpraynaud jpraynaud temporarily deployed to testing-sanchonet June 19, 2024 15:40 — with GitHub Actions Inactive
@jpraynaud jpraynaud merged commit e4692ff into main Jun 19, 2024
43 checks passed
@jpraynaud jpraynaud deleted the ensemble/1350-threat-model-doc branch June 19, 2024 15:42
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.

Threat modeling and risk analysis
7 participants