Comment 11 for bug 1953341

Revision history for this message
Mark Esler (eslerm) wrote :

I reviewed pcs 0.11.4-1ubuntu3 as checked into lunar. This shouldn't be considered a full audit but rather a quick gauge of maintainability.

- CVE History:
  - eleven past CVEs
  - lunar package currenty affected by CVE-2022-2735
  - has a Security Policy \o/
    - https://github.com/ClusterLabs/pcs/blob/main/SECURITY.md
  - upstream communicates vulnerabilities well
  - upsteam responds to reports quickly
- Build-Depends?
  - lunar main
    - debhelper-compat (debhelper)
    - libpam0g-dev (pam)
    - pkg-config
    - psmisc
    - puma (mir ack'd)
    - python3
    - python3-cryptography (python-cryptography)
    - python3-dacite (dacite, ack'd to main)
    - python3-dateutil (python-dateutil)
    - python3-lxml (lxml)
    - python3-pycurl (pycurl)
    - python3-pyparsing (pyparsing)
    - python3-tornado (python-tornado, ack'd to main)
    - ruby
    - ruby-childprocess (ack'd to main)
    - ruby-dev
    - ruby-ethon (ack'd to main)
    - ruby-sinatra (ack'd to main)
    - ruby-rubygems (rubygems)
    - ruby-webrick
    - systemd
    - wget
  - lunar universe
    - dh-python
    - python3-pip (python-pip)
    - python3-pyagentx (pyagentx)
    - python3-setuptools (python-setuptools)
    - python3-setuptools-scm (setuptools-scm)
    - ruby-json
    - ruby-rack-test
- pre/post inst/rm scripts?
  - sets permissions and links services
- init scripts?
  - from debian/pcs.*.init
    - /etc/init.d/pcsd
    - /etc/init.d/pcsd-ruby
    - /etc/init.d/pcs_snmp_agent
  - like systemd services, but more rudimentary
    - no env loading
- systemd units?
  - minimal systemd services to load env and manage processes
    - /lib/systemd/system/pcsd-ruby.service
    - /lib/systemd/system/pcsd.service
    - /lib/systemd/system/pcs_snmp_agent.service
- dbus services?
  - none
- setuid binaries?
  - none
- binaries in PATH?
  - /usr/sbin/pcs
  - /usr/sbin/pcsd
- etc files
  - /etc/default/pcsd
  - /etc/logrotate.d/pcsd
  - /etc/pam.d/pcsd
  - /etc/default/pcs_snmp_agent
- sudo fragments?
  - none
- polkit files?
  - none
- udev rules?
  - none
- unit tests / autopkgtests?
  - contains (many) build tests
    - 6230 total
    - they are even distinguished by tier
  - has autopkgtests
    - lunar failing amd64!
- cron jobs?
  - none
- Build logs:
  - E: pcs: depends-on-obsolete-package Depends: lsb-base (>= 3.0-6)

- Processes spawned?
  - heavy use
  - kill_services effectively runs killall -9
  - most handled by lib.external.CommandRunner which is used by lib.utils.cmd_runner()
    - lib.utils.run() is present and marked as deprecated in favor of CommandRunner
    - api inherits this power
    - extremely powerful if misused
- Memory management?
  - standard python and ruby
- File IO?
  - heavy use
  - mostly standard python and some ruby
- Logging?
  - lots of logging, reporting, and debug support
  - looks well thought out and safe
- Environment variable usage?
  - heavy use
    - especially in the context of executing processes
  - low sanitization for env or other inputs generally
  - systemd services load specific env file
- Use of privileged functions?
  - ability to change file permissions
  - seven if branches depend on uid/euid 0
    - this encourages running as root
    - running or forking pcs into a group with limited permissions would be ideal
- Use of cryptography / random number sources etc?
  - pam authentication
    - for cli and web api
  - pcsd/pam/pcsd.debian
    - @include common-auth
    - @include common-account
    - @include common-password
    - @include common-session
  - only hosts.py allows token
    - similar only allows PAM creds
  - passwords can be given with argument or keyboard input
  - acl_role, acl_permissions, etc, can be set
  - many checks for verifying hosts and peers
  - lib.utils.read_known_hosts_file_not_cached(): "TODO remove [...] Cli should never read known hosts from /var/lib/pcsd/."
    - !
  - curl wrapper supports SSL
  - openssl use by python and ruby
  - generates rsa 3072 tls key and cert with python's ssl and x509 modules
    - key and cert can be manually set
- Use of temp files?
  - many
- Use of networking?
  - heavy networking and socket use
  - provides an API
  - sendHTTPRequest allows communication with unknown hosts
- Use of WebKit?
  - none
- Use of PolicyKit?
  - none

- Any significant cppcheck results?
  - none
- Any significant Coverity results?
  - not significant
- Any significant shellcheck results?
  - none impact package
- Any significant bandit results?
  - issues in tests, ignored for MIR
  - XML vulnerabilities
    - https://docs.python.org/3.7/library/xml.html#xml-vulnerabilities
    - can be mitigated by replacing minidom parsers with their defusedxml equivalent
    - several results in deprecated functions
    - from in-line comments, upstream is shifting to lxml
      - note some, but drastically fewer, lxml functions are suseptible to above attack

Generally, this codebase is defensively written, clean, and well maintained. Their many tests speak quality.

Many functions are marked as deprecated, but remain in code.

Server Team is aware that upstream intends to rewrite pcsd in Python.

pcs gives it's users incredible power. Bypassing API authentication compromises all systems in a cluster. See impact in related software https://lists.schedmd.com/pipermail/slurm-announce/2022/000072.html

pcs 0.11.5 (2023-03-1) includes commit 3347a3f for "daemon: authentication refactoring". Upgrading pcs for 23.04 would be greatly appreciated for security and maintainability. >= 0.11.5 should be used in 23.10.

Lunar is affected by CVE-2022-2735.

Security team ACK for promoting pcs to main, after CVE-2022-2735 has been resolved (ideally by upgrading to 0.11.5).