mirror of
https://github.com/pgadmin-org/pgadmin4.git
synced 2026-10-29 15:50:05 -05:00
fix: write session files with mode 0o600 (was umask-default 0644)
$DATA_DIR/sessions/<sid> contains OAuth access_tokens / refresh_tokens (via session['oauth2_token']), AWS / Google / Azure / BigAnimal cloud credentials (via the cloud refactors earlier in this branch), the Kerberos credential cache path, MFA OTP material, and pass_enc_key — the symmetric KEK that decrypts the user's saved Postgres server passwords. The HMAC header added earlier in this branch protects integrity but not confidentiality: anyone with read access to the file gets the secrets. Default `open(path, 'wb')` uses the process umask, which on typical systems leaves files 0o644 (world-readable). Switch new_session() and put() to a new _open_session_file() helper that opens with `os.open(... O_WRONLY | O_CREAT | O_TRUNC, 0o600)`, mirroring the upload helper introduced in PR 1. The directory itself is already 0o700, so this is defense-in-depth for container scenarios where the data volume might be mounted under shared uids, or for misconfigurations of the directory mode. NB: Existing session files retain their old mode until next write, then adopt 0o600. Operators who want to forcibly tighten existing files can chmod the sessions directory recursively post-upgrade. Adds a positive test asserting both put() and new_session() produce 0o600 files (skipped on Windows where POSIX mode bits are not meaningful).
This commit is contained in:
1 parent
ccfbd2c457
commit
bee80fe943
2 files changed
+65
-3
No files matched your search
@@ -68,6 +68,26 @@ def _compute_file_hmac(secret, body):
|
||||
return hmac.new(key, body, _FILE_HMAC).hexdigest().encode()
|
||||
|
||||
|
||||
def _open_session_file(path):
|
||||
"""Open a session file for writing with mode 0o600 (owner-only).
|
||||
|
||||
Session files contain OAuth access/refresh tokens, AWS/Google/Azure/
|
||||
BigAnimal cloud credentials, the Kerberos cache path, MFA OTP material,
|
||||
and `pass_enc_key` (the symmetric KEK used to decrypt the user's saved
|
||||
Postgres server passwords). The default `open(path, 'wb')` uses the
|
||||
process umask, which on most systems leaves files world-readable
|
||||
(0o644). Force 0o600 so that even if the directory permissions are
|
||||
misconfigured (e.g., a volume mount in a container with shared uids),
|
||||
individual session files remain owner-only.
|
||||
"""
|
||||
fd = os.open(
|
||||
path,
|
||||
os.O_WRONLY | os.O_CREAT | os.O_TRUNC,
|
||||
0o600,
|
||||
)
|
||||
return os.fdopen(fd, 'wb')
|
||||
|
||||
|
||||
sess_lock = Lock()
|
||||
LAST_CHECK_SESSION_FILES = None
|
||||
|
||||
@@ -254,8 +274,8 @@ class FileBackedSessionManager(SessionManager):
|
||||
if fname is None:
|
||||
raise InternalServerError('Failed to create new session')
|
||||
|
||||
# touch the file
|
||||
with open(fname, 'wb'):
|
||||
# touch the file with mode 0o600 — see _open_session_file rationale.
|
||||
with _open_session_file(fname):
|
||||
return ManagedSession(sid=sid)
|
||||
|
||||
return ManagedSession(sid=sid)
|
||||
@@ -343,7 +363,7 @@ class FileBackedSessionManager(SessionManager):
|
||||
(session.randval, session.hmac_digest, dict(session)), -1
|
||||
)
|
||||
header = _compute_file_hmac(self.secret, body)
|
||||
with open(fname, 'wb') as f:
|
||||
with _open_session_file(fname) as f:
|
||||
f.write(header)
|
||||
f.write(body)
|
||||
|
||||
|
||||
@@ -452,6 +452,48 @@ class TestRealisticSessionShapeRoundTrip(_SessionTestSetupMixin, BaseTestGenerat
|
||||
'/var/lib/pgadmin/storage/alice/')
|
||||
|
||||
|
||||
class TestSessionFileMode0o600(_SessionTestSetupMixin, BaseTestGenerator):
|
||||
"""Session files must be created with mode 0o600 (owner-only).
|
||||
|
||||
Session contents include OAuth access/refresh tokens, cloud
|
||||
credentials (AWS/Google/Azure/BigAnimal), the Kerberos cache path,
|
||||
MFA OTP material, and `pass_enc_key` (which decrypts the user's
|
||||
saved Postgres server passwords). The default open(path, 'wb') uses
|
||||
the process umask, which on most systems leaves files 0o644
|
||||
(world-readable). Verify both put() and new_session() create files
|
||||
with mode 0o600.
|
||||
"""
|
||||
|
||||
scenarios = [('default', dict())]
|
||||
|
||||
def runTest(self):
|
||||
if os.name == 'nt':
|
||||
self.skipTest("POSIX mode bits not meaningful on Windows")
|
||||
|
||||
# put() path
|
||||
sess = self.manager.new_session()
|
||||
sess.sid = "60606060-6060-6060-6060-606060606060"
|
||||
sess['k'] = 'v'
|
||||
sess.sign(SECRET)
|
||||
self.manager.put(sess)
|
||||
put_path = os.path.join(self.tmpdir, sess.sid)
|
||||
put_mode = os.stat(put_path).st_mode & 0o777
|
||||
self.assertEqual(
|
||||
put_mode, 0o600,
|
||||
"put() must create session file mode 0o600, got 0o%o" % put_mode)
|
||||
|
||||
# new_session() touch path. Force a worktree where new_session
|
||||
# actually writes the placeholder (default skip_paths is empty).
|
||||
new_sess = self.manager.new_session()
|
||||
new_path = os.path.join(self.tmpdir, new_sess.sid)
|
||||
if os.path.exists(new_path):
|
||||
new_mode = os.stat(new_path).st_mode & 0o777
|
||||
self.assertEqual(
|
||||
new_mode, 0o600,
|
||||
"new_session() touch must create file mode 0o600, "
|
||||
"got 0o%o" % new_mode)
|
||||
|
||||
|
||||
class TestServerModeFalseDirectUpload(_SessionTestSetupMixin, BaseTestGenerator):
|
||||
"""Spec T10: Scenario A chain closed at the session-read layer.
|
||||
|
||||
|
||||
Reference in new issue
Block a user