diff --git a/msal/oauth2cli/authcode.py b/msal/oauth2cli/authcode.py index 4c60fb4d..0a5f0689 100644 --- a/msal/oauth2cli/authcode.py +++ b/msal/oauth2cli/authcode.py @@ -41,17 +41,28 @@ def obtain_auth_code(listen_port, auth_uri=None): # Historically only used in t def _is_inside_docker(): + # Marker files created by the container runtimes themselves are the most + # reliable signal. "/.dockerenv" is created by Docker (including Docker + # Desktop on Mac); "/run/.containerenv" is created by Podman. + if os.path.exists("/.dockerenv") or os.path.exists("/run/.containerenv"): + return True try: with open("/proc/1/cgroup") as f: # https://stackoverflow.com/a/20012536/728675 # Search keyword "/proc/pid/cgroup" in this link for the file format # https://man7.org/linux/man-pages/man7/cgroups.7.html for line in f.readlines(): cgroup_path = line.split(":", 2)[2].strip() - if cgroup_path.strip() != "/": + # Only a recognized container cgroup counts as "inside a container". + # We must NOT treat any non-"/" path as a container: on a cgroups v2 + # host, systemd puts PID 1 in "/init.scope" (rather than "/"), which + # is not a container and would otherwise cause this server to bind to + # 0.0.0.0 (all interfaces) instead of loopback. See issue #886. + if any(marker in cgroup_path for marker in ( + "docker", "containerd", "kubepods", "lxc", "libpod")): return True except IOError: pass # We are probably not running on Linux - return os.path.exists("/.dockerenv") # Docker on Mac will run this line + return False def is_wsl(): diff --git a/tests/test_authcode.py b/tests/test_authcode.py index 8d7c9d03..178cf285 100644 --- a/tests/test_authcode.py +++ b/tests/test_authcode.py @@ -2,9 +2,14 @@ import socket import sys +try: # Python 3 + from unittest.mock import patch, mock_open +except ImportError: # Python 2 + from mock import patch, mock_open + import requests -from msal.oauth2cli.authcode import AuthCodeReceiver +from msal.oauth2cli.authcode import AuthCodeReceiver, _is_inside_docker class TestAuthCodeReceiver(unittest.TestCase): @@ -104,3 +109,52 @@ def test_post_request_state_mismatch(self): )] result = receiver.get_auth_response(timeout=3, state="expected_state") self.assertIsNone(result, "Should not receive auth response due to state mismatch") + + +class TestIsInsideDocker(unittest.TestCase): + """Guard the container-detection heuristic used to decide the bind address. + + A false positive makes the auth-code receiver bind to 0.0.0.0 (all + interfaces) instead of loopback, which is a security concern per + RFC 8252 Section 8.3. See issue #886. + """ + + def _run(self, cgroup_content, existing_files=()): + existing = set(existing_files) + if cgroup_content is None: + open_mock = mock_open() + open_mock.side_effect = IOError("no such file") # e.g. Windows/Mac + else: + open_mock = mock_open(read_data=cgroup_content) + with patch("msal.oauth2cli.authcode.open", open_mock, create=True), \ + patch( + "msal.oauth2cli.authcode.os.path.exists", + side_effect=lambda p: p in existing): + return _is_inside_docker() + + def test_systemd_cgroups_v2_host_is_not_docker(self): + # Modern systemd host on cgroups v2: PID 1 lives in /init.scope, not "/". + self.assertFalse(self._run("0::/init.scope\n")) + + def test_cgroups_v1_host_is_not_docker(self): + content = "12:pids:/\n11:hugetlb:/\n0::/\n" + self.assertFalse(self._run(content)) + + def test_dockerenv_marker_is_docker(self): + self.assertTrue(self._run("0::/\n", existing_files=("/.dockerenv",))) + + def test_containerenv_marker_is_container(self): + self.assertTrue( + self._run("0::/\n", existing_files=("/run/.containerenv",))) + + def test_docker_cgroup_path_is_docker(self): + content = "12:pids:/docker/abc123\n0::/docker/abc123\n" + self.assertTrue(self._run(content)) + + def test_kubepods_cgroup_path_is_container(self): + content = "0::/kubepods/besteffort/pod123/abc\n" + self.assertTrue(self._run(content)) + + def test_non_linux_without_marker_is_not_docker(self): + # No /proc/1/cgroup (open raises IOError) and no marker files. + self.assertFalse(self._run(None))