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

private keys shouldn't be world-readable #82

Closed
EvilBit opened this issue Jul 14, 2021 · 8 comments
Closed

private keys shouldn't be world-readable #82

EvilBit opened this issue Jul 14, 2021 · 8 comments

Comments

@EvilBit
Copy link
Contributor

EvilBit commented Jul 14, 2021

All key material, including private keys, are created world-readable (0644 here) at the moment.

Exposing this secret key material to unprivileged users and processes poses a security vulnerability.

@EvilBit
Copy link
Contributor Author

EvilBit commented Jul 14, 2021

Oh, and by the way, thanks a lot for this great tool - finally a workable secure boot management tool :)

@Foxboron
Copy link
Owner

Yolo, copy-pasted code. Thanks :)

@ericonr
Copy link
Contributor

ericonr commented Jul 15, 2021

Interesting, my keys are 600... I wonder when the regression happened.

@EvilBit
Copy link
Contributor Author

EvilBit commented Jul 15, 2021

Interesting, my keys are 600... I wonder when the regression happened.

I just traversed backwards through git blame and the permissions in SaveKey were 0644 since the initial commit.

But here the initial creation of the files was removed from CreateKey (which set 0600) and relied now on the erroneous permissions in SaveKey.

@EvilBit
Copy link
Contributor Author

EvilBit commented Jul 15, 2021

Maybe the code should even check for too permissive file permissions and refuse signing in this case. Like OpenSSH enforces it for private keys.

@ericonr
Copy link
Contributor

ericonr commented Jul 16, 2021

I think that's reasonable, but please with better error messages :p

@Foxboron
Copy link
Owner

Foxboron commented Jul 16, 2021

We can just do an var ErrInvalidPrivKeyPerms = errors.new("...something") and match it in main.go to give something human readable error back.

https://github.com/Foxboron/sbctl/blob/master/cmd/sbctl/main.go#L69-L78

@Foxboron
Copy link
Owner

Fixed with ea325ca

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

No branches or pull requests

3 participants