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

Better certificate protection - 600 #6652

Open
Golffies opened this issue Dec 7, 2024 · 6 comments · May be fixed by #6667
Open

Better certificate protection - 600 #6652

Golffies opened this issue Dec 7, 2024 · 6 comments · May be fixed by #6667
Labels
client feature-request This issue or PR deals with a new feature good first issue Good for first-time contributors linux macOS

Comments

@Golffies
Copy link

Golffies commented Dec 7, 2024

Hello,

on macOS, it seems to me that the certificate and private key used by Mumble are saved in a JSON file, located here:

/Users/<whoever>/Library/Preferences/Mumble/Mumble/mumble_settings.json

However, this file and its backup copy located in the same folder are created with 644 rights, and their content can therefore be read by any user:

-rw-r--r--@ 1 youruser  staff   4,6K  7 déc 12:18 /Users/youruser/Library/Preferences/Mumble/Mumble/mumble_settings.json
-rw-r--r--@ 1 youruser  staff   4,6K  7 déc 12:16 /Users/youruser/Library/Preferences/Mumble/Mumble/mumble_settings.json.back

Since the private key is present, it would be better for Mumble to apply more restricted rights to them, such as 600.

-rw-------@ 1 youruser  staff   4,6K  7 déc 12:18 /Users/youruser/Library/Preferences/Mumble/Mumble/mumble_settings.json
-rw-------@ 1 youruser  staff   4,6K  7 déc 12:16 /Users/youruser/Library/Preferences/Mumble/Mumble/mumble_settings.json.back

This becomes a more than academic concern when you consider importing into Mumble a personal certificate signed by an external certification authority and also used to sign and encrypt emails.

Obviously, on macOS, the best practice is to access the certificate and its private key from the Keychain, and not to export it from the Keychain to create a copy to be imported into Mumble. But restricting access rights to the JSON file is already a step towards greater security.

Note that on macOS, when an application is authorised to use the certificate and private key in the Keychain, the user is not asked to enter their session password every time. In the Keychain, the application can be declared as always having the right to access them.

Environment
macOS 14.6.1
Mumble 1.5.634

@Kissaki

This comment was marked as outdated.

@Kissaki Kissaki transferred this issue from mumble-voip/mumble-www Dec 7, 2024
@Hartmnt Hartmnt added client linux macOS feature-request This issue or PR deals with a new feature good first issue Good for first-time contributors labels Dec 8, 2024
@Hartmnt
Copy link
Member

Hartmnt commented Dec 8, 2024

Interesting. On Linux the same permissions are used, so this could also benefit from a change here.

@Hartmnt Hartmnt changed the title macOS - Better certificate protection - 600 Better certificate protection - 600 Dec 8, 2024
@theboywholived
Copy link

Can I work on this issue - I'm new to OSS and this repo.

@Krzmbrzl
Copy link
Member

Absolutely. The relevant part of the code is in https://github.com/mumble-voip/mumble/blob/master/src/mumble/Settings.cpp#L143

@theboywholived
Copy link

Sure - I will pick this up? Could you point me to anyway I can replicate this and test this fix once I have written the code?

@Krzmbrzl
Copy link
Member

Just run Mumble once and inspect the file permissions of the created settings JSON file (e.g. via ls -al).

theboywholived pushed a commit to theboywholived/mumble that referenced this issue Dec 14, 2024
Changes permission for settings.json and corresponding backup file to 600.

Fixes mumble-voip#6652
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
client feature-request This issue or PR deals with a new feature good first issue Good for first-time contributors linux macOS
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants