custom pam #1772

samrose · 2025-08-25T19:31:58Z

What kind of change does this PR introduce?

passes in an arg to use go 1.24 since that is not the default go version we're using in nixpkgs in this flake
packages gatekeeper by using the https://flake.parts/ pattern of creating a module with configuration for the package at nix/packages/gatekeeper.nix and then uses the package in nix/packages/default.nix

I will follow up with an explanation as to why we use this flake-parts approach soon.

The fundamental reasons are discussed at https://flake.parts/index.html

steve-chavez · 2025-08-26T21:10:06Z

packages gatekeeper by using the https://flake.parts/ pattern of creating a module with configuration for the package at nix/packages/gatekeeper.nix and then uses the package in nix/packages/default.nix

@samrose What's the advantage of the above compared to callPackage? Overall looks like the flake way is much more verbose.

samrose · 2025-08-26T21:30:29Z

packages gatekeeper by using the https://flake.parts/ pattern of creating a module with configuration for the package at nix/packages/gatekeeper.nix and then uses the package in nix/packages/default.nix

@samrose What's the advantage of the above compared to callPackage? Overall looks like the flake way is much more verbose.

@steve-chavez for what it's worth, here are the advantages to using flakes, and the flake-parts framework https://gist.github.com/samrose/a9a19b7fad0aed8fd8355bdf0b87df08

ansible/tasks/setup-postgres.yml

staaldraad · 2025-08-29T15:03:23Z

this and #1765 are directly related. I don't feel like merging the two into one version though, as the change in 1765 is pretty significant already.

ansible/tasks/stage2-setup-postgres.yml

samrose · 2025-08-31T18:42:01Z

this and #1765 are directly related. I don't feel like merging the two into one version though, as the change in 1765 is pretty significant already.

@staaldraad I personally support whatever works best for you. I only created this PR to trigger some of the tests and builds and checks that let us know things are working as desired + give a simpler way to see changs. But thought you could either use this PR, or move work as you see fit.

samrose · 2025-09-10T18:00:08Z

@staaldraad I believe it is the case that you are not yet ready to merge this correct?

staaldraad · 2025-09-12T12:34:48Z

@staaldraad I believe it is the case that you are not yet ready to merge this correct?

I'm ready, I've not been available to Shepard it through. And wanted to wait on the PG upgrade PR. 👍🏼

samrose · 2025-09-12T13:18:45Z

@staaldraad I believe it is the case that you are not yet ready to merge this correct?

I'm ready, I've not been available to Shepard it through. And wanted to wait on the PG upgrade PR. 👍🏼

@staaldraad not trying to be an unreasonable blocker, but checking in as we're being extra careful lately:

Do you think this has been thoroughly tested against latest release in our local infra?

staaldraad · 2025-09-12T15:45:05Z

@staaldraad I believe it is the case that you are not yet ready to merge this correct?

I'm ready, I've not been available to Shepard it through. And wanted to wait on the PG upgrade PR. 👍🏼

@staaldraad not trying to be an unreasonable blocker, but checking in as we're being extra careful lately:

Do you think this has been thoroughly tested against latest release in our local infra?

Great call out @samrose fully understood and you aren't being a blocker. It has been tested and it default off, the trigger for enabling it hasn't been enabled yet in the API. We can wait for Monday, I'll give it an extra set of testing

samrose · 2025-09-12T18:10:40Z

@staaldraad is it true that this package should not be installed on a pg 15 image?

This reverts commit 70206f2.

This reverts commit 1dfd426.

Co-authored-by: Douglas J Hunley <[email protected]>

samrose · 2025-09-12T18:34:21Z

@staaldraad did a rebase on develop since we have launched pg 17.6 and 15.14 since the time you started this work.

staaldraad · 2025-09-22T15:25:37Z

@samrose who hits the merge button on this one? 😄

samrose · 2025-09-26T15:21:41Z

@staaldraad I'll ping a few folks to do one last review, and we can merge on Monday, deploy on Tue

jfroche · 2025-09-29T08:48:23Z

.github/workflows/nix-build.yml

+      - name: Setup SSH for deploy key
+        run: |
+          mkdir -p ~/.ssh
+          echo "${{ secrets.GK_DEPLOY_KEY }}" > ~/.ssh/id_ed25519


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 ?

jfroche · 2025-09-29T08:53:55Z

ansible/tasks/setup-postgres.yml

