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

cli: consul tls: create private keys with mode 0600 #11781

Merged
merged 2 commits into from Dec 21, 2021

Conversation

marco-m
Copy link
Contributor

@marco-m marco-m commented Dec 8, 2021

This applies to

consul tls ca create
consul tls cert create -client
consul tls cert create -server

Closes: #11741

This applies to

consul tls ca create
consul tls cert create -client
consul tls cert create -server

Closes: hashicorp#11741
@marco-m
Copy link
Contributor Author

marco-m commented Dec 15, 2021

Friendly ping @Amier3 :-)

@Amier3
Copy link
Contributor

Amier3 commented Dec 16, 2021

Thanks for the contribution @marco-m , it's much appreciated!

We'll try to get this reviewed and merged as soon as we can, i'll note that their may be a bit of a delay due to the holidays.

@dnephin dnephin added backport/1.10 theme/cli Flags and documentation for the CLI interface labels Dec 21, 2021
Copy link
Contributor

@dnephin dnephin left a comment

Choose a reason for hiding this comment

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

Thank you! This looks great.

I've pushed one commit which adds a changelog entry. That should re-run some of the flaky tests as well. If CI is still happy I will merge.

Comment on lines +128 to +130
if want, have := fs.FileMode(0600), fi.Mode().Perm(); want != have {
t.Fatalf("private key file %s: permissions: want: %o; have: %o", keyPath, want, have)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I generally we try to use require.Equal to match the other assertions, but this is ok too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hello @dnephin, I had noticed that consul uses testify; I went with this style is because I am used to it and thought that I could not express the same with testify, but now that I think about it, I could have passed the two integers to require.Assert; on the other hand I wanted to put in the error message also the file path... I should have searched before if there was a way to do so with testify.

@dnephin dnephin merged commit 1eb3178 into hashicorp:main Dec 21, 2021
@hc-github-team-consul-core
Copy link
Collaborator

🍒 If backport labels were added before merging, cherry-picking will start automatically.

To retroactively trigger a backport after merging, add backport labels and re-run https://circleci.com/gh/hashicorp/consul/535488.

@hc-github-team-consul-core
Copy link
Collaborator

🍒✅ Cherry pick of commit 1eb3178 onto release/1.11.x succeeded!

hc-github-team-consul-core pushed a commit that referenced this pull request Dec 21, 2021
cli: consul tls: create private keys with mode 0600
@hc-github-team-consul-core
Copy link
Collaborator

🍒✅ Cherry pick of commit 1eb3178 onto release/1.10.x succeeded!

hc-github-team-consul-core pushed a commit that referenced this pull request Dec 21, 2021
cli: consul tls: create private keys with mode 0600
@hc-github-team-consul-core
Copy link
Collaborator

🍒✅ Cherry pick of commit 1eb3178 onto release/1.9.x succeeded!

hc-github-team-consul-core pushed a commit that referenced this pull request Dec 21, 2021
cli: consul tls: create private keys with mode 0600
@marco-m marco-m deleted the private-key-0600-permission branch December 22, 2021 16:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
theme/cli Flags and documentation for the CLI interface
Projects
None yet
Development

Successfully merging this pull request may close these issues.

"consul tls ca create" creates a world-readable private key
4 participants