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

doc: Add initial 'Cryptography' section #517

Merged
merged 2 commits into from
Sep 16, 2024

Conversation

slyon
Copy link
Collaborator

@slyon slyon commented Sep 11, 2024

Description

Add initial 'Cryptography' section do our "Security" documentation.

Also include some new tooling in tools/grep_cryptography.sh, to identify cryptography related terminology in our codebase.

FR-7556

Checklist

  • Runs make check successfully.
  • Retains code coverage (make check-coverage).
  • New/changed keys in YAML format are documented.
  • (Optional) Adds example YAML for new feature.
  • (Optional) Closes an open bug in Launchpad.

@slyon slyon marked this pull request as ready for review September 12, 2024 07:52
@slyon
Copy link
Collaborator Author

slyon commented Sep 12, 2024

@CodiumAI-Agent /review
--pr_reviewer.inline_code_comments=true
--pr_reviewer.require_score_review=true
--pr_reviewer.require_can_be_split_review=true
--pr_reviewer.require_soc2_ticket=true
--pr_reviewer.num_code_suggestions="2"

@CodiumAI-Agent
Copy link

CodiumAI-Agent commented Sep 12, 2024

PR Reviewer Guide 🔍

(Review updated until commit 4e21d25)

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🏅 Score: 85
🧪 No relevant tests
🔒 No security concerns identified
🔀 Multiple PR themes

Sub-PR theme: Add tooling for identifying cryptographic terms

Relevant files:

  • tools/grep_cryptography.sh

Sub-PR theme: Document cryptographic technologies and configurations

Relevant files:

  • doc/security.md

⚡ Key issues to review

Possible Bug
The comment correction from '265bit' to '256 bit' is good, but the actual regex pattern and logic for validating a WireGuard key might not be robust enough to ensure it's a valid key. Consider adding more rigorous checks or using a dedicated library for validation.

src/validation.c Show resolved Hide resolved
tools/grep_cryptography.sh Show resolved Hide resolved
@slyon slyon added the documentation Documentation improvements. label Sep 12, 2024
Copy link
Collaborator

@daniloegea daniloegea left a comment

Choose a reason for hiding this comment

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

I left 2 comments in the text that we might want to address

doc/security.md Outdated Show resolved Hide resolved
doc/security.md Outdated Show resolved Hide resolved
Also include some new tooling in tools/grep_cryptography.sh, to indentify
cryptography related terminiology in our codebase.
Copy link
Collaborator

@daniloegea daniloegea left a comment

Choose a reason for hiding this comment

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

Thanks, it looks good to me.

@slyon slyon merged commit e0e8711 into canonical:main Sep 16, 2024
17 checks passed
@CodiumAI-Agent
Copy link

Persistent review updated to latest commit 4e21d25

src/validation.c Show resolved Hide resolved
tools/grep_cryptography.sh Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Documentation improvements.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants