Skip to content

Commit 590de9e

Browse files
Revert config files permission check (#672)
* Revert "Add workaround for occassional AMI test failure on CI during chmod postinst phase (#665)" This reverts commit a782534 * Revert "Don't fail if chmod step as part of postinstall fails." This reverts commit 1a081f1 * Revert "chmod step failing should not be fatal." This reverts commit d0e0a9d * Revert "Better handling of default agent config file permissions (#657)" This reverts commit 6195a81
1 parent f162fe3 commit 590de9e

File tree

14 files changed

+17
-510
lines changed

14 files changed

+17
-510
lines changed

CHANGELOG.md

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
11
Scalyr Agent 2 Changes By Release
22
=================================
33

4-
## 2.1.15 "Endora" - December 15, 2020
4+
## 2.1.15 "Endora" - December 16, 2020
55

66
<!---
7-
Packaged by Tomaz Muraus <tomaz@scalyr.com> on Dec 2, 2020 14:00 -0800
7+
Packaged by Arthur Kamalov <arthur@scalyr.com> on Dec 16, 2020 14:00 -0800
88
--->
99
Feature:
1010
* Ability to upload logs to different Scalyr team accounts by specifying different API keys for different log files. See [RELEASE_NOTES](https://github.com/scalyr/scalyr-agent-2/blob/master/RELEASE_NOTES.md) for more details.
@@ -16,7 +16,6 @@ Improvements:
1616

1717
Misc:
1818
* The default value for the `k8s_cri_query_filesystem` Kubernetes monitor config option (set via the `SCALYR_K8S_CRI_QUERY_FILESYSTEM` environment var) has changed to `True`. This means that by default when in CRI mode, the monitor will only query the filesystem for the list of active containers, rather than first querying the Kubelet API. If you wish to revert to the original default to prefer using the Kubelet API, set `SCALYR_K8S_CRI_QUERY_FILESYSTEM` the environment variable to "false" for the Scalyr Agent daemonset.
19-
* On startup and when parsing a config file, agent now emits a warning if the config file is readable by others.
2019
* New ``global_monitor_sample_interval_enable_jitter`` config option has been added which is enabled by default. When this option is enabled, random sleep between 2/10 and 8/10 of the configured monitor sample gather interval is used before gathering the sample for the first time. This ensures that sample gathering for all the monitors doesn't run at the same time. This comes in handy when running agent configured with many monitors on lower powered devices to spread the monitor sample gathering related load spike across a longer time frame.
2120

2221
Bug fixes:
@@ -26,9 +25,6 @@ Bug fixes:
2625
* Update Windows System Metrics monitor to better handle a situation when disk io counters are not available.
2726
* Docker monitor has been fixed that when running in "API mode" (``docker_raw_logs: false``) it also correctly ingests logs from container ``stderr``. Previously only logs from ``stdout`` have been ingested.
2827

29-
Security fixes and improvements:
30-
* Agent installation artifacts have been updated so the default ``agent.json`` file which is bundled with the agent is not readable by "other" system users by default anymore. For more context, details and impact, please see [RELEASE_NOTES](https://github.com/scalyr/scalyr-agent-2/blob/master/RELEASE_NOTES.md).
31-
3228
## 2.1.14 "Hydrus" - November 4, 2020
3329

3430
<!---

RELEASE_NOTES.md

Lines changed: 0 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -170,57 +170,6 @@ NOTE: If the ``api_key_id`` is omitted, then the ``default`` API key is used.
170170
by default so if you want to specify custom options for it, you need to add
171171
``implicit_metric_monitor: false,`` option to the config as well.
172172

173-
* This release fixes default permissions for the ``agent.json`` file and ``agent.d/`` directory
174-
and ``*.json`` files inside that directory and makes sure those files are not readable by
175-
others by default (aka "other" permission bit in octal notation for that file is ``0`` in case
176-
of a file and ``1`` in case of a directory).
177-
178-
In the previous releases, ``agent.json`` file was readable by others by default which means if a
179-
user didn't explicitly lock down and change the permission of that file after installation,
180-
other users who have access to the system where the agent is running on could potentially read
181-
that file and access information such as the API key which is used to send / upload logs to
182-
Scalyr.
183-
184-
This issue only affects users which run agent on servers with multiple users. Container
185-
installations (Docker, Kubernetes) are single user by default so those are not affected.
186-
187-
This issue also doesn't affect any installations which use a configuration management tool such
188-
as Ansible, Chef, Puppet, Terraform or similar to manage the config file content and permissions.
189-
190-
In fact, using configuration management tool to manage configuration file access and locking down
191-
the permission is the best practice and recommended approach by us.
192-
193-
As part of the fix, agent will now lock down those file permissions and also fix them on upgrade
194-
as part of the post install step for the existing files and installations.
195-
196-
If you intentionally changed "other" permission bits for any of those files to something else than
197-
``0``, you will need to change it again after installing / upgrading the agent.
198-
199-
If you believe you may be affected, we recommend revoking the old write API key used to send logs
200-
and generating a new one.
201-
202-
Keep in mind that the API key used by the agent is a write one which only has access to send logs
203-
and nothing else.
204-
205-
It's also worth noting that this only affects agent.json file bundled with the default agent
206-
installation. If you manually removed that file and created a new one, that is out of the agent
207-
scope and domain of ``umask`` on Linux.
208-
209-
On Windows, permissions are not rectified automatically because some users run the agent under a
210-
custom non-Administrator user account so automatically fixing the permissions would break this
211-
scenario.
212-
213-
In this case, user can manually run ``scalyr-agent-2-config.exe`` as administrator to revoke
214-
permissions for "Users" group for the agent config.
215-
216-
```powershell
217-
& "C:\Program Files (x86)\Scalyr\config\scalyr-agent-2-config.exe" "--fix-config-permissions"
218-
```
219-
220-
Keep in mind that after running this script you need to use Administrator account to grant read
221-
permissions to user account which is used to run the agent in case this user is not Administrator
222-
or not a member of Administrators group.
223-
224173
## 2.1.8 "Titan" - August 3, 2020
225174

226175
* The `status -v` and the new `status -H` command contain health check information and will have a return code

build_package.py

Lines changed: 3 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -294,13 +294,11 @@ def build_win32_installer_package(variant, version):
294294
)
295295

296296
# Copy the config file.
297-
agent_json_path = make_path(agent_source_root, "config/agent.json")
298297
cat_files(
299-
[agent_json_path], "agent_config.tmpl", convert_newlines=True,
298+
[make_path(agent_source_root, "config/agent.json")],
299+
"agent_config.tmpl",
300+
convert_newlines=True,
300301
)
301-
# NOTE: We in intentionally set this permission bit for agent.json to make sure it's not
302-
# readable by others.
303-
os.chmod(agent_json_path, int("640", 8))
304302

305303
os.chdir("..")
306304
# We need to place a 'setup.py' here so that when we executed py2exe it finds it.
@@ -340,10 +338,6 @@ def build_win32_installer_package(variant, version):
340338
make_directory("Scalyr/logs")
341339
make_directory("Scalyr/data")
342340
make_directory("Scalyr/config/agent.d")
343-
# NOTE: We in intentionally set this permission bit for agent.d directory to make sure it's not
344-
# readable by others.
345-
os.chmod("Scalyr/config/agent.d", int("741", 8))
346-
347341
os.rename(os.path.join("dist", "scalyr-agent-2"), convert_path("Scalyr/bin"))
348342
shutil.copy(
349343
make_path(agent_source_root, "win32/ScalyrShell.cmd"),
@@ -591,9 +585,6 @@ def build_common_docker_and_package_files(create_initd_link, base_configs=None):
591585

592586
# Make sure there is an agent.d directory regardless of the config directory we used.
593587
make_directory("root/etc/scalyr-agent-2/agent.d")
594-
# NOTE: We in intentionally set this permission bit for agent.d directory to make sure it's not
595-
# readable by others.
596-
os.chmod("root/etc/scalyr-agent-2/agent.d", int("741", 8))
597588

598589
# Create the links to the appropriate commands in /usr/sbin and /etc/init.d/
599590
if create_initd_link:
@@ -843,16 +834,6 @@ def build_rpm_or_deb_package(is_rpm, variant, version):
843834
" --directories /usr/share/scalyr-agent-2 "
844835
" --directories /var/lib/scalyr-agent-2 "
845836
" --directories /var/log/scalyr-agent-2 "
846-
# NOTE: By default fpm won't preserve all the permissions we set on the files so we need
847-
# to use those flags.
848-
# If we don't do that, fpm will use 77X for directories and we don't really want 7 for
849-
# "group"
850-
" --rpm-use-file-permissions --deb-use-file-permissions "
851-
# NOTE: Sadly we can't use defattrdir since it breakes permissions for some other
852-
# directories such as /etc/init.d and we need to handle that in postinst :/
853-
# " --rpm-auto-add-directories "
854-
# " --rpm-defattrfile 640"
855-
# " --rpm-defattrdir 751"
856837
" -C root usr etc var" % (package_type, version, iteration_arg, description),
857838
exit_on_fail=True,
858839
command_name="fpm",
@@ -892,9 +873,6 @@ def build_tarball_package(variant, version, no_versioned_file_name):
892873
make_directory("scalyr-agent-2/data")
893874
make_directory("scalyr-agent-2/log")
894875
make_directory("scalyr-agent-2/config/agent.d")
895-
# NOTE: We in intentionally set this permission bit for agent.d directory to make sure it's not
896-
# readable by others.
897-
os.chmod("scalyr-agent-2/config/agent.d", int("741", 8))
898876

899877
# Create a file named packageless. This signals to the agent that
900878
# this a tarball install instead of an RPM/Debian install, which changes
@@ -1019,12 +997,8 @@ def build_base_files(base_configs="config"):
1019997
config_path = base_configs
1020998
else:
1021999
config_path = "config"
1022-
10231000
shutil.copytree(make_path(agent_source_root, config_path), "config")
10241001

1025-
# Make sure config file has 640 permissions
1026-
os.chmod("config/agent.json", int("640", 8))
1027-
10281002
# Create the trusted CA root list.
10291003
os.chdir("certs")
10301004
cat_files(

installer/scripts/postinstall.sh

Lines changed: 5 additions & 104 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@
1313
# See the License for the specific language governing permissions and
1414
# limitations under the License.
1515

16+
17+
1618
# Used below to execute a command to retrieve the Python interpreter version.
1719
run_and_check_persion_version() {
1820
command=$1
@@ -78,120 +80,19 @@ check_python_version() {
7880
echo "Warning, no valid Python interpreter found."
7981
}
8082

81-
# Function which ensures that the provided file path permissions for "group" match the
82-
# provided permission bit in octal notation.
83-
# This function can operate on a file or on a directory.
84-
ensure_path_group_permissions() {
85-
file_path=$1
86-
wanted_permissions_group=$2
87-
88-
if [ "${wanted_permissions_group}" -lt 0 ] || [ "${wanted_permissions_group}" -gt 7 ]; then
89-
echo "wanted_permissions_group value needs to be between 0 and 7"
90-
return 1
91-
fi
92-
93-
# Will output permissions on octal mode - xyz, e.g. 644
94-
file_permissions=$(stat -c %a "${file_path}")
95-
# Permissions for owner - e.g. 6
96-
file_permissions_owner=$(echo -n "$file_permissions" | head -c 1)
97-
# Permissions for group - e.g. 4
98-
file_permissions_group=$(echo -n "$file_permissions" | head -c 2 | tail -c 1)
99-
# Permissions for other - e.g. 4
100-
file_permissions_others=$(echo -n "$file_permissions" | tail -c 1)
101-
102-
# NOTE: We re-use existing fs permissions for owner and other
103-
if [ "${file_permissions_group}" -ne "${wanted_permissions_group}" ]; then
104-
new_permissions="${file_permissions_owner}${wanted_permissions_group}${file_permissions_others}"
105-
echo "Changing permissions for file ${file_path} from \"${file_permissions}\" to \"${new_permissions}\"."
106-
107-
# NOTE: On CI chmod sometimes fails with 'getpwuid(): uid not found: 1001' which is likely
108-
# related to some unfinished provisioning on the CI or similar. We simply ignore any
109-
# errors returned by chmod.
110-
set +e
111-
set +o pipefail
112-
chmod "${new_permissions}" "${file_path}" > /dev/null 2>&1 || true;
113-
set -e
114-
set -o pipefail
115-
fi
116-
}
117-
118-
# Function which ensures that the provided file path permissions for "other" users match the
119-
# provided permission bit in octal notation.
120-
# This function can operate on a file or on a directory.
121-
ensure_path_other_permissions() {
122-
file_path=$1
123-
wanted_permissions_other=$2
124-
125-
if [ "${wanted_permissions_other}" -lt 0 ] || [ "${wanted_permissions_other}" -gt 7 ]; then
126-
echo "wanted_permissions_other value needs to be between 0 and 7"
127-
return 1
128-
fi
129-
130-
# Will output permissions on octal mode - xyz, e.g. 644
131-
file_permissions=$(stat -c %a "${file_path}")
132-
# Permissions for owner and group - e.g. 644
133-
file_permissions_owner_group=$(echo -n "$file_permissions" | head -c 2)
134-
# Permissions for other - e.g. 4
135-
file_permissions_others=$(echo -n "$file_permissions" | tail -c 1)
136-
137-
# NOTE: We re-use existing fs permissions for owner and group
138-
if [ "${file_permissions_others}" -ne "${wanted_permissions_other}" ]; then
139-
new_permissions="${file_permissions_owner_group}${wanted_permissions_other}"
140-
echo "Changing permissions for file ${file_path} from \"${file_permissions}\" to \"${new_permissions}\"."
141-
142-
# NOTE: On CI chmod sometimes fails with 'getpwuid(): uid not found: 1001' which is likely
143-
# related to some unfinished provisioning on the CI or similar. We simply ignore any
144-
# errors returned by chmod.
145-
set +e
146-
set +o pipefail
147-
chmod "${new_permissions}" "${file_path}" > /dev/null 2>&1 || true;
148-
set -e
149-
set -o pipefail
150-
fi
151-
}
152-
153-
# Function which ensures that the provided file path is not readable by "other" users aka has
154-
# "0" value for permission in the octal notation. If permissions don't match, we update them
155-
# and ensure value for the user part is "0".
156-
# This function can operate on a file or on a directory.
157-
ensure_path_not_readable_by_others() {
158-
file_path=$1
159-
ensure_path_other_permissions "${file_path}" "0"
160-
}
161-
16283
check_python_version
16384

164-
config_owner=$(stat -c %U /etc/scalyr-agent-2/agent.json)
165-
script_owner=$(stat -c %U /usr/share/scalyr-agent-2/bin/scalyr-agent-2)
85+
config_owner=`stat -c %U /etc/scalyr-agent-2/agent.json`
86+
script_owner=`stat -c %U /usr/share/scalyr-agent-2/bin/scalyr-agent-2`
16687

16788
# Determine if the agent had been previously configured to run as a
168-
# different user than root. We can determine this if agent.json
89+
# different user than root. We can determine this if agentConfig.json
16990
# has a different user. If so, then make sure the newly installed files
17091
# (like agent.sh) are changed to the correct owners.
17192
if [ "$config_owner" != "$script_owner" ]; then
172-
echo "Changing owner for /etc/scalyr-agent-2/agent.json file from $script_owner to $config_owner"
17393
/usr/share/scalyr-agent-2/bin/scalyr-agent-2-config --set_user "$config_owner" > /dev/null 2>&1;
17494
fi
17595

176-
# Ensure /etc/scalyr-agent-2/agent.json file is not readable by others
177-
ensure_path_not_readable_by_others "/etc/scalyr-agent-2/agent.json"
178-
179-
# We also change agent.d group permissions to 751 since it used to be 771 due to default fpm behavior
180-
ensure_path_group_permissions "/etc/scalyr-agent-2/agent.d" "5"
181-
182-
# Ensure agent.d/*.json files are note readable by others
183-
# NOTE: Most software gives +x bit on *.d directories so we do the same
184-
ensure_path_other_permissions "/etc/scalyr-agent-2/agent.d" "1"
185-
186-
if [ -d "/etc/scalyr-agent-2/agent.d" ]; then
187-
# NOTE: find + -print0 correctly handles whitespaces in filenames and it's more robust than for,
188-
# but it may have cross platform issues. In that case we may need to revert to for.
189-
#for config_fragment_path in /etc/scalyr-agent-2/agent.d/*.json; do
190-
find /etc/scalyr-agent-2/agent.d/ -name "*.json" -print0 | while read -r -d $'\0' config_fragment_path; do
191-
ensure_path_not_readable_by_others "${config_fragment_path}"
192-
done
193-
fi
194-
19596
# Add in the symlinks in the appropriate /etc/rcX.d/ directories
19697
# to stop and start the service at boot time.
19798
if [ -f /sbin/chkconfig ] || [ -f /usr/sbin/chkconfig ]; then

scalyr_agent/config_main.py

Lines changed: 0 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,6 @@
8181
from scalyr_agent.platform_controller import PlatformController
8282
from scalyr_agent import compat
8383

84-
from scalyr_agent.util import win_remove_user_file_path_permissions
8584
import scalyr_agent.util as scalyr_util
8685

8786

@@ -1432,23 +1431,6 @@ def make_symlink(source, target):
14321431
help="Starts the agent if the conditional restart file marker exists.",
14331432
)
14341433

1435-
if sys.platform.startswith("win"):
1436-
# Special flag which is used by the Windows installer. We use it to indicate to this binary
1437-
# to fix up permissions for agent.json file and agent.d/ directory. This file is also used
1438-
# to create initial config by the install which means we can't correctly set those
1439-
# permissions inside the .wxs wix spec file.
1440-
# Right now it can only be used with "--init-config" flag on Windows.
1441-
parser.add_option(
1442-
"",
1443-
"--fix-config-permissions",
1444-
dest="fix_config_permissions",
1445-
action="store_true",
1446-
default=False,
1447-
help=(
1448-
"Fix permissions for agent.json file and agent.d/ directory and make sure it's. "
1449-
"not readable by others. Applies to Windows only."
1450-
),
1451-
)
14521434
(options, args) = parser.parse_args()
14531435
if len(args) > 1:
14541436
print("Could not parse commandline arguments.", file=sys.stderr)
@@ -1543,24 +1525,6 @@ def make_symlink(source, target):
15431525

15441526
controller.consume_config(config_file, options.config_filename)
15451527

1546-
if sys.platform.startswith("win") and options.fix_config_permissions:
1547-
print(
1548-
"Changing permissions for agent.json and agent.d and making sure it's not readable "
1549-
"by Users"
1550-
)
1551-
1552-
config_path = options.config_filename
1553-
configs_directory = os.path.dirname(config_path)
1554-
agent_json_path = os.path.join(configs_directory, "agent.json")
1555-
agent_d_path = os.path.join(configs_directory, "agent.d")
1556-
1557-
win_remove_user_file_path_permissions(
1558-
file_path=agent_json_path, username="Users"
1559-
)
1560-
win_remove_user_file_path_permissions(file_path=agent_d_path, username="Users")
1561-
1562-
print("Permissions updated.")
1563-
15641528
# See if we have to start the agent. This is only used by Windows right now as part of its install process.
15651529
if sys.platform.startswith("win") and options.mark_conditional_restart:
15661530
mark_conditional_restart(controller, config_file)

0 commit comments

Comments
 (0)