+  set_fact:
+    is_psql_15: "{{ psql_version in ['psql_15'] }}"
+
+- name: create placeholder pam config


Why do we need to create an empty file here ? Is there another process relying on the presence of this file ?

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

jfroche · 2025-09-29T09:19:38Z

nix/packages/gatekeeper.nix

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 ?
We could use it as an input to this flake and specify the version we rely on in the flake input parameters.

jfroche · 2025-09-29T09:24:50Z

nix/packages/gatekeeper.nix

+  src = pkgs.fetchFromGitHub {
+    owner = "supabase";
+    repo = "jit-db-gatekeeper";
+    rev = "refs/heads/main";


I would be explicit here about the exact ref (revision or tag) we use.
It would be cool if the tag of the version we use, would be available upstream to rely on it.

jfroche · 2025-09-29T09:42:25Z

nix/packages/gatekeeper.nix

+  buildGoModule = pkgs.buildGoModule.override { go = go124; };
+in
+
+buildGoModule {


As jit-db-gatekeeper contains unittest, it might be worth adding a check to run them:

checkPhase = '' runHook preCheck go test -v ./... runHook postCheck '';

jfroche · 2025-09-29T09:55:50Z

testinfra/test_ami_nix.py

+        try:
+            pg_major_version = int(result["stdout"].strip())
+        except ValueError:
+            pass


We want to know if we cannot parse the postgresql version. We should fail instead of using a default value.

jfroche · 2025-09-29T10:04:49Z

ansible/tasks/stage2-setup-postgres.yml

+    - 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


Are we trying to write in the store here ? Did you mean /usr/lib/x86_64-linux-gnu/security/pam_jit_pg.so where all the other pam modules are deployed in distribution like Ubuntu ?

Do we really need to copy that module if we can specify the full module path in the pam configuration (/etc/pam.d/postgresql) ?

auth required /var/lib/postgresql/.nix-profile/lib/security/pam_jwt_pg.so jwks=https://auth.supabase.green/auth/v1/.well-known/jwks.json mappings=/tmp/users.yaml account required /var/lib/postgresql/.nix-profile/lib/security/pam_jwt_pg.so jwks=https://auth.supabase.green/auth/v1/.well-known/jwks.json mappings=/tmp/users.yaml

did not try with the full path, nice!

Did you mean /usr/lib/x86_64-linux-gnu/security/pam_jit_pg.so

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

jfroche · 2025-09-29T10:10:20Z

testinfra/test_ami_nix.py

+    # 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",


Might be related to a previous comment but this will return the lib in the gatekeeper package like this: /nix/store/miwaglm79i2vpilmp8rxd2mymjs7nibz-gatekeeper-0.1.0/lib/security/pam_jit_pg.so. I guess we want to check that /usr/lib/x86_64-linux-gnu/security/pam_jit_pg.so exists instead ?

jfroche · 2025-09-29T10:14:54Z

testinfra/test_ami_nix.py

+    # 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",


I would rather do: nix profile list --json | jq -r '.elements.gatekeeper.storePaths[0]' to find the store path and use that path in the next check (running find on the nix-store can take a while).

jfroche · 2025-09-29T11:36:44Z

testinfra/test_ami_nix.py

+        print(f"\nJIT PAM module dependencies:\n{result['stdout']}")
+
+        # Check for required libraries
+        required_libs = ["libpam", "libc"]


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:

$ ldd /var/lib/postgresql/.nix-profile/lib/security/pam_jit_pg.so linux-vdso.so.1 (0x00007fffa3338000) libpam.so.0 => /nix/store/k491acz8rcfhrpivv0rg6q83a14zss2j-linux-pam-1.6.0/lib/libpam.so.0 (0x00007fe0c901c000) libpthread.so.0 => /nix/store/ddwyrxif62r8n6xclvskjyy6szdhvj60-glibc-2.39-5/lib/libpthread.so.0 (0x00007fe0c9017000) libresolv.so.2 => /nix/store/ddwyrxif62r8n6xclvskjyy6szdhvj60-glibc-2.39-5/lib/libresolv.so.2 (0x00007fe0c9006000) libc.so.6 => /nix/store/ddwyrxif62r8n6xclvskjyy6szdhvj60-glibc-2.39-5/lib/libc.so.6 (0x00007fe0c8613000) libaudit.so.1 => /nix/store/0i8hhak73jnlilfgfsh1wh0fd53pb0r9-audit-3.1.2/lib/libaudit.so.1 (0x00007fe0c8fd1000) $ patchelf --print-rpath /root/.nix-profile/lib/security/pam_jit_pg.so /nix/store/k491acz8rcfhrpivv0rg6q83a14zss2j-linux-pam-1.6.0/lib:/nix/store/ddwyrxif62r8n6xclvskjyy6szdhvj60-glibc-2.39-5/lib $ dpkg -l libpam-runtime Desired=Unknown/Install/Remove/Purge/Hold | Status=Not/Inst/Conf-files/Unpacked/halF-conf/Half-inst/trig-aWait/Trig-pend |/ Err?=(none)/Reinst-required (Status,Err: uppercase=bad) ||/ Name Version Architecture Description +++-==============-================-============-=================================== ii libpam-runtime 1.5.3-5ubuntu5.4 all Runtime support for the PAM library

We can instead remove the rpath and let the dynamic linker do its job (without looking at other nix libraries):

$ cp /var/lib/postgres/.nix-profile/lib/security/pam_jit_pg.so /usr/lib/x86_64-linux-gnu/security/ $ patchelf --remove-rpath /usr/lib/x86_64-linux-gnu/security/pam_jit_pg.so $ ldd /usr/lib/x86_64-linux-gnu/security/pam_jit_pg.so linux-vdso.so.1 (0x00007fff3179e000) libpam.so.0 => /lib/x86_64-linux-gnu/libpam.so.0 (0x000078aa645ea000) libpthread.so.0 => /lib/x86_64-linux-gnu/libpthread.so.0 (0x000078aa645e5000) libresolv.so.2 => /lib/x86_64-linux-gnu/libresolv.so.2 (0x000078aa645d2000) libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x000078aa63a00000) libaudit.so.1 => /lib/x86_64-linux-gnu/libaudit.so.1 (0x000078aa645a4000) /lib64/ld-linux-x86-64.so.2 (0x000078aa64602000) libcap-ng.so.0 => /lib/x86_64-linux-gnu/libcap-ng.so.0 (0x000078aa6459a000)

We could remove that rpath after the build of the package:

postFixup = '' patchelf --remove-rpath $out/lib/security/pam_jit_pg.so '';

But still there might be other surprises as the gatekeeper module has been build based on PAM 1.6.0.
Usually PAM has a good backward compatibility and the amount of dyn symbols used by gatekeeper is rather limited:

$ objdump -T /var/lib/postgres/.nix-profile/lib/security/pam_jit_pg.so | grep PAM 0000000000000000 DF *UND* 0000000000000000 (LIBPAM_1.0) pam_strerror 0000000000000000 DF *UND* 0000000000000000 (LIBPAM_EXTENSION_1.0) pam_syslog 0000000000000000 DF *UND* 0000000000000000 (LIBPAM_1.0) pam_get_item 0000000000000000 DF *UND* 0000000000000000 (LIBPAM_1.0) pam_get_user 0000000000000000 DF *UND* 0000000000000000 (LIBPAM_EXTENSION_1.1) pam_get_authtok

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 dlopen errors in the logs.

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 /nix/store/*-linux-pam-*/lib/security because the linker looks for the modules there instead of the default Ubuntu path)

