Page MenuHomeFreeBSD
Differential D41464 Diff 147093 usr.sbin/syslogd/syslogd_cap_config.c

Changeset View

Standalone View

View Options

usr.sbin/syslogd/syslogd_cap_config.c

  • This file was added.
/*-
* SPDX-License-Identifier: BSD-2-Clause
*
* Copyright (c) 2023 The FreeBSD Foundation
*
* This software was developed by Jake Freeland <jfree@FreeBSD.org>
* under sponsorship from the FreeBSD Foundation.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
* are met:
* 1. Redistributions of source code must retain the above copyright
* notice, this list of conditions and the following disclaimer.
* 2. Redistributions in binary form must reproduce the above copyright
* notice, this list of conditions and the following disclaimer in the
* documentation and/or other materials provided with the distribution.
*
* THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' AND
* ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
* IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
* ARE DISCLAIMED. IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE
* FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
* DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
* OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
* HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
* LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
* OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
* SUCH DAMAGE.
*/
#include <sys/types.h>
#include <sys/socket.h>
#include <assert.h>
#include <err.h>
#include <libcasper.h>
#include <netdb.h>
#include <string.h>
#include <casper/cap_net.h>
#include "syslogd_cap.h"
/*
* Convert the given prop_filter structure into an nvlist.
* Return a heap allocated pointer to the resulting nvlist.
*/
nvlist_t *
prop_filter_to_nvlist(const struct prop_filter *pfilter)
{
nvlist_t *nvl_prop_filter = nvlist_create(0);
nvlist_add_number(nvl_prop_filter, "prop_type", pfilter->prop_type);
nvlist_add_number(nvl_prop_filter, "cmp_type", pfilter->cmp_type);
nvlist_add_number(nvl_prop_filter, "cmp_flags", pfilter->cmp_flags);
nvlist_add_string(nvl_prop_filter, "pflt_strval", pfilter->pflt_strval);
/*
markjUnsubmitted
Not Done
Inline Actions

In general, I'd like for us to move towards copying structures using nvlist_add_binary(), to try and simplify this tedious serialization code. Of course, special care is needed for pointers and file descriptors, but most of these fields don't belong in those two categories.

Once the patch set has landed, we should try and simplify this code further.

markj: In general, I'd like for us to move towards copying structures using nvlist_add_binary(), to…
* Do not bother adding pflt_re. It will be recompiled
* using pflt_strval later, if applicable.
*/
return (nvl_prop_filter);
}
/*
* Convert the given nvlist into a prop_filter structure.
* Return a heap allocated pointer to the resulting prop_filter.
*/
struct prop_filter *
nvlist_to_prop_filter(const nvlist_t *nvl_prop_filter)
{
struct prop_filter *pfilter;
pfilter = calloc(1, sizeof(*pfilter));
markjUnsubmitted
Done
Inline Actions

What's the point of having filed_count as an nvlist element? Doesn't nvlist_get_nvlist_array() return the size of the array in filed_count?

markj: What's the point of having `filed_count` as an nvlist element? Doesn't nvlist_get_nvlist_array…
if (pfilter == NULL)
err(1, "calloc");
pfilter->prop_type = nvlist_get_number(nvl_prop_filter, "prop_type");
pfilter->cmp_type = nvlist_get_number(nvl_prop_filter, "cmp_type");
pfilter->cmp_flags = nvlist_get_number(nvl_prop_filter, "cmp_flags");
pfilter->pflt_strval = strdup(nvlist_get_string(nvl_prop_filter,
"pflt_strval"));
if (pfilter->cmp_type == FILT_CMP_REGEX) {
markjUnsubmitted
Done
Inline Actions

Does the casper service do any validation of the config file path? Doesn't this interface allow syslogd to parse any file as a syslogd configuration file and derive capabilities from it?

Most likely we should handle this by making the configuration service take a limit, set by cap_limit_set(). During initialization, syslogd would limit the service to only /etc/syslog.conf (or whatever is specified by -f). An attempt to parse any other file should be rejected by the casper service.

markj: Does the casper service do any validation of the config file path? Doesn't this interface allow…
jfreeAuthorUnsubmitted
Done
Inline Actions

Does the casper service do any validation of the config file path? Doesn't this interface allow syslogd to parse any file as a syslogd configuration file and derive capabilities from it?

Most likely we should handle this by making the configuration service take a limit, set by cap_limit_set(). During initialization, syslogd would limit the service to only /etc/syslog.conf (or whatever is specified by -f). An attempt to parse any other file should be rejected by the casper service.

I made ConfFile a global variable and did a strcmp() to verify that the passed-in path matched libcasper's original copy.

jfree: > Does the casper service do any validation of the config file path? Doesn't this interface…
int re_flags = REG_NOSUB;
pfilter->pflt_re = calloc(1, sizeof(*pfilter->pflt_re));
if (pfilter->pflt_re == NULL)
errx(1, "RE calloc() error");
if ((pfilter->cmp_flags & FILT_FLAG_EXTENDED) != 0)
re_flags |= REG_EXTENDED;
if ((pfilter->cmp_flags & FILT_FLAG_ICASE) != 0)
re_flags |= REG_ICASE;
if (regcomp(pfilter->pflt_re, pfilter->pflt_strval,
re_flags) != 0)
errx(1, "RE compilation error");
}
return (pfilter);
}
/*
* Convert the given struct filed into an nvl_filed nvlist.
* Return a heap allocated pointer to the resulting nvlist.
*/
nvlist_t *
filed_to_nvlist(const struct filed *filed)
{
nvlist_t *nvl_filed = nvlist_create(0);
enum f_type f_type = filed->f_type;
size_t i, sz;
nvlist_add_number(nvl_filed, "f_type", f_type);
nvlist_add_string(nvl_filed, "f_host", filed->f_host);
nvlist_add_string(nvl_filed, "f_program", filed->f_program);
if (filed->f_prop_filter != NULL) {
nvlist_add_nvlist(nvl_filed, "f_prop_filter",
prop_filter_to_nvlist(filed->f_prop_filter));
}
sz = nitems(filed->f_pmask);
for (i = 0; i < sz; ++i) {
nvlist_append_number_array(nvl_filed, "f_pmask",
filed->f_pmask[i]);
}
sz = nitems(filed->f_pcmp);
for (i = 0; i < sz; ++i) {
nvlist_append_number_array(nvl_filed, "f_pcmp",
filed->f_pcmp[i]);
}
if (filed->f_file >= 0)
nvlist_add_descriptor(nvl_filed, "f_file", filed->f_file);
nvlist_add_number(nvl_filed, "f_flags", filed->f_flags);
markjUnsubmitted
Done
Inline Actions

Per my comment in D41463, we should continue factoring this code out to make it less confusing. In an ideal design, IMO, readconfigfile() would return the nvlist instead of mucking with global variables.

That doesn't need to happen in this revision, it's just a comment about the direction I think we should go.

markj: Per my comment in D41463, we should continue factoring this code out to make it less confusing.
if (f_type == F_WALL || f_type == F_USERS) {
sz = nitems(filed->f_uname);
for (i = 0; i < sz; ++i) {
nvlist_append_string_array(nvl_filed, "f_uname",
filed->f_uname[i]);
}
} else if (f_type == F_FILE || f_type == F_CONSOLE || f_type == F_TTY) {
nvlist_add_string(nvl_filed, "f_fname", filed->f_fname);
} else if (f_type == F_FORW) {
struct addrinfo *ai = filed->f_addr, *cur;
nvlist_t *nvl_addrinfo;
nvlist_add_string(nvl_filed, "f_hname", filed->f_hname);
if (filed->f_addr != NULL) {
for (cur = ai; cur != NULL; cur = cur->ai_next) {
nvl_addrinfo = addrinfo_pack(cur);
nvlist_append_nvlist_array(nvl_filed,
markjUnsubmitted
Done
Inline Actions

This is maybe a painful request, but I wonder if it's possible to avoid having to serialize addrinfo here at all. syslogd really just wants the socket addr and address family. If we go down this path, it can be done in a separate patch.

markj: This is maybe a painful request, but I wonder if it's possible to avoid having to serialize…
jfreeAuthorUnsubmitted
Done
Inline Actions

As you pointed out in another comment, nvlist_add_binary() would probably be less tedious since a lot of struct filed members are inline (not pointers or descriptors).

I think I originally went with this approach because I didn't like the idea of storing a binary blob with invalid descriptor numbers and pointers; I feel like doing that generally leads to confusion for future contributors that are not completely familiar with which specific members should be xfered outside of the binary blob.

jfree: As you pointed out in another comment, nvlist_add_binary() would probably be less tedious since…
"f_addr", nvl_addrinfo);
nvlist_destroy(nvl_addrinfo);
}
}
} else if (filed->f_type == F_PIPE) {
nvlist_add_string(nvl_filed, "f_pname", filed->f_pname);
if (filed->f_procdesc >= 0) {
nvlist_add_descriptor(nvl_filed, "f_procdesc",
filed->f_procdesc);
}
}
/*
* Book-keeping fields are not transferred.
*/
return (nvl_filed);
}
/*
* Convert the given nvl_filed nvlist into a struct filed.
* Return a heap allocated pointer to the resulting struct
* filed.
*/
struct filed *
nvlist_to_filed(const nvlist_t *nvl_filed)
{
struct filed *filed;
enum f_type f_type;
const uint64_t *narr;
size_t i, sz;
filed = calloc(1, sizeof(*filed));
if (filed == NULL)
err(1, "calloc");
f_type = filed->f_type = nvlist_get_number(nvl_filed, "f_type");
(void)strlcpy(filed->f_host, nvlist_get_string(nvl_filed, "f_host"),
sizeof(filed->f_host));
(void)strlcpy(filed->f_program, nvlist_get_string(nvl_filed,
"f_program"), sizeof(filed->f_program));
if (nvlist_exists_nvlist(nvl_filed, "f_prop_filter")) {
filed->f_prop_filter = nvlist_to_prop_filter(
nvlist_get_nvlist(nvl_filed, "f_prop_filter"));
}
narr = nvlist_get_number_array(nvl_filed, "f_pmask", &sz);
assert(sz == nitems(filed->f_pmask));
for (i = 0; i < sz; ++i)
filed->f_pmask[i] = narr[i];
narr = nvlist_get_number_array(nvl_filed, "f_pcmp", &sz);
assert(sz == nitems(filed->f_pcmp));
for (i = 0; i < sz; ++i)
filed->f_pcmp[i] = narr[i];
if (nvlist_exists_descriptor(nvl_filed, "f_file"))
filed->f_file = dup(nvlist_get_descriptor(nvl_filed, "f_file"));
else
filed->f_file = -1;
filed->f_flags = nvlist_get_number(nvl_filed, "f_flags");
if (f_type == F_WALL || f_type == F_USERS) {
const char * const *f_uname;
f_uname = nvlist_get_string_array(nvl_filed, "f_uname", &sz);
assert(sz == nitems(filed->f_uname));
for (i = 0; i < sz; ++i) {
(void)strlcpy(filed->f_uname[i], f_uname[i],
sizeof(filed->f_uname[i]));
}
} else if (f_type == F_FILE || f_type == F_CONSOLE || f_type == F_TTY) {
(void)strlcpy(filed->f_fname, nvlist_get_string(nvl_filed,
"f_fname"), sizeof(filed->f_fname));
} else if (f_type == F_FORW) {
const nvlist_t * const *f_addr;
struct addrinfo *ai, **next = NULL;
(void)strlcpy(filed->f_hname, nvlist_get_string(nvl_filed,
"f_hname"), sizeof(filed->f_hname));
f_addr = nvlist_get_nvlist_array(nvl_filed, "f_addr", &sz);
for (i = 0; i < sz; ++i) {
ai = addrinfo_unpack(f_addr[i]);
if (next == NULL)
filed->f_addr = ai;
else
*next = ai;
next = &ai->ai_next;
}
} else if (filed->f_type == F_PIPE) {
(void)strlcpy(filed->f_pname, nvlist_get_string(nvl_filed,
"f_pname"), sizeof(filed->f_pname));
if (nvlist_exists_descriptor(nvl_filed, "f_procdesc")) {
filed->f_procdesc = dup(nvlist_get_descriptor(nvl_filed,
"f_procdesc"));
} else {
markjUnsubmitted
Done
Inline Actions

Can we not use nvlist_take_descriptor() here?

markj: Can we not use nvlist_take_descriptor() here?
jfreeAuthorUnsubmitted
Done
Inline Actions

This function does not modify the state of nvl_filed. Using take_descriptor() would remove (and free) the descriptor from nvl_filed. We just get it and dup(2) it instead

jfree: This function does not modify the state of `nvl_filed`. Using take_descriptor() would remove…
filed->f_procdesc = -1;
}
}
/*
* Book-keeping fields are not transferred.
*/
return (filed);
}
nvlist_t *
cap_readconfigfile(cap_channel_t *chan, const char *path)
{
nvlist_t *nvl, *nvl_conf;
nvl = nvlist_create(0);
nvlist_add_string(nvl, "cmd", "readconfigfile");
nvlist_add_string(nvl, "path", path);
/* It is possible that our hostname has changed. */
nvlist_add_string(nvl, "LocalHostName", LocalHostName);
nvl = cap_xfer_nvlist(chan, nvl);
if (nvl == NULL) {
logerror("Failed to xfer configuration nvlist");
exit(1);
}
nvl_conf = nvlist_take_nvlist(nvl, "nvl_conf");
nvlist_destroy(nvl);
return (nvl_conf);
}
/*
* Now that we're executing as libcasper, we can obtain the
* resources specified in the configuration.
*/
int
casper_readconfigfile(nvlist_t *nvlin, nvlist_t *nvlout)
{
const char *path;
/*
* Verify that syslogd did not manipulate the
* configuration file path.
*/
path = nvlist_get_string(nvlin, "path");
if (strcmp(path, ConfFile) != 0)
err(1, "Configuration file mismatch: %s != %s", path, ConfFile);
/* Refresh our copy of LocalHostName, in case it changed. */
strlcpy(LocalHostName, nvlist_get_string(nvlin, "LocalHostName"),
sizeof(LocalHostName));
nvlist_move_nvlist(nvlout, "nvl_conf", readconfigfile(path));
return (0);
}