[raop] Change device verification so we don't risk stale sesssions

Before, if a user never verified the device, we would have a device->session
even though the device was not streaming and was in a failed state.

This solution should be more clean and in line with the overall principle that
we only have a session when communicating with the device.

Also includes a bit of code refactoring.
This commit is contained in:
ejurgensen 2020-05-27 22:23:00 +02:00
parent 138d482510
commit 384b1171d9

View File

@ -1,5 +1,5 @@
/* /*
* Copyright (C) 2012-2019 Espen Jürgensen <espenjurgensen@gmail.com> * Copyright (C) 2012-2020 Espen Jürgensen <espenjurgensen@gmail.com>
* Copyright (C) 2010-2011 Julien BLACHE <jb@jblache.org> * Copyright (C) 2010-2011 Julien BLACHE <jb@jblache.org>
* *
* RAOP AirTunes v2 * RAOP AirTunes v2
@ -152,10 +152,8 @@ enum raop_state {
RAOP_STATE_TEARDOWN = RAOP_STATE_F_CONNECTED | 0x03, RAOP_STATE_TEARDOWN = RAOP_STATE_F_CONNECTED | 0x03,
// Session is failed, couldn't startup or error occurred // Session is failed, couldn't startup or error occurred
RAOP_STATE_FAILED = RAOP_STATE_F_FAILED | 0x01, RAOP_STATE_FAILED = RAOP_STATE_F_FAILED | 0x01,
// Password issue: unknown password or bad password // Password issue: unknown password or bad password, or pending PIN from user
RAOP_STATE_PASSWORD = RAOP_STATE_F_FAILED | 0x02, RAOP_STATE_PASSWORD = RAOP_STATE_F_FAILED | 0x02,
// Device requires verification, pending PIN from user
RAOP_STATE_UNVERIFIED= RAOP_STATE_F_FAILED | 0x03,
}; };
// Info about the device, which is not required by the player, only internally // Info about the device, which is not required by the player, only internally
@ -1726,7 +1724,7 @@ raop_status(struct raop_session *rs)
switch (rs->state) switch (rs->state)
{ {
case RAOP_STATE_PASSWORD ... RAOP_STATE_UNVERIFIED: case RAOP_STATE_PASSWORD:
state = OUTPUT_STATE_PASSWORD; state = OUTPUT_STATE_PASSWORD;
break; break;
case RAOP_STATE_FAILED: case RAOP_STATE_FAILED:
@ -1850,28 +1848,26 @@ session_free(struct raop_session *rs)
if (!rs) if (!rs)
return; return;
master_session_cleanup(rs->master_session); if (rs->master_session)
master_session_cleanup(rs->master_session);
evrtsp_connection_set_closecb(rs->ctrl, NULL, NULL); if (rs->ctrl)
{
evrtsp_connection_set_closecb(rs->ctrl, NULL, NULL);
evrtsp_connection_free(rs->ctrl);
}
evrtsp_connection_free(rs->ctrl); if (rs->deferredev)
event_free(rs->deferredev);
event_free(rs->deferredev); if (rs->server_fd >= 0)
close(rs->server_fd);
close(rs->server_fd); free(rs->realm);
free(rs->nonce);
if (rs->realm) free(rs->session);
free(rs->realm); free(rs->address);
if (rs->nonce) free(rs->devname);
free(rs->nonce);
if (rs->session)
free(rs->session);
if (rs->address)
free(rs->address);
if (rs->devname)
free(rs->devname);
free(rs); free(rs);
} }
@ -1986,44 +1982,106 @@ deferredev_cb(int fd, short what, void *arg)
} }
} }
static struct raop_session * static int
session_make(struct output_device *rd, int family, int callback_id, bool only_probe) session_connection_setup(struct raop_session *rs, struct output_device *rd, int family)
{ {
struct raop_session *rs;
struct raop_extra *re;
char *address; char *address;
char *intf; char *intf;
unsigned short port; unsigned short port;
int ret; int ret;
re = rd->extra_device_info; rs->sa.ss.ss_family = family;
switch (family) switch (family)
{ {
case AF_INET: case AF_INET:
/* We always have the v4 services, so no need to check */ /* We always have the v4 services, so no need to check */
if (!rd->v4_address) if (!rd->v4_address)
return NULL; return -1;
address = rd->v4_address; address = rd->v4_address;
port = rd->v4_port; port = rd->v4_port;
rs->timing_svc = &timing_4svc;
rs->control_svc = &control_4svc;
ret = inet_pton(AF_INET, address, &rs->sa.sin.sin_addr);
break; break;
case AF_INET6: case AF_INET6:
if (!rd->v6_address || rd->v6_disabled || (timing_6svc.fd < 0) || (control_6svc.fd < 0)) if (!rd->v6_address || rd->v6_disabled || (timing_6svc.fd < 0) || (control_6svc.fd < 0))
return NULL; return -1;
address = rd->v6_address; address = rd->v6_address;
port = rd->v6_port; port = rd->v6_port;
rs->timing_svc = &timing_6svc;
rs->control_svc = &control_6svc;
intf = strchr(address, '%');
if (intf)
*intf = '\0';
ret = inet_pton(AF_INET6, address, &rs->sa.sin6.sin6_addr);
if (intf)
{
*intf = '%';
intf++;
rs->sa.sin6.sin6_scope_id = if_nametoindex(intf);
if (rs->sa.sin6.sin6_scope_id == 0)
{
DPRINTF(E_LOG, L_RAOP, "Could not find interface %s\n", intf);
ret = -1;
break;
}
}
break; break;
default: default:
return NULL; return -1;
} }
if (ret <= 0)
{
DPRINTF(E_LOG, L_RAOP, "Device '%s' has invalid address (%s) for %s\n", rd->name, address, (family == AF_INET) ? "ipv4" : "ipv6");
return -1;
}
rs->ctrl = evrtsp_connection_new(address, port);
if (!rs->ctrl)
{
DPRINTF(E_LOG, L_RAOP, "Could not create control connection to '%s' (%s)\n", rd->name, address);
return -1;
}
evrtsp_connection_set_base(rs->ctrl, evbase_player);
rs->address = strdup(address);
rs->family = family;
return 0;
}
static struct raop_session *
session_make(struct output_device *rd, int callback_id, bool only_probe)
{
struct raop_session *rs;
struct raop_extra *re;
int ret;
re = rd->extra_device_info;
CHECK_NULL(L_RAOP, rs = calloc(1, sizeof(struct raop_session))); CHECK_NULL(L_RAOP, rs = calloc(1, sizeof(struct raop_session)));
CHECK_NULL(L_RAOP, rs->deferredev = evtimer_new(evbase_player, deferredev_cb, rs)); CHECK_NULL(L_RAOP, rs->deferredev = evtimer_new(evbase_player, deferredev_cb, rs));
rs->devname = strdup(rd->name);
rs->volume = rd->volume;
rs->state = RAOP_STATE_STOPPED; rs->state = RAOP_STATE_STOPPED;
rs->only_probe = only_probe; rs->only_probe = only_probe;
rs->reqs_in_flight = 0; rs->reqs_in_flight = 0;
@ -2072,77 +2130,19 @@ session_make(struct output_device *rd, int family, int callback_id, bool only_pr
break; break;
} }
rs->ctrl = evrtsp_connection_new(address, port); ret = session_connection_setup(rs, rd, AF_INET6);
if (!rs->ctrl) if (ret < 0)
{ {
DPRINTF(E_LOG, L_RAOP, "Could not create control connection to '%s' (%s)\n", rd->name, address); ret = session_connection_setup(rs, rd, AF_INET);
if (ret < 0)
goto out_free_event; goto error;
} }
evrtsp_connection_set_base(rs->ctrl, evbase_player);
rs->sa.ss.ss_family = family;
switch (family)
{
case AF_INET:
rs->timing_svc = &timing_4svc;
rs->control_svc = &control_4svc;
ret = inet_pton(AF_INET, address, &rs->sa.sin.sin_addr);
break;
case AF_INET6:
rs->timing_svc = &timing_6svc;
rs->control_svc = &control_6svc;
intf = strchr(address, '%');
if (intf)
*intf = '\0';
ret = inet_pton(AF_INET6, address, &rs->sa.sin6.sin6_addr);
if (intf)
{
*intf = '%';
intf++;
rs->sa.sin6.sin6_scope_id = if_nametoindex(intf);
if (rs->sa.sin6.sin6_scope_id == 0)
{
DPRINTF(E_LOG, L_RAOP, "Could not find interface %s\n", intf);
ret = -1;
break;
}
}
break;
default:
ret = -1;
break;
}
if (ret <= 0)
{
DPRINTF(E_LOG, L_RAOP, "Device '%s' has invalid address (%s) for %s\n", rd->name, address, (family == AF_INET) ? "ipv4" : "ipv6");
goto out_free_evcon;
}
rs->devname = strdup(rd->name);
rs->address = strdup(address);
rs->family = family;
rs->volume = rd->volume;
rs->master_session = master_session_make(&rd->quality, rs->encrypt); rs->master_session = master_session_make(&rd->quality, rs->encrypt);
if (!rs->master_session) if (!rs->master_session)
{ {
DPRINTF(E_LOG, L_RAOP, "Could not attach a master session for device '%s'\n", rd->name); DPRINTF(E_LOG, L_RAOP, "Could not attach a master session for device '%s'\n", rd->name);
goto out_free_evcon; goto error;
} }
// Attach to list of sessions // Attach to list of sessions
@ -2154,11 +2154,8 @@ session_make(struct output_device *rd, int family, int callback_id, bool only_pr
return rs; return rs;
out_free_evcon: error:
evrtsp_connection_free(rs->ctrl); session_free(rs);
out_free_event:
event_free(rs->deferredev);
free(rs);
return NULL; return NULL;
} }
@ -3553,13 +3550,7 @@ raop_cb_pin_start(struct evrtsp_request *req, void *arg)
if (ret < 0) if (ret < 0)
goto error; goto error;
rs->state = RAOP_STATE_UNVERIFIED; rs->state = RAOP_STATE_PASSWORD;
raop_status(rs);
// TODO If the user never verifies the session will remain stale
return;
error: error:
session_failure(rs); session_failure(rs);
@ -4202,8 +4193,8 @@ raop_cb_verification_verify_step2(struct evrtsp_request *req, void *arg)
return; return;
error: error:
rs->state = RAOP_STATE_UNVERIFIED; rs->state = RAOP_STATE_PASSWORD;
raop_status(rs); session_failure(rs);
} }
static void static void
@ -4233,11 +4224,11 @@ raop_cb_verification_verify_step1(struct evrtsp_request *req, void *arg)
return; return;
error: error:
rs->state = RAOP_STATE_UNVERIFIED;
raop_status(rs);
verification_verify_free(rs->verification_verify_ctx); verification_verify_free(rs->verification_verify_ctx);
rs->verification_verify_ctx = NULL; rs->verification_verify_ctx = NULL;
rs->state = RAOP_STATE_PASSWORD;
session_failure(rs);
} }
static int static int
@ -4259,9 +4250,6 @@ raop_verification_verify(struct raop_session *rs)
return 0; return 0;
error: error:
rs->state = RAOP_STATE_UNVERIFIED;
raop_status(rs);
verification_verify_free(rs->verification_verify_ctx); verification_verify_free(rs->verification_verify_ctx);
rs->verification_verify_ctx = NULL; rs->verification_verify_ctx = NULL;
return -1; return -1;
@ -4299,7 +4287,7 @@ raop_cb_verification_setup_step3(struct evrtsp_request *req, void *arg)
// A blocking db call... :-~ // A blocking db call... :-~
db_speaker_save(device); db_speaker_save(device);
// No longer UNVERIFIED // No longer RAOP_STATE_PASSWORD
rs->state = RAOP_STATE_STOPPED; rs->state = RAOP_STATE_STOPPED;
out: out:
@ -4333,7 +4321,7 @@ raop_cb_verification_setup_step2(struct evrtsp_request *req, void *arg)
error: error:
verification_setup_free(rs->verification_setup_ctx); verification_setup_free(rs->verification_setup_ctx);
rs->verification_setup_ctx = NULL; rs->verification_setup_ctx = NULL;
raop_status(rs); session_failure(rs);
} }
static void static void
@ -4355,7 +4343,7 @@ raop_cb_verification_setup_step1(struct evrtsp_request *req, void *arg)
error: error:
verification_setup_free(rs->verification_setup_ctx); verification_setup_free(rs->verification_setup_ctx);
rs->verification_setup_ctx = NULL; rs->verification_setup_ctx = NULL;
raop_status(rs); session_failure(rs);
} }
static int static int
@ -4374,6 +4362,8 @@ raop_verification_setup(struct raop_session *rs, const char *pin)
if (ret < 0) if (ret < 0)
goto error; goto error;
rs->state = RAOP_STATE_PASSWORD;
return 0; return 0;
error: error:
@ -4385,17 +4375,21 @@ raop_verification_setup(struct raop_session *rs, const char *pin)
static int static int
raop_device_authorize(struct output_device *device, const char *pin, int callback_id) raop_device_authorize(struct output_device *device, const char *pin, int callback_id)
{ {
struct raop_session *rs = device->session; struct raop_session *rs;
int ret; int ret;
// Make a session so we can communicate with the device
rs = session_make(device, callback_id, true);
if (!rs) if (!rs)
return -1; return -1;
ret = raop_verification_setup(rs, pin); ret = raop_verification_setup(rs, pin);
if (ret < 0) if (ret < 0)
return -1; {
DPRINTF(E_LOG, L_RAOP, "Could not send verification setup request to '%s' (address %s)\n", device->name, rs->address);
rs->callback_id = callback_id; session_cleanup(rs);
return -1;
}
return 1; return 1;
} }
@ -4706,26 +4700,7 @@ raop_device_start_generic(struct output_device *device, int callback_id, bool on
* address and build our session URL for all subsequent requests. * address and build our session URL for all subsequent requests.
*/ */
rs = session_make(device, AF_INET6, callback_id, only_probe); rs = session_make(device, callback_id, only_probe);
if (rs)
{
if (device->auth_key)
ret = raop_verification_verify(rs);
else if (device->requires_auth)
ret = raop_send_req_pin_start(rs, raop_cb_pin_start, "device_start");
else
ret = raop_send_req_options(rs, raop_cb_startup_options, "device_start");
if (ret == 0)
return 1;
else
{
DPRINTF(E_WARN, L_RAOP, "Could not send verification or OPTIONS request on IPv6\n");
session_cleanup(rs);
}
}
rs = session_make(device, AF_INET, callback_id, only_probe);
if (!rs) if (!rs)
return -1; return -1;
@ -4738,7 +4713,7 @@ raop_device_start_generic(struct output_device *device, int callback_id, bool on
if (ret < 0) if (ret < 0)
{ {
DPRINTF(E_WARN, L_RAOP, "Could not send verification or OPTIONS request on IPv4\n"); DPRINTF(E_WARN, L_RAOP, "Could not send verification or OPTIONS request to '%s' (address %s)\n", device->name, rs->address);
session_cleanup(rs); session_cleanup(rs);
return -1; return -1;
} }