$ patchelf --print-rpath /nix/store/2x79j9fi9j13hhf5n0yc9825qa4nynv3-postgresql-and-plugins-17.6/bin/.postgres-wrapped /nix/store/bvjhqzpwbzmfi350nir8gf7h45igvp5z-libxml2-2.12.6/lib:/nix/store/wcwqdhl9jpwgj1fdr0pw9dbaf5m43f17-lz4-1.9.4/lib:/nix/store/mxvqrp4jj6zklxc1i32ra8iapnzynyzc-zstd-1.5.5/lib:/nix/store/ram2bh4rs56rp8sr0gcdb108wcc609si-icu4c-73.2/lib:/nix/store/ymav8nb29vsdryddi435xcig3pk7i00q-zlib-1.3.1/lib:/nix/store/0kfbzhd8qdsxsd4qf9dkpcd3f916krvr-openssl-3.0.13/lib:/nix/store/ss6bfrdx69ng9f448d5hq1683hkpxcng-systemd-255.4/lib:/nix/store/vaz0prypndg3y6zxmdcpabinwg9dmmq9-libkrb5-1.21.2/lib:/nix/store/4rdpzzfssknp4k4cn59wc7xcdiyv9j53-linux-pam-1.6.0/lib:/nix/store/10pd749c2v3m35d4hk5xdfylyfr1hzqz-glibc-2.39-5/lib

