client-conf: Don't create multiple cookie files

The old code loaded cookies at the time of loading the client
configuration, which could lead to creation of multiple cookie files.
For example, when pa_client_conf_load() was called, the default cookie
file was created, and then if PULSE_COOKIE was set,
pa_client_conf_env() would create another cookie file.

This patch moves the loading of the cookie to a separate function,
which pa_context calls just before needing the cookie, so the cookie
won't be loaded from the default file if PULSE_COOKIE is set. This
patch also splits the single cookie and cookie_file fields in
pa_client_conf into multiple fields, one for each possible cookie
source. That change allows falling back to another cookie source if
the primary source doesn't work.

BugLink: https://bugs.freedesktop.org/show_bug.cgi?id=75006

src/pulse/client-conf-x11.c

@@ -92,10 +92,12 @@ int pa_client_conf_from_x11(pa_client_conf *c) {
     }
 
     if (pa_x11_get_prop(xcb, screen, "PULSE_COOKIE", t, sizeof(t))) {
-        if (pa_client_conf_load_cookie_from_hex(c, t) < 0) {
+        if (pa_parsehex(t, c->cookie_from_x11, sizeof(c->cookie_from_x11)) != sizeof(c->cookie_from_x11)) {
             pa_log(_("Failed to parse cookie data"));
             goto finish;
         }
+
+        c->cookie_from_x11_valid = true;
     }
 
     ret = 0;

src/pulse/client-conf.c

@@ -57,23 +57,22 @@ static const pa_client_conf default_conf = {
     .default_source = NULL,
     .default_server = NULL,
     .default_dbus_server = NULL,
+    .cookie_file_from_env = NULL,
+    .cookie_from_x11_valid = false,
+    .cookie_file_from_application = NULL,
+    .cookie_file_from_client_conf = NULL,
     .autospawn = true,
     .disable_shm = false,
-    .cookie_file = NULL,
-    .cookie_valid = false,
     .shm_size = 0,
     .auto_connect_localhost = false,
     .auto_connect_display = false
 };
 
-static int parse_cookie_file(pa_client_conf* c);
-
 pa_client_conf *pa_client_conf_new(void) {
     pa_client_conf *c = pa_xmemdup(&default_conf, sizeof(default_conf));
 
     c->daemon_binary = pa_xstrdup(PA_BINARY);
     c->extra_arguments = pa_xstrdup("--log-target=syslog");
-    c->cookie_file = NULL;
 
     return c;
 }
@@ -86,7 +85,9 @@ void pa_client_conf_free(pa_client_conf *c) {
     pa_xfree(c->default_source);
     pa_xfree(c->default_server);
     pa_xfree(c->default_dbus_server);
-    pa_xfree(c->cookie_file);
+    pa_xfree(c->cookie_file_from_env);
+    pa_xfree(c->cookie_file_from_application);
+    pa_xfree(c->cookie_file_from_client_conf);
     pa_xfree(c);
 }
 
@@ -104,7 +105,7 @@ int pa_client_conf_load(pa_client_conf *c) {
         { "default-server",         pa_config_parse_string,   &c->default_server, NULL },
         { "default-dbus-server",    pa_config_parse_string,   &c->default_dbus_server, NULL },
         { "autospawn",              pa_config_parse_bool,     &c->autospawn, NULL },
-        { "cookie-file",            pa_config_parse_string,   &c->cookie_file, NULL },
+        { "cookie-file",            pa_config_parse_string,   &c->cookie_file_from_client_conf, NULL },
         { "disable-shm",            pa_config_parse_bool,     &c->disable_shm, NULL },
         { "enable-shm",             pa_config_parse_not_bool, &c->disable_shm, NULL },
         { "shm-size-bytes",         pa_config_parse_size,     &c->shm_size, NULL },
@@ -119,9 +120,6 @@ int pa_client_conf_load(pa_client_conf *c) {
 
     r = f ? pa_config_parse(fn, f, table, NULL, NULL) : 0;
 
-    if (!r)
-        r = parse_cookie_file(c);
-
 finish:
     pa_xfree(fn);
 
@@ -131,6 +129,67 @@ finish:
     return r;
 }
 
+int pa_client_conf_load_cookie(pa_client_conf *c, uint8_t *cookie, size_t cookie_length) {
+    int r;
+
+    pa_assert(c);
+    pa_assert(cookie);
+    pa_assert(cookie_length > 0);
+
+    if (c->cookie_file_from_env) {
+        r = pa_authkey_load_auto(c->cookie_file_from_env, true, cookie, cookie_length);
+        if (r >= 0)
+            return 0;
+
+        pa_log_warn("Failed to load cookie from %s (configured with environment variable PULSE_COOKIE): %s",
+                    c->cookie_file_from_env, pa_cstrerror(errno));
+    }
+
+    if (c->cookie_from_x11_valid) {
+        if (cookie_length == sizeof(c->cookie_from_x11)) {
+            memcpy(cookie, c->cookie_from_x11, cookie_length);
+            return 0;
+        }
+
+        pa_log_warn("Failed to load cookie from X11 root window property PULSE_COOKIE: size mismatch.");
+    }
+
+    if (c->cookie_file_from_application) {
+        r = pa_authkey_load_auto(c->cookie_file_from_application, true, cookie, cookie_length);
+        if (r >= 0)
+            return 0;
+
+        pa_log_warn("Failed to load cookie from %s (configured by the application): %s", c->cookie_file_from_application,
+                    pa_cstrerror(errno));
+    }
+
+    if (c->cookie_file_from_client_conf) {
+        r = pa_authkey_load_auto(c->cookie_file_from_client_conf, true, cookie, cookie_length);
+        if (r >= 0)
+            return 0;
+
+        pa_log_warn("Failed to load cookie from %s (configured in client.conf): %s", c->cookie_file_from_client_conf,
+                    pa_cstrerror(errno));
+    }
+
+    r = pa_authkey_load_auto(PA_NATIVE_COOKIE_FILE, false, cookie, cookie_length);
+    if (r >= 0)
+        return 0;
+
+    r = pa_authkey_load_auto(PA_NATIVE_COOKIE_FILE_FALLBACK, false, cookie, cookie_length);
+    if (r >= 0)
+        return 0;
+
+    r = pa_authkey_load_auto(PA_NATIVE_COOKIE_FILE, true, cookie, cookie_length);
+    if (r >= 0)
+        return 0;
+
+    pa_log("Failed to load cookie file from %s: %s", PA_NATIVE_COOKIE_FILE, pa_cstrerror(errno));
+    memset(cookie, 0, cookie_length);
+
+    return -1;
+}
+
 int pa_client_conf_env(pa_client_conf *c) {
     char *e;
 
@@ -157,73 +216,18 @@ int pa_client_conf_env(pa_client_conf *c) {
         c->daemon_binary = pa_xstrdup(e);
     }
 
-    if ((e = getenv(ENV_COOKIE_FILE))) {
+    if ((e = getenv(ENV_COOKIE_FILE)) && *e) {
-        return pa_client_conf_load_cookie_from_file(c, e);
+        pa_xfree(c->cookie_file_from_env);
+        c->cookie_file_from_env = pa_xstrdup(e);
     }
 
     return 0;
 }
 
-static int parse_cookie_file(pa_client_conf* c) {
+void pa_client_conf_set_cookie_file_from_application(pa_client_conf *c, const char *cookie_file) {
-    int k;
-
     pa_assert(c);
+    pa_assert(!cookie_file || *cookie_file);
 
-    c->cookie_valid = false;
+    pa_xfree(c->cookie_file_from_application);
-
+    c->cookie_file_from_application = pa_xstrdup(cookie_file);
-    if (c->cookie_file)
-        k = pa_authkey_load_auto(c->cookie_file, true, c->cookie, sizeof(c->cookie));
-    else {
-        k = pa_authkey_load_auto(PA_NATIVE_COOKIE_FILE, false, c->cookie, sizeof(c->cookie));
-
-        if (k < 0) {
-            k = pa_authkey_load_auto(PA_NATIVE_COOKIE_FILE_FALLBACK, false, c->cookie, sizeof(c->cookie));
-
-            if (k < 0)
-                k = pa_authkey_load_auto(PA_NATIVE_COOKIE_FILE, true, c->cookie, sizeof(c->cookie));
-        }
-    }
-
-    if (k < 0)
-        return k;
-
-    c->cookie_valid = true;
-    return 0;
-}
-
-int pa_client_conf_load_cookie_from_hex(pa_client_conf* c, const char *cookie_in_hex) {
-    uint8_t cookie[PA_NATIVE_COOKIE_LENGTH];
-
-    pa_assert(c);
-    pa_assert(cookie_in_hex);
-
-    if (pa_parsehex(cookie_in_hex, cookie, sizeof(cookie)) != sizeof(cookie)) {
-        pa_log(_("Failed to parse cookie data"));
-        return -PA_ERR_INVALID;
-    }
-
-    pa_xfree(c->cookie_file);
-    c->cookie_file = NULL;
-
-    return pa_client_conf_set_cookie(c, cookie, PA_NATIVE_COOKIE_LENGTH);
-}
-
-int pa_client_conf_load_cookie_from_file(pa_client_conf *c, const char *cookie_file_path) {
-    pa_assert(c);
-    pa_assert(cookie_file_path);
-
-    pa_xfree(c->cookie_file);
-    c->cookie_file = pa_xstrdup(cookie_file_path);
-    return parse_cookie_file(c);
-}
-
-int pa_client_conf_set_cookie(pa_client_conf *c, uint8_t *cookie, size_t cookie_size) {
-    pa_assert(c);
-    pa_assert(cookie);
-
-    if (cookie_size != PA_NATIVE_COOKIE_LENGTH)
-        return -PA_ERR_INVALID;
-    memcpy(c->cookie, cookie, cookie_size);
-    c->cookie_valid = true;
-    return 0;
 }

src/pulse/client-conf.h

@@ -28,10 +28,18 @@
 /* A structure containing configuration data for PulseAudio clients. */
 
 typedef struct pa_client_conf {
-    char *daemon_binary, *extra_arguments, *default_sink, *default_source, *default_server, *default_dbus_server, *cookie_file;
+    char *daemon_binary;
+    char *extra_arguments;
+    char *default_sink;
+    char *default_source;
+    char *default_server;
+    char *default_dbus_server;
+    char *cookie_file_from_env;
+    uint8_t cookie_from_x11[PA_NATIVE_COOKIE_LENGTH];
+    bool cookie_from_x11_valid;
+    char *cookie_file_from_application;
+    char *cookie_file_from_client_conf;
     bool autospawn, disable_shm, auto_connect_localhost, auto_connect_display;
-    uint8_t cookie[PA_NATIVE_COOKIE_LENGTH];
-    bool cookie_valid; /* non-zero, when cookie is valid */
     size_t shm_size;
 } pa_client_conf;
 
@@ -43,17 +51,16 @@ void pa_client_conf_free(pa_client_conf *c);
  * the current settings in *c. */
 int pa_client_conf_load(pa_client_conf *c);
 
+/* Load the cookie from the cookie sources specified in the configuration, or
+ * if nothing is specified or none of the sources work, load the cookie from
+ * the default source. If the default source doesn't work either, this function
+ * returns a negative value and initializes the cookie to all-zeroes. */
+int pa_client_conf_load_cookie(pa_client_conf *c, uint8_t *cookie, size_t cookie_length);
+
 /* Load the configuration data from the environment of the current
    process, overwriting the current settings in *c. */
 int pa_client_conf_env(pa_client_conf *c);
 
-/* Load cookie data from cookie_file_path into c->cookie */
+void pa_client_conf_set_cookie_file_from_application(pa_client_conf *c, const char *cookie_file);
-int pa_client_conf_load_cookie_from_file(pa_client_conf *c, const char *cookie_file_path);
-
-/* Load cookie data from hexdecimal string into c->cookie */
-int pa_client_conf_load_cookie_from_hex(pa_client_conf *c, const char *cookie_in_hex);
-
-/* Set cookie direct from memory */
-int pa_client_conf_set_cookie(pa_client_conf *c, uint8_t *cookie, size_t cookie_size);
 
 #endif

src/pulse/context.c

@@ -506,6 +506,7 @@ finish:
 }
 
 static void setup_context(pa_context *c, pa_iochannel *io) {
+    uint8_t cookie[PA_NATIVE_COOKIE_LENGTH];
     pa_tagstruct *t;
     uint32_t tag;
 
@@ -524,7 +525,7 @@ static void setup_context(pa_context *c, pa_iochannel *io) {
     pa_assert(!c->pdispatch);
     c->pdispatch = pa_pdispatch_new(c->mainloop, c->use_rtclock, command_table, PA_COMMAND_MAX);
 
-    if (!c->conf->cookie_valid)
+    if (pa_client_conf_load_cookie(c->conf, cookie, sizeof(cookie)) < 0)
         pa_log_info(_("No cookie loaded. Attempting to connect without."));
 
     t = pa_tagstruct_command(c, PA_COMMAND_AUTH, &tag);
@@ -538,7 +539,7 @@ static void setup_context(pa_context *c, pa_iochannel *io) {
     /* Starting with protocol version 13 we use the MSB of the version
      * tag for informing the other side if we could do SHM or not */
     pa_tagstruct_putu32(t, PA_PROTOCOL_VERSION | (c->do_shm ? 0x80000000U : 0));
-    pa_tagstruct_put_arbitrary(t, c->conf->cookie, sizeof(c->conf->cookie));
+    pa_tagstruct_put_arbitrary(t, cookie, sizeof(cookie));
 
 #ifdef HAVE_CREDS
 {
@@ -1451,11 +1452,13 @@ size_t pa_context_get_tile_size(pa_context *c, const pa_sample_spec *ss) {
 
 int pa_context_load_cookie_from_file(pa_context *c, const char *cookie_file_path) {
     pa_assert(c);
-    pa_assert(cookie_file_path);
     pa_assert(PA_REFCNT_VALUE(c) >= 1);
 
     PA_CHECK_VALIDITY(c, !pa_detect_fork(), PA_ERR_FORKED);
     PA_CHECK_VALIDITY(c, c->state == PA_CONTEXT_UNCONNECTED, PA_ERR_BADSTATE);
+    PA_CHECK_VALIDITY(c, !cookie_file_path || *cookie_file_path, PA_ERR_INVALID);
 
-    return pa_client_conf_load_cookie_from_file(c->conf, cookie_file_path);
+    pa_client_conf_set_cookie_file_from_application(c->conf, cookie_file_path);
+
+    return 0;
 }