Skip to content

Commit

Permalink
Restrict local configuration to read-only, add new option for read-wr…
Browse files Browse the repository at this point in the history
…ite.

Thanks to Dave Taht and Julien Cristau for pointing out the issue.
  • Loading branch information
jech committed Feb 11, 2016
1 parent 5387cf0 commit 3208c75
Show file tree
Hide file tree
Showing 6 changed files with 85 additions and 42 deletions.
17 changes: 12 additions & 5 deletions babeld.c
Expand Up @@ -173,7 +173,7 @@ main(int argc, char **argv)

while(1) {
opt = getopt(argc, argv,
"m:p:h:H:i:k:A:sruS:d:g:lwz:M:t:T:c:C:DL:I:V");
"m:p:h:H:i:k:A:sruS:d:g:G:lwz:M:t:T:c:C:DL:I:V");
if(opt < 0)
break;

Expand Down Expand Up @@ -239,6 +239,13 @@ main(int argc, char **argv)
break;
case 'g':
local_server_port = parse_nat(optarg);
local_server_write = 0;
if(local_server_port <= 0 || local_server_port > 0xFFFF)
goto usage;
break;
case 'G':
local_server_port = parse_nat(optarg);
local_server_write = 1;
if(local_server_port <= 0 || local_server_port > 0xFFFF)
goto usage;
break;
Expand Down Expand Up @@ -289,7 +296,7 @@ main(int argc, char **argv)
break;
case 'C':
rc = parse_config_from_string(optarg, strlen(optarg));
if(rc != CONFIG_DONE) {
if(rc != CONFIG_ACTION_DONE) {
fprintf(stderr,
"Couldn't parse configuration from command line.\n");
exit(1);
Expand Down Expand Up @@ -842,13 +849,13 @@ main(int argc, char **argv)
" "
"[-h hello] [-H wired_hello] [-z kind[,factor]]\n"
" "
"[-k metric] [-A metric] [-s] [-l] [-w] [-r] [-u] [-g port]\n"
"[-g port] [-G port] [-k metric] [-A metric] [-s] [-l] [-w] [-r]\n"
" "
"[-t table] [-T table] [-c file] [-C statement]\n"
"[-u] [-t table] [-T table] [-c file] [-C statement]\n"
" "
"[-d level] [-D] [-L logfile] [-I pidfile]\n"
" "
"[id] interface...\n",
"interface...\n",
BABELD_VERSION);
exit(1);

Expand Down
1 change: 1 addition & 0 deletions babeld.h
Expand Up @@ -99,6 +99,7 @@ extern int have_id;
extern const unsigned char zeroes[16], ones[16];

extern int protocol_port, local_server_port;
extern int local_server_write;
extern unsigned char protocol_group[16];
extern int protocol_socket;
extern int kernel_socket;
Expand Down
32 changes: 27 additions & 5 deletions babeld.man
Expand Up @@ -110,12 +110,19 @@ requests tracing every message sent or received. A value of
is 0.
.TP
.BI \-g " port"
Listen for connections from a front-end on port
.IR port .
The protocol is described in the section
Set up a local configuration server on port
.I port
in read-only mode. The protocol is described in the section
.B Local Configuration Protocol
below.
.TP
.BI \-G " port"
Set up a local configuration server on port
.I port
in read-write mode. This allows any local user to change
.BR babeld 's
configuration, and may therefore be a security issue.
.TP
.BI \-t " table"
Use the given kernel routing table for routes inserted by
.BR babeld .
Expand Down Expand Up @@ -207,10 +214,19 @@ option
.BI local-port " port"
This specifies the TCP port on which
.B babeld
will listen for connections from a front-end, and is equivalent to the
command-line option
will listen for connections from a configuration client in read-only mode,
and is equivalent to the command-line option
.BR \-g .
.TP
.BI local-port-readwrite " port"
This specifies the TCP port on which
.B babeld
will listen for connections from a configuration client in read-write mode,
and is equivalent to the command-line option
.BR \-G . This allows any local user to change
.BR babeld 's
configuration, and may therefore be a security issue.
.TP
.BI export-table " table"
This specifies the kernel routing table to use for routes inserted by
.BR babeld ,
Expand Down Expand Up @@ -636,6 +652,12 @@ Babel is a completely insecure protocol: any attacker able to inject
IP packets with a link-local source address can disrupt the protocol's
operation. This is no different from unsecured neighbour discovery or ARP.

Usage of the
.B \-G
flag allows any user logged on the local host to change
.BR babeld 's
configuration.

Since Babel uses link-local IPv6 packets only, there is no need to update
firewalls to allow forwarding of Babel protocol packets. If local
filtering is being done, UDP datagrams to the port used by the protocol
Expand Down
52 changes: 30 additions & 22 deletions configuration.c
Expand Up @@ -848,7 +848,7 @@ parse_config_line(int c, gnc_t gnc, void *closure, int *action_return)
{
char *token;
if(action_return)
*action_return = CONFIG_DONE;
*action_return = CONFIG_ACTION_DONE;

c = skip_whitespace(c, gnc, closure);
if(c < 0 || c == '\n' || c == '#')
Expand All @@ -858,7 +858,34 @@ parse_config_line(int c, gnc_t gnc, void *closure, int *action_return)
if(c < -1)
return c;

if(strcmp(token, "in") == 0) {
/* Directives allowed in read-only mode */
if(strcmp(token, "quit") == 0) {
c = skip_eol(c, gnc, closure);
if(c < -1 || !action_return)
goto fail;
*action_return = CONFIG_ACTION_QUIT;
} else if(strcmp(token, "dump") == 0) {
c = skip_eol(c, gnc, closure);
if(c < -1 || !action_return)
goto fail;
*action_return = CONFIG_ACTION_DUMP;
} else if(strcmp(token, "monitor") == 0) {
c = skip_eol(c, gnc, closure);
if(c < -1 || !action_return)
goto fail;
*action_return = CONFIG_ACTION_MONITOR;
} else if(strcmp(token, "unmonitor") == 0) {
c = skip_eol(c, gnc, closure);
if(c < -1 || !action_return)
goto fail;
*action_return = CONFIG_ACTION_UNMONITOR;
} else if(config_finalised && !local_server_write) {
/* The remaining directives are only allowed in read-write mode. */
c = skip_to_eol(c, gnc, closure);
if(c < -1 || !action_return)
goto fail;
*action_return = CONFIG_ACTION_NO;
} else if(strcmp(token, "in") == 0) {
struct filter *filter;
c = parse_filter(c, gnc, closure, &filter);
if(c < -1)
Expand Down Expand Up @@ -916,31 +943,12 @@ parse_config_line(int c, gnc_t gnc, void *closure, int *action_return)
free(token2);
free(ifname);
}
} else if(strcmp(token, "quit") == 0) {
c = skip_eol(c, gnc, closure);
if(c < -1 || !action_return)
goto fail;
*action_return = CONFIG_QUIT;
} else if(strcmp(token, "dump") == 0) {
c = skip_eol(c, gnc, closure);
if(c < -1 || !action_return)
goto fail;
*action_return = CONFIG_DUMP;
} else if(strcmp(token, "monitor") == 0) {
c = skip_eol(c, gnc, closure);
if(c < -1 || !action_return)
goto fail;
*action_return = CONFIG_MONITOR;
} else if(strcmp(token, "unmonitor") == 0) {
c = skip_eol(c, gnc, closure);
if(c < -1 || !action_return)
goto fail;
*action_return = CONFIG_UNMONITOR;
} else {
c = parse_option(c, gnc, closure, token);
if(c < -1)
goto fail;
}

free(token);
return c;

Expand Down
11 changes: 6 additions & 5 deletions configuration.h
Expand Up @@ -22,11 +22,12 @@ THE SOFTWARE.

/* Values returned by parse_config_from_string. */

#define CONFIG_DONE 0
#define CONFIG_QUIT 1
#define CONFIG_DUMP 2
#define CONFIG_MONITOR 3
#define CONFIG_UNMONITOR 4
#define CONFIG_ACTION_DONE 0
#define CONFIG_ACTION_QUIT 1
#define CONFIG_ACTION_DUMP 2
#define CONFIG_ACTION_MONITOR 3
#define CONFIG_ACTION_UNMONITOR 4
#define CONFIG_ACTION_NO 5

struct filter_result {
unsigned int add_metric; /* allow = 0, deny = INF, metric = <0..INF> */
Expand Down
14 changes: 9 additions & 5 deletions local.c
Expand Up @@ -44,6 +44,7 @@ int local_server_socket = -1;
struct local_socket local_sockets[MAX_LOCAL_SOCKETS];
int num_local_sockets = 0;
int local_server_port = -1;
int local_server_write = 0;

static int
write_timeout(int fd, const void *buf, int len)
Expand Down Expand Up @@ -337,22 +338,25 @@ local_read(struct local_socket *s)

rc = parse_config_from_string(s->buf, eol + 1 - s->buf);
switch(rc) {
case CONFIG_DONE:
case CONFIG_ACTION_DONE:
break;
case CONFIG_QUIT:
case CONFIG_ACTION_QUIT:
shutdown(s->fd, 1);
reply = NULL;
break;
case CONFIG_DUMP:
case CONFIG_ACTION_DUMP:
local_notify_all_1(s);
break;
case CONFIG_MONITOR:
case CONFIG_ACTION_MONITOR:
local_notify_all_1(s);
s->monitor = 1;
break;
case CONFIG_UNMONITOR:
case CONFIG_ACTION_UNMONITOR:
s->monitor = 0;
break;
case CONFIG_ACTION_NO:
reply = "no\n";
break;
default:
reply = "bad\n";
}
Expand Down

0 comments on commit 3208c75

Please sign in to comment.