-->

/nix/store/4rdpzzfssknp4k4cn59wc7xcdiyv9j53-linux-pam-1.6.0/lib

Tried modifying the rpath on postgres-wrapped but that leads to problems looking up libaudit.so.1 (even though it is in the new rpath and when specifying the path manually to the location in the store)

samrose requested review from a team as code owners August 25, 2025 19:31

soedirgo approved these changes Aug 26, 2025

View reviewed changes

staaldraad force-pushed the sam/custom-pam branch from 5a65e37 to 07162ca Compare August 26, 2025 14:06

hunleyd requested changes Aug 27, 2025

View reviewed changes

ansible/tasks/setup-postgres.yml Outdated Show resolved Hide resolved

samrose commented Aug 28, 2025

View reviewed changes

ansible/tasks/setup-postgres.yml Outdated Show resolved Hide resolved

staaldraad force-pushed the sam/custom-pam branch from 01e56d8 to 43363e2 Compare August 29, 2025 11:34

hunleyd requested changes Aug 29, 2025

View reviewed changes

ansible/tasks/stage2-setup-postgres.yml Outdated Show resolved Hide resolved

staaldraad force-pushed the sam/custom-pam branch from a796ea8 to fdd95c8 Compare August 30, 2025 05:47

samrose requested a review from hunleyd August 31, 2025 18:40

hunleyd approved these changes Sep 2, 2025

View reviewed changes

samrose changed the title ~~Sam/custom pam~~ custom pam Sep 10, 2025

staaldraad and others added 9 commits September 12, 2025 14:30

feat: add gatekeeper

2a344a3

test: custom flake

3e0d704

feat: use gatekeeper package from upstream private repo

7f2fd90

chore: add overlay

74a58b5

chore: add deploy_key for gk repo

4a3b66c

fix: fmt and go version, hopefully

7af1e1d

fix: fmt and go version, hopefully

731a984

fix: pass go override in overlays

485b2ba

fix: pass go override in overlays

94417b0

samrose and others added 10 commits September 12, 2025 14:30

feat: package gatekeeper in this package set

4424005

fix: update source of go 1.24 nixpkgs

23e0f72

Revert "fix: update source of go 1.24 nixpkgs"

99f56b3

This reverts commit 70206f2.

Revert "feat: package gatekeeper in this package set"

8335a33

This reverts commit 1dfd426.

fix: clean up nix flake and lock, drop overlay

383a8b2

chore: install gatekeeper with ansible

04a64f1

fix: smallest typo

fda4cab

Apply suggestion from @hunleyd

4b0b60b

Co-authored-by: Douglas J Hunley <[email protected]>

fix: syntax error

a118e56

test: some sanity tests for jit pam module

2f18016

samrose force-pushed the sam/custom-pam branch from f9ef9b7 to 2f18016 Compare September 12, 2025 18:33

samrose added 2 commits September 12, 2025 14:38

fix: get treefmt to pass

90f48c5

test: more fixes to test`

804a2dc

hunleyd requested a review from staaldraad September 16, 2025 13:10

staaldraad approved these changes Sep 18, 2025

View reviewed changes

jfroche reviewed Sep 29, 2025

View reviewed changes

Uh oh!

custom pam #1772

Are you sure you want to change the base?

custom pam #1772

Uh oh!

Conversation

samrose commented Aug 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What kind of change does this PR introduce?

Uh oh!

steve-chavez commented Aug 26, 2025

Uh oh!

samrose commented Aug 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

staaldraad commented Aug 29, 2025

Uh oh!

Uh oh!

samrose commented Aug 31, 2025

Uh oh!

samrose commented Sep 10, 2025

Uh oh!

staaldraad commented Sep 12, 2025

Uh oh!

samrose commented Sep 12, 2025

Uh oh!

staaldraad commented Sep 12, 2025

Uh oh!

samrose commented Sep 12, 2025

Uh oh!

samrose commented Sep 12, 2025

Uh oh!

staaldraad commented Sep 22, 2025

Uh oh!

samrose commented Sep 26, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

samrose commented Aug 25, 2025 •

edited

Loading

samrose commented Aug 26, 2025 •

edited

Loading