docs: add a topic about security #433
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rkratky
requested changes
Jan 11, 2024
There was a problem hiding this comment.
@daniloegea Thank you for the addition of the topic. I left some comments and suggestions.
doc/security.md
Outdated
| @@ -0,0 +1,45 @@ | |||
| --- | |||
| title: "Netplan Security" | |||
There was a problem hiding this comment.
Suggested change
| title: "Netplan Security" | |
| title: "Netplan security" |
doc/security.md
Outdated
|
|
||
| ## Storing credentials | ||
|
|
||
| Credentials, such as VPN keys and wifi passwords, are stored along with the rest |
There was a problem hiding this comment.
Suggested change
| Credentials, such as VPN keys and wifi passwords, are stored along with the rest | |
| Credentials, such as VPN keys and Wi-Fi passwords, are stored along with the rest |
doc/security.md
Outdated
| ## Storing credentials | ||
|
|
||
| Credentials, such as VPN keys and wifi passwords, are stored along with the rest | ||
| of the configuration in YAML files. Netplan expects that all your YAML files |
There was a problem hiding this comment.
"Netplan expects" is an anthropomorphism. Let's try to avoid it. In order to reword the sentence, could you please clarify if it is a hard requirement for the YAML files to be owned by root and only readable by root (i.e. would Netplan fail to function properly if this condition was not met)?
There was a problem hiding this comment.
It's not a hard requirement. I guess something like The recommended set of permissions is... would be better.
doc/security.md
Outdated
|
|
||
| Credentials, such as VPN keys and wifi passwords, are stored along with the rest | ||
| of the configuration in YAML files. Netplan expects that all your YAML files | ||
| will belong to the root user and only have permissions to be read by root (`chmod 600`). |
There was a problem hiding this comment.
Suggested change
| will belong to the root user and only have permissions to be read by root (`chmod 600`). | |
| belong to the root user and only have permissions to be read by root (`chmod 600`). |
Unless there's something really temporal happening, try to avoid using any other tense but present.
doc/security.md
Outdated
| run unit tests and the Netplan generator against a number of YAML files. This helps | ||
| us to detect issues, such as memory leaks and buffer overflows, at runtime using real | ||
| configuration as input. When a memory issue is detected the process will crash, letting | ||
| us know that some issue was introduced in the change. |
There was a problem hiding this comment.
Suggested change
| us know that some issue was introduced in the change. | |
| that some issue was introduced in the change. |
doc/security.md
Outdated
| configuration as input. When a memory issue is detected the process will crash, letting | ||
| us know that some issue was introduced in the change. | ||
|
|
||
| Every time a Pull Request is created or changes are merged to the main branch, |
There was a problem hiding this comment.
Suggested change
| Every time a Pull Request is created or changes are merged to the main branch, | |
| Every time a pull request is created or changes are merged to the main branch, |
doc/security.md
Outdated
| us know that some issue was introduced in the change. | ||
|
|
||
| Every time a Pull Request is created or changes are merged to the main branch, | ||
| these tests will be executed and, if a crash happens, the workflow will fail. |
There was a problem hiding this comment.
Suggested change
| these tests will be executed and, if a crash happens, the workflow will fail. | |
| CI executes these tests, and, if a crash happens, the workflow fails. |
doc/security.md
Outdated
| ## Binary package hardening | ||
|
|
||
| On Ubuntu and Debian, Netplan is built (and in fact most of the binary packages are) | ||
| with a number of security flags that will apply some hardening to the resulting binary. |
There was a problem hiding this comment.
Suggested change
| with a number of security flags that will apply some hardening to the resulting binary. | |
| with a number of security flags that apply some hardening to the resulting binary. |
doc/security.md
Outdated
| On Ubuntu and Debian, Netplan is built (and in fact most of the binary packages are) | ||
| with a number of security flags that will apply some hardening to the resulting binary. | ||
| That is intended to make the life of attackers harder in case any security issue is | ||
| discovered. See `dpkg-buildflags(1)` for details. |
There was a problem hiding this comment.
Suggested change
| discovered. See `dpkg-buildflags(1)` for details. | |
| discovered. See the `dpkg-buildflags(1)` manual page for details. |
daniloegea
force-pushed
the
security_topic_docs
branch
from
6c88df9
to
024a203
Compare
|
Thank you folks. I tried to address all the issues you have found. Please, let me know if there is anything else to improve. |
rkratky
approved these changes
Jan 17, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Checklist
make checksuccessfully.make check-coverage).