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

Upgrade setting permissions on weewx.conf #948

Closed
ps-crawford opened this issue Mar 31, 2024 · 6 comments
Closed

Upgrade setting permissions on weewx.conf #948

ps-crawford opened this issue Mar 31, 2024 · 6 comments
Assignees

Comments

@ps-crawford
Copy link

It appears that the upgrade process for WeeWX re-sets permissions on the /etc/weewx directory including weewx.conf and that makes the config file world-readable. However, that file can include user name and password combinations for interfacing to 3rd party sites for ftp upload, Wunderground, etc, and those should not be readable by world+dog on any multi-user system.

Would it make more sense for that file to be always set to 660 permission and any user that needs non-sudo ability to edit or read the file is then added to the weewx group?

@tkeffer
Copy link
Contributor

tkeffer commented Mar 31, 2024

Good point. We'll take a look at it.

@matthewwall
Copy link
Contributor

matthewwall commented Apr 7, 2024

which files should have 660 permissions?

each time the config file is modified by weectl, weectl makes a copy. weectl does not ensure 660 permissions.

during and upgrade, there may be many copies of the config file. all of those should have the same permissions as the 'active' config file.

if it is weewx-multi configuration, there will be multiple 'active' config files.

so i'm not sure what the right approach is here.

option 1: only modify permissions and ownership when upgrading from a v4 to v5

option 2: always set permissions and ownership

option 3: always set permissions and ownership, but set permissions on every /etc/weewx/*.conf to 660

@ps-crawford
Copy link
Author

The only file that bothered me from a security perspective is the /etc/weewx/weewx.conf file as it has login details for 3rd party sites that might be sensitive information. You are right to point out that often has version copies/diffs placed in the same directory with differing names (sometimes appended to the '.conf' side) so all potentially have such information.

In terms of options then your 'option 3' seems the simplest and safest. My initial thought was based on looking at the pkg/debian/postinst file and changing it so following the (current) line 427 it removes other read/write permission, so something like this:

set_config_permissions() {
    echo "Setting permissions $WEEWX_USER:$WEEWX_GROUP on /etc/weewx"
    set_permissions $WEEWX_USER $WEEWX_GROUP /etc/weewx
    chmod o-rw /etc/weewx/*.conf*
}

This might not be perfect as there remains a window of opportunity from setting all files globally readable to removing that on the conf file(s) but given:

  • Very short time they would be accessible
  • Infrequent nature of such upgrade changes
  • Few cases where a WeeWX machine has multiple users of low trust

It would seem quite acceptable to me.

@matthewwall
Copy link
Contributor

chmod o-rw /etc/weewx/.conf

this is what i was thinking.

@ps-crawford
Copy link
Author

I don't understand much about the package manager process, but that seemed like the sort of change that ought to work.

@matthewwall
Copy link
Contributor

this will appear in weewx 5.1.0

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