1
0
mirror of git://sourceware.org/git/lvm2.git synced 2025-01-17 06:04:23 +03:00

dmeventd: Extend checks on client socket.

Reinstate and extend checks removed by e1b111b02accb4145b82b8b47ce57ed93b1a7184.

The code has always assumed that only root has access to the directory
containing the fifos and that they are under the complete control of
dmeventd code.  If anything is found not to be as expected, then open()
should certainly not be attempted!
This commit is contained in:
Alasdair G Kergon 2015-12-08 00:54:32 +00:00
parent 00bab9d9cd
commit 94dab390ef
2 changed files with 34 additions and 2 deletions

View File

@ -1,5 +1,6 @@
Version 1.02.114 - Version 1.02.114 -
==================================== ====================================
Extend validity checks on dmeventd client socket.
Version 1.02.113 - 5th December 2015 Version 1.02.113 - 5th December 2015
==================================== ====================================

View File

@ -412,12 +412,41 @@ static int _start_daemon(char *dmeventd_path, struct dm_event_fifos *fifos)
char default_dmeventd_path[] = DMEVENTD_PATH; char default_dmeventd_path[] = DMEVENTD_PATH;
char *args[] = { dmeventd_path ? : default_dmeventd_path, NULL }; char *args[] = { dmeventd_path ? : default_dmeventd_path, NULL };
/*
* FIXME Explicitly verify the code's requirement that client_path is secure:
* - All parent directories owned by root without group/other write access unless sticky.
*/
/* If client fifo path exists, only use it if it is root-owned fifo mode 0600 */
if ((lstat(fifos->client_path, &statbuf) < 0)) {
if (errno == ENOENT)
/* Jump ahead if fifo does not already exist. */
goto start_server;
else {
log_sys_error("stat", fifos->client_path);
return 0;
}
} else if (!S_ISFIFO(statbuf.st_mode)) {
log_error("%s must be a fifo.", fifos->client_path);
return 0;
} else if (statbuf.st_uid) {
log_error("%s must be owned by uid 0.", fifos->client_path);
return 0;
} else if (statbuf.st_mode & (S_IEXEC | S_IRWXG | S_IRWXO)) {
log_error("%s must have mode 0600.", fifos->client_path);
return 0;
}
/* Anyone listening? If not, errno will be ENXIO */ /* Anyone listening? If not, errno will be ENXIO */
fifos->client = open(fifos->client_path, O_WRONLY | O_NONBLOCK); fifos->client = open(fifos->client_path, O_WRONLY | O_NONBLOCK);
if (fifos->client >= 0) { if (fifos->client >= 0) {
/* Should never happen if all the above checks passed. */
if ((fstat(fifos->client, &statbuf) < 0) || if ((fstat(fifos->client, &statbuf) < 0) ||
!S_ISFIFO(statbuf.st_mode)) { !S_ISFIFO(statbuf.st_mode) || statbuf.st_uid ||
log_error("%s is not a fifo.", fifos->client_path); (statbuf.st_mode & (S_IEXEC | S_IRWXG | S_IRWXO))) {
log_error("%s is no longer a secure root-owned fifo with mode 0600.", fifos->client_path);
if (close(fifos->client))
log_sys_debug("close", fifos->client_path);
return 0; return 0;
} }
@ -431,6 +460,7 @@ static int _start_daemon(char *dmeventd_path, struct dm_event_fifos *fifos)
return 0; return 0;
} }
start_server:
/* server is not running */ /* server is not running */
if ((args[0][0] == '/') && stat(args[0], &statbuf)) { if ((args[0][0] == '/') && stat(args[0], &statbuf)) {
@ -724,6 +754,7 @@ int dm_event_get_registered_device(struct dm_event_handler *dmevh, int next)
uuid = dm_task_get_uuid(dmt); uuid = dm_task_get_uuid(dmt);
/* FIXME Distinguish errors connecting to daemon */
if (_do_event(next ? DM_EVENT_CMD_GET_NEXT_REGISTERED_DEVICE : if (_do_event(next ? DM_EVENT_CMD_GET_NEXT_REGISTERED_DEVICE :
DM_EVENT_CMD_GET_REGISTERED_DEVICE, dmevh->dmeventd_path, DM_EVENT_CMD_GET_REGISTERED_DEVICE, dmevh->dmeventd_path,
&msg, dmevh->dso, uuid, dmevh->mask, 0)) { &msg, dmevh->dso, uuid, dmevh->mask, 0)) {