Skip to content
Snippets Groups Projects

WIP: pressure-vessel: Use a shorter random string for the server socket

Closed Ludovico de Nittis requested to merge wip/shorten_socket into main
2 unresolved threads

The AF_UNIX sockets that are used as rendezvous-point are limited to 108 bytes by the Linux kernel.

In order to be more flexible with the socket directory size, we avoid using the 36 bytes long UUID as the server socket. Instead we randomize a 12 bytes string that saves us 24 bytes, that we can spare for the socket directory, but it is still long enough to prevent a DOS attack.

Edited by Ludovico de Nittis

Merge request reports

Merge request pipeline #9820 passed

Merge request pipeline passed for 994d800b

Closed by Simon McVittieSimon McVittie 2 years ago (Feb 6, 2023 7:04pm UTC)

Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • mentioned in merge request steamlinuxruntime!34 (closed)

  • Ludovico de Nittis marked this merge request as draft

    marked this merge request as draft

  • build:devel is not happy with this change. Its first build went flawlessly but then I changed my commit message and now it is failing...

    I need to investigate what is going on.

    • Sorry, I'm not intending to land this or steamlinuxruntime!34 (closed) right now: we need a new beta before we land more stuff.

      Is there a concrete issue that this solves? I thought Steam now sets PRESSURE_VESSEL_SOCKET_DIR for us, to a path in /tmp that is certainly short enough?

      The route I'm more likely to take, as part of eventually absorbing the launcher/session functionality into the C code, is to make PRESSURE_VESSEL_SOCKET_DIR mandatory for "session mode", and stop trying to cope with it being attacker-controlled - effectively, it would become part of what we might call the "Steam compat protocol". That would mean we can use pressure-vessel-launcher --socket=... and don't need random strings at all.

    • This was useful when used in combination to the user provided XDG_RUNTIME_DIR, which we have no control over it.

      But if we permanently move away from it, I guess there is no need to try to shorten the random socket name.

    • Please register or sign in to reply
50 50 G_DEFINE_AUTOPTR_CLEANUP_FUNC (PvLauncher1Skeleton, g_object_unref)
51 51 #endif
52 52
53 /* Format: xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx */
54 #define PV_UUID_STRLEN 36
53 /* Use a 12 characters long random server socket name */
54 #define PV_SERVER_SOCKET_STRLEN 12
  • Is this really enough entropy to make a retry loop unnecessary, if we're operating in a directory that might not be fully under our control?

    Bear in mind that g_rand_int_range() is a fairly weak PRNG, so the usual security model is to assume that an attacker can predict it.

  • An alternative approach might be to stat the --socket-dir, refuse to use it unless it is owned by the current uid with permissions 0755 or stricter, and document --socket-dir accordingly. Then we could use a socket name that doesn't have to be so defensive.

  • Please register or sign in to reply
  • Simon McVittie changed target branch from master to main

    changed target branch from master to main

  • Merging steamlinuxruntime!53 (merged) made this relatively useless. I deprecated the --socket-directory option in a3ea4583 and one day I'll remove it completely.

  • Please register or sign in to reply
    Loading