-
-
Notifications
You must be signed in to change notification settings - Fork 208
custom pam #1772
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
custom pam #1772
Changes from all commits
2a344a3
3e0d704
7f2fd90
74a58b5
4a3b66c
7af1e1d
731a984
485b2ba
94417b0
4424005
23e0f72
99f56b3
8335a33
383a8b2
04a64f1
fda4cab
4b0b60b
a118e56
2f18016
90f48c5
804a2dc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -139,6 +139,19 @@ | |
group: postgres | ||
when: debpkg_mode or nixpkg_mode | ||
|
||
- name: Check if psql_version is psql_15 | ||
set_fact: | ||
is_psql_15: "{{ psql_version in ['psql_15'] }}" | ||
|
||
- name: create placeholder pam config | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why do we need to create an empty file here ? Is there another process relying on the presence of this file ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yes, supabase-admin-api expects this file to be read-writeable. it is also the default name Postgres will use to lookup PAM rules when pg_hba lists pam as the login service |
||
file: | ||
path: '/etc/pam.d/postgresql' | ||
state: touch | ||
owner: postgres | ||
group: postgres | ||
mode: 0664 | ||
when: (debpkg_mode or nixpkg_mode) and not is_psql_15 | ||
|
||
# Add pg_hba.conf | ||
- name: import pg_hba.conf | ||
template: | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -94,7 +94,26 @@ | |
shell: | | ||
sudo -u postgres bash -c ". /nix/var/nix/profiles/default/etc/profile.d/nix-daemon.sh && nix profile install github:supabase/postgres/{{ git_commit_sha }}#{{postgresql_version}}_src" | ||
when: stage2_nix | ||
|
||
|
||
- name: Check if psql_version is psql_15 | ||
set_fact: | ||
is_psql_15: "{{ psql_version == 'psql_15' }}" | ||
|
||
- name: Install gatekeeper if not pg15 | ||
when: | ||
- stage2_nix | ||
- not is_psql_15 | ||
block: | ||
- name: Install gatekeeper from nix binary cache | ||
become: yes | ||
shell: | | ||
sudo -u postgres bash -c ". /nix/var/nix/profiles/default/etc/profile.d/nix-daemon.sh && nix profile install github:supabase/postgres/{{ git_commit_sha }}#gatekeeper" | ||
- name: Create symbolic link for linux-pam to find pam_jit_pg.so | ||
become: yes | ||
shell: | | ||
sudo ln -s /var/lib/postgresql/.nix-profile/lib/security/pam_jit_pg.so $(find /nix/store -type d -path "/nix/store/*-linux-pam-*/lib/security" -print -quit)/pam_jit_pg.so | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are we trying to write in the store here ? Did you mean Do we really need to copy that module if we can specify the full module path in the pam configuration (
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. did not try with the full path, nice!
Yes original intention was to use that location, but during testing pam was failing to lookup the module and was looking in the store instead. Full paths would solve this |
||
- name: Set ownership and permissions for /etc/ssl/private | ||
become: yes | ||
file: | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we need to declare the package here if the same package is publicly available in https://github.com/supabase/jit-db-gatekeeper/blob/ac9c0ddef7a31f4143674a095e5bd96aee984685/default.nix ? |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,50 @@ | ||
{ | ||
inputs, | ||
system, | ||
pkgs, | ||
... | ||
}: | ||
let | ||
go124 = inputs.nixpkgs-go124.legacyPackages.${system}.go_1_24; | ||
# Use completely clean nixpkgs without any overlays for gatekeeper | ||
#cleanPkgs = inputs.nixpkgs.legacyPackages.${system}; | ||
buildGoModule = pkgs.buildGoModule.override { go = go124; }; | ||
in | ||
|
||
buildGoModule { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As jit-db-gatekeeper contains unittest, it might be worth adding a check to run them:
|
||
pname = "gatekeeper"; | ||
version = "0.1.0"; | ||
|
||
src = pkgs.fetchFromGitHub { | ||
owner = "supabase"; | ||
repo = "jit-db-gatekeeper"; | ||
rev = "refs/heads/main"; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would be explicit here about the exact ref (revision or tag) we use. |
||
hash = "sha256-hrYh1dBxk+aN3b/J9mZqk/ZXHmWA/MIqZLVgICT7e90="; | ||
}; | ||
|
||
vendorHash = "sha256-G9x2TARSJMn30R6ZOlsggxEtn5t2ezWz1YtkLXdYiAE="; | ||
|
||
buildInputs = [ | ||
pkgs.pam | ||
] ++ pkgs.lib.optionals pkgs.stdenv.isDarwin [ pkgs.darwin.apple_sdk.frameworks.Security ]; | ||
|
||
buildPhase = '' | ||
runHook preBuild | ||
go build -buildmode=c-shared -o pam_jit_pg.so | ||
runHook postBuild | ||
''; | ||
|
||
installPhase = '' | ||
runHook preInstall | ||
mkdir -p $out/lib/security | ||
cp pam_jit_pg.so $out/lib/security/ | ||
runHook postInstall | ||
''; | ||
|
||
meta = with pkgs.lib; { | ||
description = "PAM module for JWT authentication with PostgreSQL backend"; | ||
homepage = "https://github.com/supabase/jit-db-gatekeeper"; | ||
license = licenses.mit; | ||
platforms = platforms.unix; | ||
}; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -617,6 +617,227 @@ def test_libpq5_version(host): | |
print("✓ libpq5 version is >= 14") | ||
|
||
|
||
def test_jit_pam_module_installed(host): | ||
"""Test that the JIT PAM module (pam_jit_pg.so) is properly installed.""" | ||
# Check PostgreSQL version first | ||
result = run_ssh_command( | ||
host["ssh"], "sudo -u postgres psql --version | grep -oE '[0-9]+' | head -1" | ||
) | ||
pg_major_version = 15 # Default | ||
if result["succeeded"] and result["stdout"].strip(): | ||
try: | ||
pg_major_version = int(result["stdout"].strip()) | ||
except ValueError: | ||
pass | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We want to know if we cannot parse the postgresql version. We should fail instead of using a default value. |
||
|
||
# Skip test for PostgreSQL 15 as gatekeeper is not installed for PG15 | ||
if pg_major_version == 15: | ||
print("\nSkipping JIT PAM module test for PostgreSQL 15 (not installed)") | ||
return | ||
|
||
# Check if gatekeeper is installed via Nix | ||
result = run_ssh_command( | ||
host["ssh"], | ||
"sudo -u postgres ls -la /var/lib/postgresql/.nix-profile/lib/security/pam_jit_pg.so 2>/dev/null", | ||
) | ||
if result["succeeded"]: | ||
print(f"\nJIT PAM module found in Nix profile:\n{result['stdout']}") | ||
else: | ||
print("\nJIT PAM module not found in postgres user's Nix profile") | ||
assert False, "JIT PAM module (pam_jit_pg.so) not found in expected location" | ||
|
||
# Check if the symlink exists in the Linux PAM security directory | ||
result = run_ssh_command( | ||
host["ssh"], | ||
"find /nix/store -type f -path '*/lib/security/pam_jit_pg.so' 2>/dev/null | head -5", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Might be related to a previous comment but this will return the lib in the gatekeeper package like this: |
||
) | ||
if result["succeeded"] and result["stdout"].strip(): | ||
print(f"\nJIT PAM module symlinks found:\n{result['stdout']}") | ||
else: | ||
print("\nNo JIT PAM module symlinks found in /nix/store") | ||
|
||
# Verify the module is a valid shared library | ||
result = run_ssh_command( | ||
host["ssh"], "file /var/lib/postgresql/.nix-profile/lib/security/pam_jit_pg.so" | ||
) | ||
if result["succeeded"]: | ||
print(f"\nJIT PAM module file type:\n{result['stdout']}") | ||
assert ( | ||
"shared object" in result["stdout"].lower() | ||
or "dynamically linked" in result["stdout"].lower() | ||
), "JIT PAM module is not a valid shared library" | ||
|
||
print("✓ JIT PAM module is properly installed") | ||
|
||
|
||
def test_pam_postgresql_config(host): | ||
"""Test that the PAM configuration for PostgreSQL exists and is properly configured.""" | ||
# Check PostgreSQL version to determine if PAM config should exist | ||
result = run_ssh_command( | ||
host["ssh"], "sudo -u postgres psql --version | grep -oE '[0-9]+' | head -1" | ||
) | ||
pg_major_version = 15 # Default | ||
if result["succeeded"] and result["stdout"].strip(): | ||
try: | ||
pg_major_version = int(result["stdout"].strip()) | ||
except ValueError: | ||
pass | ||
|
||
print(f"\nPostgreSQL major version: {pg_major_version}") | ||
|
||
# PAM config should exist for non-PostgreSQL 15 versions | ||
if pg_major_version != 15: | ||
# Check if PAM config file exists | ||
result = run_ssh_command(host["ssh"], "ls -la /etc/pam.d/postgresql") | ||
if result["succeeded"]: | ||
print(f"\nPAM config file found:\n{result['stdout']}") | ||
|
||
# Check file permissions | ||
result = run_ssh_command( | ||
host["ssh"], "stat -c '%a %U %G' /etc/pam.d/postgresql" | ||
) | ||
if result["succeeded"]: | ||
perms = result["stdout"].strip() | ||
print(f"PAM config permissions: {perms}") | ||
# Should be owned by postgres:postgres with 664 permissions | ||
assert ( | ||
"postgres postgres" in perms | ||
), "PAM config not owned by postgres:postgres" | ||
else: | ||
print("\nPAM config file not found") | ||
assert False, "PAM configuration file /etc/pam.d/postgresql not found" | ||
else: | ||
print("\nSkipping PAM config check for PostgreSQL 15") | ||
# For PostgreSQL 15, the PAM config should NOT exist | ||
result = run_ssh_command(host["ssh"], "test -f /etc/pam.d/postgresql") | ||
if result["succeeded"]: | ||
print("\nWARNING: PAM config exists for PostgreSQL 15 (not expected)") | ||
|
||
print("✓ PAM configuration is properly set up") | ||
|
||
|
||
def test_jit_pam_gatekeeper_profile(host): | ||
"""Test that the gatekeeper package is properly installed in the postgres user's Nix profile.""" | ||
# Check PostgreSQL version first | ||
result = run_ssh_command( | ||
host["ssh"], "sudo -u postgres psql --version | grep -oE '[0-9]+' | head -1" | ||
) | ||
pg_major_version = 15 # Default | ||
if result["succeeded"] and result["stdout"].strip(): | ||
try: | ||
pg_major_version = int(result["stdout"].strip()) | ||
except ValueError: | ||
pass | ||
|
||
# Skip test for PostgreSQL 15 as gatekeeper is not installed for PG15 | ||
if pg_major_version == 15: | ||
print("\nSkipping gatekeeper profile test for PostgreSQL 15 (not installed)") | ||
return | ||
|
||
# Check if gatekeeper is in the postgres user's Nix profile | ||
result = run_ssh_command( | ||
host["ssh"], | ||
"sudo -u postgres nix profile list 2>/dev/null | grep -i gatekeeper", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would rather do: |
||
) | ||
if result["succeeded"] and result["stdout"].strip(): | ||
print(f"\nGatekeeper found in Nix profile:\n{result['stdout']}") | ||
else: | ||
# Try alternative check | ||
result = run_ssh_command( | ||
host["ssh"], | ||
"sudo -u postgres ls -la /var/lib/postgresql/.nix-profile/ | grep -i gate", | ||
) | ||
if result["succeeded"] and result["stdout"].strip(): | ||
print(f"\nGatekeeper-related files in profile:\n{result['stdout']}") | ||
else: | ||
print("\nGatekeeper not found in postgres user's Nix profile") | ||
# This might be expected if it's installed system-wide instead | ||
|
||
# Check if we can find the gatekeeper derivation | ||
result = run_ssh_command( | ||
host["ssh"], | ||
"find /nix/store -maxdepth 1 -type d -name '*gatekeeper*' 2>/dev/null | head -5", | ||
) | ||
if result["succeeded"] and result["stdout"].strip(): | ||
print(f"\nGatekeeper derivations found:\n{result['stdout']}") | ||
else: | ||
print("\nNo gatekeeper derivations found in /nix/store") | ||
|
||
print("✓ Gatekeeper package installation check completed") | ||
|
||
|
||
def test_jit_pam_module_dependencies(host): | ||
"""Test that the JIT PAM module has all required dependencies.""" | ||
# Check PostgreSQL version first | ||
result = run_ssh_command( | ||
host["ssh"], "sudo -u postgres psql --version | grep -oE '[0-9]+' | head -1" | ||
) | ||
pg_major_version = 15 # Default | ||
if result["succeeded"] and result["stdout"].strip(): | ||
try: | ||
pg_major_version = int(result["stdout"].strip()) | ||
except ValueError: | ||
pass | ||
|
||
# Skip test for PostgreSQL 15 as gatekeeper is not installed for PG15 | ||
if pg_major_version == 15: | ||
print( | ||
"\nSkipping JIT PAM module dependencies test for PostgreSQL 15 (not installed)" | ||
) | ||
return | ||
|
||
# Check dependencies of the PAM module | ||
result = run_ssh_command( | ||
host["ssh"], | ||
"ldd /var/lib/postgresql/.nix-profile/lib/security/pam_jit_pg.so 2>/dev/null", | ||
) | ||
if result["succeeded"]: | ||
print(f"\nJIT PAM module dependencies:\n{result['stdout']}") | ||
|
||
# Check for required libraries | ||
required_libs = ["libpam", "libc"] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Note that these dynamic dependencies/libraries are not coming from Ubuntu but from Nix which could cause problem as Nix and Ubuntu might not use the same PAM version. In the following test, we see that the module is loading PAM 1.6.0 but that version of Ubuntu is using PAM 1.5.3:
We can instead remove the rpath and let the dynamic linker do its job (without looking at other nix libraries):
We could remove that rpath after the build of the package:
But still there might be other surprises as the gatekeeper module has been build based on PAM 1.6.0.
If (and only if) we face issue (I would test if further first), I would build it against the same version using https://lazamar.co.uk/nix-versions/?package=linux-pam&version=1.5.3&fullName=linux-pam-1.5.3&keyName=linux-pam&revision=7a339d87931bba829f68e94621536cad9132971a&channel=nixpkgs-unstable#instructions We might want to create an automated test that exercice the module using pamtester just to verify that we don't get any There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we were/are building against 1.6.0 because of postgresql having been built against it (and hence also the hackery of copying the .so to
-->
Tried modifying the rpath on |
||
for lib in required_libs: | ||
if lib not in result["stdout"].lower(): | ||
print(f"WARNING: Required library {lib} not found in dependencies") | ||
|
||
# Check for any missing dependencies | ||
if "not found" in result["stdout"].lower(): | ||
assert False, "JIT PAM module has missing dependencies" | ||
else: | ||
print("\nCould not check JIT PAM module dependencies") | ||
|
||
print("✓ JIT PAM module dependencies are satisfied") | ||
|
||
|
||
def test_jit_pam_postgresql_integration(host): | ||
"""Test that PostgreSQL can be configured to use PAM authentication.""" | ||
# Check if PAM is available as an authentication method in PostgreSQL | ||
result = run_ssh_command( | ||
host["ssh"], | ||
"sudo -u postgres psql -c \"SELECT name, setting FROM pg_settings WHERE name LIKE '%pam%';\" 2>/dev/null", | ||
) | ||
if result["succeeded"]: | ||
print(f"\nPostgreSQL PAM-related settings:\n{result['stdout']}") | ||
|
||
# Check pg_hba.conf for potential PAM entries (even if not currently active) | ||
result = run_ssh_command( | ||
host["ssh"], | ||
"grep -i pam /etc/postgresql/pg_hba.conf 2>/dev/null || echo 'No PAM entries in pg_hba.conf'", | ||
) | ||
if result["succeeded"]: | ||
print(f"\nPAM entries in pg_hba.conf:\n{result['stdout']}") | ||
|
||
# Verify PostgreSQL was compiled with PAM support | ||
result = run_ssh_command( | ||
host["ssh"], | ||
"sudo -u postgres pg_config --configure 2>/dev/null | grep -i pam || echo 'PAM compile flag not found'", | ||
) | ||
if result["succeeded"]: | ||
print(f"\nPostgreSQL PAM compile flags:\n{result['stdout']}") | ||
|
||
print("✓ PostgreSQL PAM integration check completed") | ||
|
||
|
||
def test_postgrest_read_only_session_attrs(host): | ||
"""Test PostgREST with target_session_attrs=read-only and check for session errors.""" | ||
# First, check if PostgreSQL is configured for read-only mode | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I might miss context here, it is not clear to me why this private key is required and how it is related to this PR. It might be a leftover from that commit 7f2fd90 ?