Localhost port discovery - #37
Conversation
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: richardsonnick The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
Bot
added
the
approved
|
Just to note:
Might need a follow up PR if I'm seeing correctly that we may get PQC failures that are false positives. |
…addresses from /proc/net/tcp
ParseProcNetTCP currently discards the local address field, returning only port
numbers. Introduce ParseProcNetTCPWithAddrs which returns a map[int]string
(port -> decoded listen address) and decodeProcNetAddr whicch converts the
littleendian hex address fields used by /proc/net/tcp and /proc/net/tcp6 into
standard IP strings ("127.0.0.1", "::1" etc.).
Refactor ParseProcNetTCP to be a thin wrapper over ParseProcNetTCPWithAddrs so
existing callers are unaffected.
/proc/net/tcp reflects the pod's shared network namespace and is therefore visible from any container in the pod, including Containers[0]. lsof, by contrast, is limited to the PID namespace of the container it runs in, so ports owned by secondary containers are invisible to it. This caches the decoded listen address on `Client` via `procListenAddrMap`. `IsLocalHost` now falls back to this cache wehn lso has no entry for a port. This will allow us to correctly ID for secondary-container localhost ports as `LOCALHOST_ONLY`. `LOCALHOST_ONLY` ports do not have the same expected conformance behaviors as externally accessible ports. The IPv6 loopback address (::1) is also recognised as localhost, fixing a gap in the previous isLocalhostAddr check.
Liveness, readiness, and startup probes do not use TLS. This is okay from a TLS conformance POV. Currently scans will indicate `NO_TLS` on probes from a secondary container. This results in a number of components with spurious NO_TLS status results. Add GetPlaintextProbePorts which reads the structured probe fields from the pod spec (LivenessProbe, ReadinessProbe, StartupProbe) and returns the set of port numbers whose probes are plaintext. Named ports (e.g. "healthz") are resolved against the container's declared Ports list. Ports probed via HTTPS are excluded so they continue to be scanned normally. In the scanner discovery loop, ports identified as plaintext probe endpoints are recorded with the new PROBE_PORT status and skipped by testssl.sh, the same way LOCALHOST_ONLY ports are handled.
ff8ffdd to
d683941
Compare
I believe I fixed this in this commit . @smith-xyz |
|
Thanks for this @richardsonnick Do we want to fix #31 in this PR or in a separate one? Thanks! |
…oc data when available. Introduce a flag to indicate successful /proc port discovery. Showing that only active listener ports are included in the scan results.
|
/lgtm |
|
Fixes #31 |
3 participants
Fix false-positive NO_TLS results for localhost and health-probe ports
Closes #33, closes #34
The scanner was producing a number of spurious NO_TLS results from two (independent) root causes:
Multi-container localhost ports (#33)
lsof is executed only in Containers[0]. Because containers share a network namespace but have separate PID namespaces, lsof cannot see sockets owned by secondary containers. Those ports appeared in /proc/net/tcp (shared), had no listenInfoMap entry, and so IsLocalhostOnly returned false — sending them to testssl.sh where they were reported as NO_TLS.
Health probe ports (#34)
Liveness, readiness, and startup probes that use plaintext HTTP, TCPSocket, or gRPC listen on ports that are intentionally not TLS-protected. The scanner had no awareness of these probe endpoints and scanned them unconditionally, producing false positives for the vast majority of OpenShift components.
Solution
Fix 1 — /proc/net/tcp-based localhost detection (Option B from #33)
/proc/net/tcp is a view of the pod's shared network namespace and therefore already shows every socket across all containers — including those in secondary containers invisible to lsof. Previously only port numbers were extracted from it; the local address column was discarded.
ParseProcNetTCPWithAddrs now returns map[int]string (port -> decoded listen address). decodeProcNetAddr converts the little-endian hex fields used by /proc/net/tcp and /proc/net/tcp6 into standard IP strings ("127.0.0.1", "::1", "0.0.0.0", etc.).
DiscoverPortsFromProc caches these decoded addresses into a new procListenAddrMap on Client.
IsLocalhostOnly falls back to procListenAddrMap when lsof has no entry for a port. This is the case that will tell us if the endpoint is a secondary container listening on localhost. When lsof does have an entry it remains authoritative.
Fix 2 — Plaintext health probe port detection (#34)
The Kubernetes pod spec has structured fields for probes (livenessProbe, readinessProbe, startupProbe) with an explicit scheme field on HTTP probes (HTTP or HTTPS). This gives unambiguous, API-sourced signal about whether TLS is expected on a port!
GetPlaintextProbePorts reads all three probe fields across all containers and returns the set of ports whose probes are plaintext (HTTP, TCPSocket, or gRPC). Named port references (e.g. "healthz") are resolved against each container's Ports list.
Ports probed via HTTPS are explicitly excluded and continue to be TLS-scanned.
In the scanner discovery loop, plaintext probe ports are recorded with a new PROBE_PORT status and skipped by testssl.sh, the same way LOCALHOST_ONLY ports are handled.
New Status Values
PROBE_PORT: Indicates that this endpoint belongs to a probe. These are handled like LOCALHOST_ONLY endpoints and are skipped by testssl.sh scan.