I reviewed google-osconfig-agent 20200625.00-0ubuntu2 as checked into groovy. This shouldn't be considered a full audit but rather a quick gauge of maintainability. There's far too much code here to provide detailed feedback.
google-osconfig-agent is a cloud-specific operating system management interface that provides a common user interface over several operating systems, to manage some aspects of configuration. - CVE History: I didn't see any in our database - Build-Depends? debhelper-compat (= 12), dh-golang, golang-any - pre/post inst/rm scripts? Mostly automatically added -- the $1 == upgrade path is done manually and should use "$1" instead. - init scripts? None - systemd units? Simply starts the agent, no hardening options - dbus services? None - setuid binaries? None - binaries in PATH? /usr/bin/google_osconfig_agent - sudo fragments? None - polkit files? None - udev rules? None - unit tests / autopkgtests? Moderate number of unit tests / e2e tests, I didn't try to run them myself - cron jobs? None - Build logs: Pretty clean build logs Multiple lintian errors and warnings W: google-osconfig-agent source: debhelper-compat-file-is-missing W: google-osconfig-agent source: package-uses-deprecated-debhelper-compat-version 1 E: google-osconfig-agent source: package-uses-debhelper-but-lacks-build-depends E: google-osconfig-agent source: missing-build-dependency debhelper - Processes spawned? There's a lot of process spawning, almost all of it with probably partially-trusted inputs. The executions usually looked safe from the usual system(3)-style errors, however the inputs into the functions may be less safe. - Memory management? Go program, fine. - File IO? File IO is a weak point of this program -- file operations aren't programmed using best practices and present reliability and security risks that I feel need remediation. - Logging? I don't think this is a significant problem with Go, I didn't see anything concerning in spot-checks. - Environment variable usage? Minimal use, looked fine - Use of privileged functions? Minimal use, looked fine - Use of cryptography / random number sources etc? Some weak random number sources are used but it didn't feel security-relevant - Use of temp files? I think these looked safe - Use of networking? It's hard to be sure but I think very little, maybe some grpc. - Use of WebKit? None - Use of PolicyKit? None - Any significant cppcheck results? None - Any significant Coverity results? Unable - Any significant shellcheck results? Minor - Any significant bandit results? None Security team wishes to defer a decision at the moment for promoting google-osconfig-agent to main. We need to package a newer version -- the move 07550 directories are addressed in a newer version. I haven't checked to see if any of the other findings have been addressed, but I wouldn't be surprised if the google-osconfig-agent team decides some of these issues need CVEs and security fixes. It'd be nice to have those taken care of before the package enters main. shellcheck found a very minor issue in debian/postrm; it's almost certainly not an issue. gosec results are long and many are useless, but Chmod return value must be checked, debugging endpoint in main.go must be confirmed as desirable, hardcoded directory permissions should be confirmed. Traversal and archive bomb issues should be confirmed. There's a lot of unchecked error returns. Quite a lot of the file IO in this program is not multiprocess-safe: operations are quite often performed as if no other processes on the system may interact with the files that this program is manipulating. Other processes may find partially-written files, files or directories with incorrect permissions, and may be able to race the various chown(2) or chmod(2) operations to exploit the system. This represents not only potential security issues but also potential reliability issues: half-written files or missing files or incorrect permissions have caused problems for decades. Here's some notes I took while reading, in the hopes that they are useful. Some of them may justify CVEs to ensure all consumers are alerted to the issues. stepCopyFile() writes directly to a destination file rather than a temporary file and renaming; if someone else owns the destination file, they'll control the new file, too. Without the rename(), writes are not atomic. doesn't check error returns on Chmod call NormPath() doesn't return errors in case of '/../' in pathnames; eg extractZip() doesn't do any name filtering extractZip() uses 755 for directories extractZip() doesn't try to skip writing through symlinks extractTar() uses 700 for the first directory extractTar() doesn't do any name filtering, extractTar() doesn't try to skip writing through symlinks extractTar() unpacks hardlinks, symlinks, device nodes, and fifos extractTar() looks like it'll chown and chtimes any file in the zip -- / ? /usr/sbin/ ? etc? extractTar() looks like it'll overwrite any file on the system I'll assume all the other decompressors are the same Each of these may need CVEs depending upon the purpose of these routines. stepExecFile() blindly changes permissions on a file to 0700 -- seems odd both to set the permissions so far away from where the file was written and also perhaps with such narrow permissions -- writeScript() uses 755 writeScript() should use fchmod() on the already-open filehandle rather than closing the file and then using chmod(), to avoid a race condition. policies/policies.go -- setConfig() creates directories with mode 07550, no comment nearby to describe why setuid, setgid, sticky directories are desired. Bug? This code was removed in https://github.com/GoogleCloudPlatform/osconfig/commit/13e6066b7170803de8834b2de5e15cc5d22d4779#diff-b10efb429c8ce33a17cfa521943096fe91d1f562748b44e163052c9a12114d95 policies/policies.go -- writeIfChanged() doesn't use atomic file handling methods, concurrent reads of the named file may find an empty file or partially written file. main_linux.go -- obtainLock() incorrect permissions on /run/lock directory default_cert.go -- getClientCertificate() what happens if the program specified in ~/.secureConnect/context_aware_metadata.json never returns any data? or returns terabytes? one byte every second forever? Who controls the ~/.secureConnect/context_aware_metadata.json file and do they cooperate with this function? Thanks ** Changed in: google-osconfig-agent (Ubuntu) Assignee: Ubuntu Security Team (ubuntu-security) => Balint Reczey (rbalint) -- You received this bug notification because you are a member of Ubuntu Bugs, which is subscribed to Ubuntu. https://bugs.launchpad.net/bugs/1891934 Title: [MIR] google-osconfig-agent To manage notifications about this bug go to: https://bugs.launchpad.net/ubuntu/+source/google-osconfig-agent/+bug/1891934/+subscriptions -- ubuntu-bugs mailing list ubuntu-bugs@lists.ubuntu.com https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs
