/etc/apm-server/apm-server.yml not owned by apm-server user

[axw]

Testing 7.0.0-beta1 on Ubuntu 16.04 x86-64.

• vagrant init --minimal elastic/ubuntu-16.04-x86_64
• vagrant up
• vagrant ssh -- wget https://artifacts.elastic.co/downloads/apm-server/apm-server-7.0.0-beta1-amd64.deb
• vagrant ssh -- sudo dpkg -i apm-server-7.0.0-beta1-amd64.deb
• vagrant ssh -- apm-server export config

Exits with status code 1. Output:

error initializing beat: error loading config file: config file ("/etc/apm-server/apm-server.yml") must be owned by the beat user (uid=1000) or root

• /usr/bin/apm-server is owned by root
• /etc/apm-server/apm-server.conf is owned by apm-server (uid 998), and has perms 0600.

[axw]

Same thing happens on CentOS, though there it says beat user (uid=0) or root.

[inqueue]

I am seeing the same on CentOS running apm-server-7.0.0_rc1-1.x86_64.

[graphaelli]

This is the behavior I'd expect given the strict permissions are on by default.

# ok
vagrant ssh -- sudo -u apm-server apm-server export config
# also ok
vagrant ssh -- sudo apm-server --strict.perms=false export config

[inqueue]

This is the behavior I'd expect given the strict permissions are on by default.

With a package installation?

[graphaelli]

Yes, with a package installation. In this case the commands are being run as the vagrant user, which shouldn't be able to read the apm-server configuration by default.

[inqueue]

Ah, maybe it is a different issue. In my case, apm-server would not start directly after upgrade to 7.0.0-rc1 from 6.7.0 without modifying ownership of apm-server.yml as the file was owned by the system apm-server user.

[graphaelli]

@inqueue Thanks for clarifying. The upgrade path is indeed a bit different and there might be an issue in there. #1401 (comment) details the original decision making and again was revisited in #1833 but those docs don't actually exist yet.

[axw]

@graphaelli

This is the behavior I'd expect given the strict permissions are on by default.

That seems reasonable, but the error message isn't clear to me. It says it should be owned by the "beat" user or root. Should that be the apm-server user or root? Maybe that's something in libbeat that needs to be overridden? At any rate, the problem seems to be rather that the user executing apm-server is not the file owner.

Is this mentioned in the docs anywhere? I was approaching this like a user would (I hope), and followed the docs. https://www.elastic.co/guide/en/apm/server/current/setting-up-and-running.html shows running apm-server without any sudo/su.

Finally, do you think it would be safe and reasonable to make the apm-server executable setuid?

[axw]

Just found that sudo -u apm-server apm-server export config works on Ubuntu, but not on CentOS 7:

[vagrant@localhost ~]$ sudo -u apm-server apm-server -e export config
error initializing beat: could not initialize the keystore: open apm-server.keystore: permission denied

Using sudo su - apm-server, it does work. Seems like an issue with effective UIDs?

[graphaelli]

Just found that sudo -u apm-server apm-server export config works on Ubuntu, but not on CentOS 7:

This is a great find! elastic/beats#11325 is up to fix this.
The problem is apm-server is trying to read they keystore from the current directory and failing:

vagrant@centos-7 $ sudo -u apm-server apm-server export config
error initializing beat: could not initialize the keystore: open apm-server.keystore: permission denied
vagrant@centos-7 $ pwd
/home/vagrant
vagrant@centos-7 $ sudo -u apm-server ls -ld .
ls: cannot access .: Permission denied

on ubuntu the permissions on ~vagrant happen to be more permissive:

[email protected] $ pwd
/home/vagrant
[email protected] $ sudo -u apm-server ls -ld .
drwxr-xr-x 7 vagrant vagrant 4096 Mar 19 21:00 .

Regarding the earlier comments:

Should that be the apm-server user or root? Maybe that's something in libbeat that needs to be overridden?

Yes, that is indeed in libbeat here and here. #2026 opened to address those types of issues.

Is this mentioned in the docs anywhere?
No, #2022 is tracking that

Finally, do you think it would be safe and reasonable to make the apm-server executable setuid?

Unfortunately I wasn't able to find a way to make this safe and useful at the same time. If we make it world executable, the useful case, any user could gain access to secrets stored in the config, like

apm-server:
  host: localhost:8200
  rum:
    source_mapping:
      elasticsearch:
        password: changeme
        username: elastic
  ssl:
    key_passphrase: secret

Less that world executable would be safer but I'm not sure there is much use in that case. Please chime in if I've missed something.

[axw]

Unfortunately I wasn't able to find a way to make this safe and useful at the same time. If we make it world executable, the useful case, any user could gain access to secrets stored in the config, like

Ah because export config will export the secret. I was thinking that the process would only read the config, and if we ensured that secrets weren't logged (which we should anyway), then we would be fine.

Maybe just complicating things, and makes doing the wrong thing too easy for comfort, but an alternative would be:

• make it world executable, suid bit set
• require uid==apm-server for "export config", not just euid==apm-server
• probably same as above for "keystore" commands

sudo -u apm-server apm-server export config would work (it sets uid not euid), but apm-server export config would not; it should fail and tell you to use sudo.

[graphaelli]

I had not considered enforcing different perms based on the subcommand, I like the solution of enforcing a uid check for commands that could leak information but I'm also concerned about how easy it would be to add a new command subcommand and get this wrong. Since that would be a feature enhancement, I'm going to drop priority on this for now.

@inqueue If you do find any issue with permissions during upgrade please do open a separate issue.

[inqueue]

If you do find any issue with permissions during upgrade please do open a separate issue.

#2022 will cover my findings.

[graphaelli]

Closing as we did not address it in time for 7.0 and won't consider it again until 8.0