feat(tcp): restore socket with UID#40
Conversation
Kimchi Code Review
Summary📊 Review Score: 90/100 (overall code quality — 0 lowest, 100 highest) 🧪 Tests: yes — A new zdtm test 📝 Found 1 issue(s). See inline comments for details. What to expectKimchi will analyze the changes in this pull request and post:
The review typically completes within a few minutes. This comment will be updated once the review is ready. Interact with Kimchi
ConfigurationReviews are configured by your organization admin. Powered by Kimchi — AI-powered code review by CAST AI |
There was a problem hiding this comment.
📊 Review Score: 90/100 (overall code quality — 0 lowest, 100 highest)
⏱️ Estimated effort to review: 2/5 (1 = trivial, 5 = very complex)
🧪 Tests: yes — A new zdtm test socket-tcp-uid was added to the static test Makefile, indicating coverage for the uid preservation logic. The actual test source was not shown in the diff, but the build integration confirms a targeted test exists.
📝 Found 1 issue(s). See inline comments for details.
| if (run_setsockcreatecon(fle->fe)) | ||
| return -1; | ||
|
|
||
| /* |
There was a problem hiding this comment.
if possible lets isolate this as a function as in other cases so its easier to merge form upstream when we need it
Istio has a bunch of iptables rules that match connections that have 'uid' at 1337 (istio internal uid). CRIU does not restore uid atm, just adding this to dump side, proto image and using the data on restore.
Note: setfsuid is process-wide call. But I believe CRIU is single-threaded at this stage (well, Claude says that), so it should not be a problem.
Note 2: the test seem to really work. Consider #41 (where the restore side is dropped). The test has failed -
FAIL: socket-tcp-uid.c:167: post-restore: sk_uid is 0, want 1337 (inode 685980) (errno = 11 (Resource temporarily unavailable)).istio iptables rules sample
Kimchi Summary
What changed
Preserve the kernel socket owner (
sk_uid) across checkpoint/restore for INET sockets. During dump CRIU captures the uid frominet_diag; during restore it temporarily switchesfsuidviasetfsuid()aroundsocket(2)so the kernel stamps the correct owner on the new socket.Why
The kernel initializes
sk_uidfromcurrent_fsuid()at socket creation time and exposes no interface to change it later. Without this, restored sockets are owned by the restore-time fsuid (typically root), breaking netfilter rules such asxt_owner --uid-ownerthat depend on the original socket owner.Key changes
images/sk-inet.proto: added optionaluidfield (field 21) toInetSkEntryto persist the dumped fsuid.criu/include/sk-inet.h: addeduidmember tostruct inet_sk_desc.criu/sk-inet.c:idiag_uidininet_collect_one()and fromst_uidfor unconstrained sockets ingen_uncon_sk().ie->uidandie->has_uidindo_dump_one_inet_fd().open_inet_sk(), callsetfsuid(ie->uid)beforesocket()and revert immediately after; warn if the fsuid could not be set.test/zdtm/static/socket-tcp-uid.c(new): ZDTM regression test that creates a socket under a non-root fsuid, verifies the expectedsk_uidvia/proc/net/tcpbefore and after C/R, and checks connection integrity.test/zdtm/static/Makefile&socket-tcp-uid.desc: wired the new test into the build with flavorh nsand flagssuid nouser samens.Impact
xt_owner --uid-ownernow remain valid after restore for checkpointed TCP connections.setfsuid()will silently fail and a warning is logged so the mismatch is visible.Kimchi Summary
What changed
Preserves per-socket ownership (
sk_uid) for INET sockets across checkpoint/restore by saving the dump-time UID in the CRIU image and temporarily switching the process filesystem UID during socket re-creation.Why
The kernel stamps
sk_uidfromcurrent_fsuid()at socket creation and provides no interface to change it later. Downstream tooling such asxt_owner(--uid-owner) netfilter rules depends on this value, so restored sockets must be recreated under their original UIDs to avoid policy mismatches.Key changes
images/sk-inet.proto: Added optionaluidfield (id 21) toinet_sk_entry.criu/include/sk-inet.h: Addeduidmember tostruct inet_sk_desc.criu/sk-inet.c:gen_uncon_sk()fromp->stat.st_uidand ininet_collect_one()fromm->idiag_uid.ie.uidand setsie.has_uidindo_dump_one_inet_fd().open_inet_sk(), callssetfsuid(ie->uid)beforesocket(2)and restores the old fsuid immediately afterward so the kernel stamps the correctsk_uid.test/zdtm/static/: Addedsocket-tcp-uidZDTM test and descriptor (socket-tcp-uid.c,socket-tcp-uid.desc); registered inMakefile. The test creates a TCP socket under a non-root UID and verifies via/proc/net/tcpthat the UID is preserved before and after C/R.Impact
sk_uid, keeping firewall and network-ownership policies consistent.setfsuid()will silently fail in that case and a warning is logged.