| Message ID | 20220113143935.195125-5-omosnace@redhat.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Allow rebuilding policy store only if there were external changes to modules | expand |
On Thu, Jan 13, 2022 at 6:37 PM Ondrej Mosnacek <omosnace@redhat.com> wrote: > > In Fedora/RHEL's selinux-policy package we ship a pre-built SELinux > policy store in the RPMs. When updating the main policy RPM, care must > be taken to rebuild the policy using `semodule -B` if there are any > other SELinux modules installed (whether shipped via another RPM or > manually installed locally). > > However, this way of shipping/managing the policy creates complications > on systems, where system files are managed by rpm-ostree (such as Fedora > CoreOS or Red Hat CoreOS), where the "package update" process is more > sophisticated. > > (Disclaimer: The following is written according to my current limited > understanding of rpm-ostree and may not be entirely accurate, but the > gist of it should match the reality.) > > Basically, one can think of rpm-ostree as a kind of Git for system > files. The package content is provided on a "branch", where each > "commit" represents a set of package updates layered on top of the > previous commit (i.e. it is a rolling release with some defined > package content snapshots). The user can then maintain their own branch > with additional package updates/installations/... and "rebase" it on top > of the main branch as needed. On top of that, the user can also have > additional configuration files (or modifications to existing files) in > /etc, which represent an additional layer on top of the package content. > > When updating the system (i.e. rebasing on a new "commit" of the "main > branch"), the files on the running system are not touched and the new > system state is prepared under a new root directory, which is chrooted > into on the next reboot. > > When an rpm-ostree system is updated, there are three moments when the > SELinux module store needs to be rebuilt to ensure that all modules are > included in the binary policy: > 1. When the local RPM installations are applied on top of the base > system snapshot. > 2. When local user configuartion is applied on top of that. > 3. On system shutdown, to ensure that any changes in local configuration > performed since (2.) are reflected in the final new system image. > > Forcing a full rebuild at each step is not optimal and in many cases is > not necessary, as the user may not have any custom modules installed. > > Thus, this patch extends libsemanage to compute a checksum of the > content of all enabled modules, which is stored in the store, and adds a > flag to the libsemanage handle that instructs it to skip rebuilding the > policy when the module content cheksum matches the one from the last > successful transaction and no other explicit changes are being done to > modules in the transaction itself. > > This will allow rpm-ostree systems to potentially reduce delays when > reconciling the module store when applying updates. > > I wasn't able to measure any noticeable overhead of the added hash > computation for every transaction (both before and after this change a > full policy rebuild took about 7 seconds on my test x86 VM). With the > new option check_ext_changes enabled, rebuilding a policy store with > unchanged modules took only about 0.96 seconds. > > Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com> > --- > libsemanage/include/semanage/handle.h | 5 + > libsemanage/src/direct_api.c | 301 +++++++++++++++++++------- > libsemanage/src/handle.c | 11 +- > libsemanage/src/handle.h | 1 + > libsemanage/src/libsemanage.map | 1 + > libsemanage/src/semanage_store.c | 1 + > libsemanage/src/semanage_store.h | 1 + > 7 files changed, 235 insertions(+), 86 deletions(-) > > diff --git a/libsemanage/include/semanage/handle.h b/libsemanage/include/semanage/handle.h > index 946d69bc..0157be4f 100644 > --- a/libsemanage/include/semanage/handle.h > +++ b/libsemanage/include/semanage/handle.h > @@ -66,6 +66,11 @@ extern void semanage_set_reload(semanage_handle_t * handle, int do_reload); > * 1 for yes, 0 for no (default) */ > extern void semanage_set_rebuild(semanage_handle_t * handle, int do_rebuild); > > +/* set whether to rebuild the policy on commit when potential changes > + * to module files since last rebuild are detected, > + * 1 for yes (default), 0 for no */ > +extern void semanage_set_check_ext_changes(semanage_handle_t * handle, int do_check); > + > /* Fills *compiler_path with the location of the hll compiler sh->conf->compiler_directory_path > * corresponding to lang_ext. > * Upon success returns 0, -1 on error. */ > diff --git a/libsemanage/src/direct_api.c b/libsemanage/src/direct_api.c > index 7eb6938b..ca55df07 100644 > --- a/libsemanage/src/direct_api.c > +++ b/libsemanage/src/direct_api.c > @@ -33,6 +33,8 @@ > #include <unistd.h> > #include <sys/stat.h> > #include <sys/types.h> > +#include <sys/mman.h> > +#include <sys/wait.h> > #include <limits.h> > #include <errno.h> > #include <dirent.h> > @@ -56,8 +58,7 @@ > #include "semanage_store.h" > #include "database_policydb.h" > #include "policy.h" > -#include <sys/mman.h> > -#include <sys/wait.h> > +#include "sha256.h" > > #define PIPE_READ 0 > #define PIPE_WRITE 1 > @@ -450,7 +451,7 @@ static int parse_module_headers(semanage_handle_t * sh, char *module_data, > /* Writes a block of data to a file. Returns 0 on success, -1 on > * error. */ > static int write_file(semanage_handle_t * sh, > - const char *filename, char *data, size_t num_bytes) > + const char *filename, const char *data, size_t num_bytes) > { > int out; > > @@ -850,8 +851,21 @@ cleanup: > return ret; > } > > +static void update_checksum_with_len(Sha256Context *context, size_t s) > +{ > + int i; > + uint8_t buffer[8]; > + > + for (i = 0; i < 8; i++) { > + buffer[i] = s & 0xff; > + s >>= 8; > + } > + Sha256Update(context, buffer, 8); > +} > + > static int semanage_compile_module(semanage_handle_t *sh, > - semanage_module_info_t *modinfo) > + semanage_module_info_t *modinfo, > + Sha256Context *context) > { > char cil_path[PATH_MAX]; > char hll_path[PATH_MAX]; > @@ -922,6 +936,11 @@ static int semanage_compile_module(semanage_handle_t *sh, > goto cleanup; > } > > + if (context) { > + update_checksum_with_len(context, cil_data_len); > + Sha256Update(context, cil_data, cil_data_len); > + } > + > status = write_compressed_file(sh, cil_path, cil_data, cil_data_len); > if (status == -1) { > ERR(sh, "Failed to write %s\n", cil_path); > @@ -950,18 +969,43 @@ cleanup: > return status; > } > > +static int modinfo_cmp(const void *a, const void *b) > +{ > + const semanage_module_info_t *ma = a; > + const semanage_module_info_t *mb = b; > + > + return strcmp(ma->name, mb->name); > +} > + > +static const char CHECKSUM_TYPE[] = "sha256"; > +static const size_t CHECKSUM_CONTENT_SIZE = sizeof(CHECKSUM_TYPE) + 1 + 2 * SHA256_HASH_SIZE; > + > static int semanage_compile_hll_modules(semanage_handle_t *sh, > - semanage_module_info_t *modinfos, > - int num_modinfos) > + semanage_module_info_t *modinfos, > + int num_modinfos, > + char *cil_checksum) > { > - int status = 0; > - int i; > + /* to be incremented when checksum input data format changes */ > + static const size_t CHECKSUM_EPOCH = 1; > + > + int i, status = 0; > char cil_path[PATH_MAX]; > struct stat sb; > + Sha256Context context; > + SHA256_HASH hash; > + struct file_contents contents = {}; > > assert(sh); > assert(modinfos); > > + /* Sort modules by name to get consistent ordering. */ > + qsort(modinfos, num_modinfos, sizeof(*modinfos), &modinfo_cmp); > + > + Sha256Initialise(&context); > + update_checksum_with_len(&context, CHECKSUM_EPOCH); > + > + /* prefix with module count to avoid collisions */ > + update_checksum_with_len(&context, num_modinfos); > for (i = 0; i < num_modinfos; i++) { > status = semanage_module_get_path( > sh, > @@ -969,31 +1013,107 @@ static int semanage_compile_hll_modules(semanage_handle_t *sh, > SEMANAGE_MODULE_PATH_CIL, > cil_path, > sizeof(cil_path)); > - if (status != 0) { > - goto cleanup; > - } > + if (status != 0) > + return -1; > > - if (semanage_get_ignore_module_cache(sh) == 0 && > - (status = stat(cil_path, &sb)) == 0) { > - continue; > - } > - if (status != 0 && errno != ENOENT) { > - ERR(sh, "Unable to access %s: %s\n", cil_path, strerror(errno)); > - goto cleanup; //an error in the "stat" call > + if (!semanage_get_ignore_module_cache(sh)) { > + status = stat(cil_path, &sb); > + if (status == 0) { > + status = map_compressed_file(sh, cil_path, &contents); > + if (status < 0) { > + ERR(sh, "Error mapping file: %s", cil_path); > + return -1; > + } > + > + /* prefix with length to avoid collisions */ > + update_checksum_with_len(&context, contents.len); > + Sha256Update(&context, contents.data, contents.len); > + > + unmap_compressed_file(&contents); > + continue; > + } else if (errno != ENOENT) { > + ERR(sh, "Unable to access %s: %s\n", cil_path, > + strerror(errno)); > + return -1; //an error in the "stat" call > + } > } > > - status = semanage_compile_module(sh, &modinfos[i]); > - if (status < 0) { > - goto cleanup; > + status = semanage_compile_module(sh, &modinfos[i], &context); > + if (status < 0) > + return -1; > + } > + Sha256Finalise(&context, &hash); > + > + cil_checksum += sprintf(cil_checksum, "%s:", CHECKSUM_TYPE); > + for (i = 0; i < SHA256_HASH_SIZE; i++) { > + cil_checksum += sprintf(cil_checksum, "%02x", > + (unsigned)hash.bytes[i]); > + } > + return 0; > +} > + > +static int semanage_compare_checksum(semanage_handle_t *sh, const char *reference) > +{ > + const char *path = semanage_path(SEMANAGE_TMP, SEMANAGE_MODULES_CHECKSUM); > + struct stat sb; > + int fd, retval; > + char *data; > + > + fd = open(path, O_RDONLY); > + if (fd == -1) { > + if (errno != ENOENT) { > + ERR(sh, "Unable to open %s: %s\n", path, strerror(errno)); > + return -1; > } > + /* Checksum file not present - force a rebuild. */ > + return 1; > + } > + > + if (fstat(fd, &sb) == -1) { > + ERR(sh, "Unable to stat %s\n", path); > + retval = -1; > + goto out_close; > } > > - status = 0; > + if (sb.st_size != CHECKSUM_CONTENT_SIZE) { > + /* Incompatible/invalid hash type - just force a rebuild. */ > + WARN(sh, "Module checksum invalid - forcing a rebuild\n"); > + retval = 1; > + goto out_close; > + } > > -cleanup: > - return status; > + data = mmap(NULL, CHECKSUM_CONTENT_SIZE, PROT_READ, MAP_PRIVATE, fd, 0); > + if (data == MAP_FAILED) { > + ERR(sh, "Unable to mmap %s\n", path); > + retval = -1; > + goto out_close; > + } > + > + retval = memcmp(data, reference, CHECKSUM_CONTENT_SIZE) != 0; > + munmap(data, sb.st_size); > +out_close: > + close(fd); > + return retval; > } > > +static int semanage_write_modules_checksum(semanage_handle_t *sh, > + const char *checksum) > +{ > + const char *path = semanage_path(SEMANAGE_TMP, SEMANAGE_MODULES_CHECKSUM); > + > + return write_file(sh, path, checksum, CHECKSUM_CONTENT_SIZE); > +} > + > +/* Files that must exist in order to skip policy rebuild. */ > +static const int semanage_computed_files[] = { > + SEMANAGE_STORE_KERNEL, > + SEMANAGE_STORE_FC, > + SEMANAGE_STORE_SEUSERS, > + SEMANAGE_LINKED, > + SEMANAGE_SEUSERS_LINKED, > + SEMANAGE_USERS_EXTRA_LINKED > +}; > + > /* Copies a file from src to dst. If dst already exists then > * overwrite it. If source doesn't exist then return success. > * Returns 0 on success, -1 on error. */ > @@ -1020,8 +1140,9 @@ static int semanage_direct_commit(semanage_handle_t * sh) > semanage_module_info_t *modinfos = NULL; > mode_t mask = umask(0077); > struct stat sb; > + char modules_checksum[CHECKSUM_CONTENT_SIZE]; > > - int do_rebuild, do_write_kernel, do_install; > + int force_rebuild, do_rebuild, do_write_kernel, do_install; > int fcontexts_modified, ports_modified, seusers_modified, > disable_dontaudit, preserve_tunables, ibpkeys_modified, > ibendports_modified; > @@ -1053,16 +1174,24 @@ static int semanage_direct_commit(semanage_handle_t * sh) > seusers_modified = seusers->dtable->is_modified(seusers->dbase); > fcontexts_modified = fcontexts->dtable->is_modified(fcontexts->dbase); > > + /* Before we do anything else, flush the join to its component parts. > + * This *does not* flush to disk automatically */ > + if (users->dtable->is_modified(users->dbase)) { > + retval = users->dtable->flush(sh, users->dbase); > + if (retval < 0) > + goto cleanup; > + } > + > /* Rebuild if explicitly requested or any module changes occurred. */ > - do_rebuild = sh->do_rebuild | sh->modules_modified; > + force_rebuild = sh->modules_modified; > > /* Create or remove the disable_dontaudit flag file. */ > path = semanage_path(SEMANAGE_TMP, SEMANAGE_DISABLE_DONTAUDIT); > if (stat(path, &sb) == 0) > - do_rebuild |= !(sepol_get_disable_dontaudit(sh->sepolh) == 1); > + force_rebuild |= !(sepol_get_disable_dontaudit(sh->sepolh) == 1); > else if (errno == ENOENT) { > /* The file does not exist */ > - do_rebuild |= (sepol_get_disable_dontaudit(sh->sepolh) == 1); > + force_rebuild |= (sepol_get_disable_dontaudit(sh->sepolh) == 1); > } else { > ERR(sh, "Unable to access %s: %s\n", path, strerror(errno)); > retval = -1; > @@ -1090,10 +1219,10 @@ static int semanage_direct_commit(semanage_handle_t * sh) > /* Create or remove the preserve_tunables flag file. */ > path = semanage_path(SEMANAGE_TMP, SEMANAGE_PRESERVE_TUNABLES); > if (stat(path, &sb) == 0) > - do_rebuild |= !(sepol_get_preserve_tunables(sh->sepolh) == 1); > + force_rebuild |= !(sepol_get_preserve_tunables(sh->sepolh) == 1); > else if (errno == ENOENT) { > /* The file does not exist */ > - do_rebuild |= (sepol_get_preserve_tunables(sh->sepolh) == 1); > + force_rebuild |= (sepol_get_preserve_tunables(sh->sepolh) == 1); > } else { > ERR(sh, "Unable to access %s: %s\n", path, strerror(errno)); > retval = -1; > @@ -1119,13 +1248,8 @@ static int semanage_direct_commit(semanage_handle_t * sh) > } > } > > - /* Before we do anything else, flush the join to its component parts. > - * This *does not* flush to disk automatically */ > - if (users->dtable->is_modified(users->dbase)) { > - retval = users->dtable->flush(sh, users->dbase); > - if (retval < 0) > - goto cleanup; > - } > + if (force_rebuild) > + goto rebuild; > > /* > * This is for systems that have already migrated with an older version > @@ -1135,70 +1259,66 @@ static int semanage_direct_commit(semanage_handle_t * sh) > * in order to skip re-linking are present; otherwise, we force > * a rebuild. > */ > - if (!do_rebuild) { > - int files[] = {SEMANAGE_STORE_KERNEL, > - SEMANAGE_STORE_FC, > - SEMANAGE_STORE_SEUSERS, > - SEMANAGE_LINKED, > - SEMANAGE_SEUSERS_LINKED, > - SEMANAGE_USERS_EXTRA_LINKED}; > - > - for (i = 0; i < (int) ARRAY_SIZE(files); i++) { > - path = semanage_path(SEMANAGE_TMP, files[i]); > - if (stat(path, &sb) != 0) { > - if (errno != ENOENT) { > - ERR(sh, "Unable to access %s: %s\n", path, strerror(errno)); > - retval = -1; > - goto cleanup; > - } > - > - do_rebuild = 1; > - goto rebuild; > + for (i = 0; i < (int) ARRAY_SIZE(semanage_computed_files); i++) { > + path = semanage_path(SEMANAGE_TMP, semanage_computed_files[i]); > + if (stat(path, &sb) != 0) { > + if (errno != ENOENT) { > + ERR(sh, "Unable to access %s: %s\n", path, strerror(errno)); > + retval = -1; > + goto cleanup; > } > + > + force_rebuild = 1; > + goto rebuild; > } > } > > rebuild: I know that the rebuild label and goto rebuild was there originally, but I would prefer to have it eliminated. instead of: if (force_rebuild) goto rebuild; ... for (... path = ... if (... ... force_rebuild = 1; goto rebuild; } } rebuild: why not: if (!force_rebuild) for (... path = ... if (... force_rebuild = 1; break; } } } Thanks, Jim > - /* > - * Now that we know whether or not a rebuild is required, > - * we can determine what else needs to be done. > - * We need to write the kernel policy if we are rebuilding > - * or if any other policy component that lives in the kernel > - * policy has been modified. > - * We need to install the policy files if any of the managed files > - * that live under /etc/selinux (kernel policy, seusers, file contexts) > - * will be modified. > - */ > - do_write_kernel = do_rebuild | ports_modified | ibpkeys_modified | > - ibendports_modified | > - bools->dtable->is_modified(bools->dbase) | > - ifaces->dtable->is_modified(ifaces->dbase) | > - nodes->dtable->is_modified(nodes->dbase) | > - users->dtable->is_modified(users_base->dbase); > - do_install = do_write_kernel | seusers_modified | fcontexts_modified; > - > - /* > - * If there were policy changes, or explicitly requested, or > - * any required files are missing, rebuild the policy. > - */ > - if (do_rebuild) { > - /* =================== Module expansion =============== */ > + do_rebuild = 0; > > + if (force_rebuild || sh->do_rebuild || sh->check_ext_changes) { > retval = semanage_get_active_modules(sh, &modinfos, &num_modinfos); > if (retval < 0) { > goto cleanup; > } > > + /* No modules - nothing to rebuild. */ > if (num_modinfos == 0) { > goto cleanup; > } > > - retval = semanage_compile_hll_modules(sh, modinfos, num_modinfos); > + retval = semanage_compile_hll_modules(sh, modinfos, num_modinfos, > + modules_checksum); > if (retval < 0) { > ERR(sh, "Failed to compile hll files into cil files.\n"); > goto cleanup; > } > > + if (force_rebuild) { > + do_rebuild = 1; > + } else if (sh->check_ext_changes) { > + retval = semanage_compare_checksum(sh, modules_checksum); > + if (retval < 0) > + goto cleanup; > + do_rebuild = retval; > + } else if (sh->do_rebuild) { > + do_rebuild = 1; > + } > + > + retval = semanage_write_modules_checksum(sh, modules_checksum); > + if (retval < 0) { > + ERR(sh, "Failed to write module checksum file.\n"); > + goto cleanup; > + } > + } > + > + /* > + * If there were policy changes, or explicitly requested, or > + * any required files are missing, rebuild the policy. > + */ > + if (do_rebuild) { > + /* =================== Module expansion =============== */ > + > retval = semanage_get_cil_paths(sh, modinfos, num_modinfos, &mod_filenames); > if (retval < 0) > goto cleanup; > @@ -1330,6 +1450,23 @@ rebuild: > } > } > > + /* > + * Determine what else needs to be done. > + * We need to write the kernel policy if we are rebuilding > + * or if any other policy component that lives in the kernel > + * policy has been modified. > + * We need to install the policy files if any of the managed files > + * that live under /etc/selinux (kernel policy, seusers, file contexts) > + * will be modified. > + */ > + do_write_kernel = do_rebuild | ports_modified | ibpkeys_modified | > + ibendports_modified | > + bools->dtable->is_modified(bools->dbase) | > + ifaces->dtable->is_modified(ifaces->dbase) | > + nodes->dtable->is_modified(nodes->dbase) | > + users->dtable->is_modified(users_base->dbase); > + do_install = do_write_kernel | seusers_modified | fcontexts_modified; > + > /* Attach our databases to the policydb we just created or loaded. */ > dbase_policydb_attach((dbase_policydb_t *) pusers_base->dbase, out); > dbase_policydb_attach((dbase_policydb_t *) pports->dbase, out); > @@ -1704,7 +1841,7 @@ static int semanage_direct_extract(semanage_handle_t * sh, > goto cleanup; > } > > - rc = semanage_compile_module(sh, _modinfo); > + rc = semanage_compile_module(sh, _modinfo, NULL); > if (rc < 0) { > goto cleanup; > } > diff --git a/libsemanage/src/handle.c b/libsemanage/src/handle.c > index bb1e6140..b2201ee3 100644 > --- a/libsemanage/src/handle.c > +++ b/libsemanage/src/handle.c > @@ -116,20 +116,23 @@ semanage_handle_t *semanage_handle_create(void) > > void semanage_set_rebuild(semanage_handle_t * sh, int do_rebuild) > { > - > assert(sh != NULL); > > sh->do_rebuild = do_rebuild; > - return; > } > > void semanage_set_reload(semanage_handle_t * sh, int do_reload) > { > - > assert(sh != NULL); > > sh->do_reload = do_reload; > - return; > +} > + > +void semanage_set_check_ext_changes(semanage_handle_t * sh, int do_check) > +{ > + assert(sh != NULL); > + > + sh->check_ext_changes = do_check; > } > > int semanage_get_hll_compiler_path(semanage_handle_t *sh, > diff --git a/libsemanage/src/handle.h b/libsemanage/src/handle.h > index e1ce83ba..4d2aae8f 100644 > --- a/libsemanage/src/handle.h > +++ b/libsemanage/src/handle.h > @@ -61,6 +61,7 @@ struct semanage_handle { > int is_in_transaction; > int do_reload; /* whether to reload policy after commit */ > int do_rebuild; /* whether to rebuild policy if there were no changes */ > + int check_ext_changes; /* whether to rebuild if external changes are detected via checksum */ > int commit_err; /* set by semanage_direct_commit() if there are > * any errors when building or committing the > * sandbox to kernel policy at /etc/selinux > diff --git a/libsemanage/src/libsemanage.map b/libsemanage/src/libsemanage.map > index 00259fc8..c8214b26 100644 > --- a/libsemanage/src/libsemanage.map > +++ b/libsemanage/src/libsemanage.map > @@ -348,4 +348,5 @@ LIBSEMANAGE_1.1 { > > LIBSEMANAGE_3.4 { > semanage_module_compute_checksum; > + semanage_set_check_ext_changes; > } LIBSEMANAGE_1.1; > diff --git a/libsemanage/src/semanage_store.c b/libsemanage/src/semanage_store.c > index 633ee731..767f05cb 100644 > --- a/libsemanage/src/semanage_store.c > +++ b/libsemanage/src/semanage_store.c > @@ -115,6 +115,7 @@ static const char *semanage_sandbox_paths[SEMANAGE_STORE_NUM_PATHS] = { > "/disable_dontaudit", > "/preserve_tunables", > "/modules/disabled", > + "/modules_checksum", > "/policy.kern", > "/file_contexts.local", > "/file_contexts.homedirs", > diff --git a/libsemanage/src/semanage_store.h b/libsemanage/src/semanage_store.h > index b9ec5664..1fc77da8 100644 > --- a/libsemanage/src/semanage_store.h > +++ b/libsemanage/src/semanage_store.h > @@ -60,6 +60,7 @@ enum semanage_sandbox_defs { > SEMANAGE_DISABLE_DONTAUDIT, > SEMANAGE_PRESERVE_TUNABLES, > SEMANAGE_MODULES_DISABLED, > + SEMANAGE_MODULES_CHECKSUM, > SEMANAGE_STORE_KERNEL, > SEMANAGE_STORE_FC_LOCAL, > SEMANAGE_STORE_FC_HOMEDIRS, > -- > 2.34.1 >
On Thu, Jan 20, 2022 at 11:08 PM James Carter <jwcart2@gmail.com> wrote: > On Thu, Jan 13, 2022 at 6:37 PM Ondrej Mosnacek <omosnace@redhat.com> wrote:\ [...] > > @@ -1119,13 +1248,8 @@ static int semanage_direct_commit(semanage_handle_t * sh) > > } > > } > > > > - /* Before we do anything else, flush the join to its component parts. > > - * This *does not* flush to disk automatically */ > > - if (users->dtable->is_modified(users->dbase)) { > > - retval = users->dtable->flush(sh, users->dbase); > > - if (retval < 0) > > - goto cleanup; > > - } > > + if (force_rebuild) > > + goto rebuild; > > > > /* > > * This is for systems that have already migrated with an older version > > @@ -1135,70 +1259,66 @@ static int semanage_direct_commit(semanage_handle_t * sh) > > * in order to skip re-linking are present; otherwise, we force > > * a rebuild. > > */ > > - if (!do_rebuild) { > > - int files[] = {SEMANAGE_STORE_KERNEL, > > - SEMANAGE_STORE_FC, > > - SEMANAGE_STORE_SEUSERS, > > - SEMANAGE_LINKED, > > - SEMANAGE_SEUSERS_LINKED, > > - SEMANAGE_USERS_EXTRA_LINKED}; > > - > > - for (i = 0; i < (int) ARRAY_SIZE(files); i++) { > > - path = semanage_path(SEMANAGE_TMP, files[i]); > > - if (stat(path, &sb) != 0) { > > - if (errno != ENOENT) { > > - ERR(sh, "Unable to access %s: %s\n", path, strerror(errno)); > > - retval = -1; > > - goto cleanup; > > - } > > - > > - do_rebuild = 1; > > - goto rebuild; > > + for (i = 0; i < (int) ARRAY_SIZE(semanage_computed_files); i++) { > > + path = semanage_path(SEMANAGE_TMP, semanage_computed_files[i]); > > + if (stat(path, &sb) != 0) { > > + if (errno != ENOENT) { > > + ERR(sh, "Unable to access %s: %s\n", path, strerror(errno)); > > + retval = -1; > > + goto cleanup; > > } > > + > > + force_rebuild = 1; > > + goto rebuild; > > } > > } > > > > rebuild: > > I know that the rebuild label and goto rebuild was there originally, > but I would prefer to have it eliminated. > > instead of: > if (force_rebuild) > goto rebuild; > ... > for (... > path = ... > if (... > ... > force_rebuild = 1; > goto rebuild; > } > } > > rebuild: > > why not: > if (!force_rebuild) > for (... > path = ... > if (... > force_rebuild = 1; > break; > } > } > } Good point, it really doesn't need to be there. I'll remove it in the next revision. Thanks for the review!
diff --git a/libsemanage/include/semanage/handle.h b/libsemanage/include/semanage/handle.h index 946d69bc..0157be4f 100644 --- a/libsemanage/include/semanage/handle.h +++ b/libsemanage/include/semanage/handle.h @@ -66,6 +66,11 @@ extern void semanage_set_reload(semanage_handle_t * handle, int do_reload); * 1 for yes, 0 for no (default) */ extern void semanage_set_rebuild(semanage_handle_t * handle, int do_rebuild); +/* set whether to rebuild the policy on commit when potential changes + * to module files since last rebuild are detected, + * 1 for yes (default), 0 for no */ +extern void semanage_set_check_ext_changes(semanage_handle_t * handle, int do_check); + /* Fills *compiler_path with the location of the hll compiler sh->conf->compiler_directory_path * corresponding to lang_ext. * Upon success returns 0, -1 on error. */ diff --git a/libsemanage/src/direct_api.c b/libsemanage/src/direct_api.c index 7eb6938b..ca55df07 100644 --- a/libsemanage/src/direct_api.c +++ b/libsemanage/src/direct_api.c @@ -33,6 +33,8 @@ #include <unistd.h> #include <sys/stat.h> #include <sys/types.h> +#include <sys/mman.h> +#include <sys/wait.h> #include <limits.h> #include <errno.h> #include <dirent.h> @@ -56,8 +58,7 @@ #include "semanage_store.h" #include "database_policydb.h" #include "policy.h" -#include <sys/mman.h> -#include <sys/wait.h> +#include "sha256.h" #define PIPE_READ 0 #define PIPE_WRITE 1 @@ -450,7 +451,7 @@ static int parse_module_headers(semanage_handle_t * sh, char *module_data, /* Writes a block of data to a file. Returns 0 on success, -1 on * error. */ static int write_file(semanage_handle_t * sh, - const char *filename, char *data, size_t num_bytes) + const char *filename, const char *data, size_t num_bytes) { int out; @@ -850,8 +851,21 @@ cleanup: return ret; } +static void update_checksum_with_len(Sha256Context *context, size_t s) +{ + int i; + uint8_t buffer[8]; + + for (i = 0; i < 8; i++) { + buffer[i] = s & 0xff; + s >>= 8; + } + Sha256Update(context, buffer, 8); +} + static int semanage_compile_module(semanage_handle_t *sh, - semanage_module_info_t *modinfo) + semanage_module_info_t *modinfo, + Sha256Context *context) { char cil_path[PATH_MAX]; char hll_path[PATH_MAX]; @@ -922,6 +936,11 @@ static int semanage_compile_module(semanage_handle_t *sh, goto cleanup; } + if (context) { + update_checksum_with_len(context, cil_data_len); + Sha256Update(context, cil_data, cil_data_len); + } + status = write_compressed_file(sh, cil_path, cil_data, cil_data_len); if (status == -1) { ERR(sh, "Failed to write %s\n", cil_path); @@ -950,18 +969,43 @@ cleanup: return status; } +static int modinfo_cmp(const void *a, const void *b) +{ + const semanage_module_info_t *ma = a; + const semanage_module_info_t *mb = b; + + return strcmp(ma->name, mb->name); +} + +static const char CHECKSUM_TYPE[] = "sha256"; +static const size_t CHECKSUM_CONTENT_SIZE = sizeof(CHECKSUM_TYPE) + 1 + 2 * SHA256_HASH_SIZE; + static int semanage_compile_hll_modules(semanage_handle_t *sh, - semanage_module_info_t *modinfos, - int num_modinfos) + semanage_module_info_t *modinfos, + int num_modinfos, + char *cil_checksum) { - int status = 0; - int i; + /* to be incremented when checksum input data format changes */ + static const size_t CHECKSUM_EPOCH = 1; + + int i, status = 0; char cil_path[PATH_MAX]; struct stat sb; + Sha256Context context; + SHA256_HASH hash; + struct file_contents contents = {}; assert(sh); assert(modinfos); + /* Sort modules by name to get consistent ordering. */ + qsort(modinfos, num_modinfos, sizeof(*modinfos), &modinfo_cmp); + + Sha256Initialise(&context); + update_checksum_with_len(&context, CHECKSUM_EPOCH); + + /* prefix with module count to avoid collisions */ + update_checksum_with_len(&context, num_modinfos); for (i = 0; i < num_modinfos; i++) { status = semanage_module_get_path( sh, @@ -969,31 +1013,107 @@ static int semanage_compile_hll_modules(semanage_handle_t *sh, SEMANAGE_MODULE_PATH_CIL, cil_path, sizeof(cil_path)); - if (status != 0) { - goto cleanup; - } + if (status != 0) + return -1; - if (semanage_get_ignore_module_cache(sh) == 0 && - (status = stat(cil_path, &sb)) == 0) { - continue; - } - if (status != 0 && errno != ENOENT) { - ERR(sh, "Unable to access %s: %s\n", cil_path, strerror(errno)); - goto cleanup; //an error in the "stat" call + if (!semanage_get_ignore_module_cache(sh)) { + status = stat(cil_path, &sb); + if (status == 0) { + status = map_compressed_file(sh, cil_path, &contents); + if (status < 0) { + ERR(sh, "Error mapping file: %s", cil_path); + return -1; + } + + /* prefix with length to avoid collisions */ + update_checksum_with_len(&context, contents.len); + Sha256Update(&context, contents.data, contents.len); + + unmap_compressed_file(&contents); + continue; + } else if (errno != ENOENT) { + ERR(sh, "Unable to access %s: %s\n", cil_path, + strerror(errno)); + return -1; //an error in the "stat" call + } } - status = semanage_compile_module(sh, &modinfos[i]); - if (status < 0) { - goto cleanup; + status = semanage_compile_module(sh, &modinfos[i], &context); + if (status < 0) + return -1; + } + Sha256Finalise(&context, &hash); + + cil_checksum += sprintf(cil_checksum, "%s:", CHECKSUM_TYPE); + for (i = 0; i < SHA256_HASH_SIZE; i++) { + cil_checksum += sprintf(cil_checksum, "%02x", + (unsigned)hash.bytes[i]); + } + return 0; +} + +static int semanage_compare_checksum(semanage_handle_t *sh, const char *reference) +{ + const char *path = semanage_path(SEMANAGE_TMP, SEMANAGE_MODULES_CHECKSUM); + struct stat sb; + int fd, retval; + char *data; + + fd = open(path, O_RDONLY); + if (fd == -1) { + if (errno != ENOENT) { + ERR(sh, "Unable to open %s: %s\n", path, strerror(errno)); + return -1; } + /* Checksum file not present - force a rebuild. */ + return 1; + } + + if (fstat(fd, &sb) == -1) { + ERR(sh, "Unable to stat %s\n", path); + retval = -1; + goto out_close; } - status = 0; + if (sb.st_size != CHECKSUM_CONTENT_SIZE) { + /* Incompatible/invalid hash type - just force a rebuild. */ + WARN(sh, "Module checksum invalid - forcing a rebuild\n"); + retval = 1; + goto out_close; + } -cleanup: - return status; + data = mmap(NULL, CHECKSUM_CONTENT_SIZE, PROT_READ, MAP_PRIVATE, fd, 0); + if (data == MAP_FAILED) { + ERR(sh, "Unable to mmap %s\n", path); + retval = -1; + goto out_close; + } + + retval = memcmp(data, reference, CHECKSUM_CONTENT_SIZE) != 0; + munmap(data, sb.st_size); +out_close: + close(fd); + return retval; } +static int semanage_write_modules_checksum(semanage_handle_t *sh, + const char *checksum) +{ + const char *path = semanage_path(SEMANAGE_TMP, SEMANAGE_MODULES_CHECKSUM); + + return write_file(sh, path, checksum, CHECKSUM_CONTENT_SIZE); +} + +/* Files that must exist in order to skip policy rebuild. */ +static const int semanage_computed_files[] = { + SEMANAGE_STORE_KERNEL, + SEMANAGE_STORE_FC, + SEMANAGE_STORE_SEUSERS, + SEMANAGE_LINKED, + SEMANAGE_SEUSERS_LINKED, + SEMANAGE_USERS_EXTRA_LINKED +}; + /* Copies a file from src to dst. If dst already exists then * overwrite it. If source doesn't exist then return success. * Returns 0 on success, -1 on error. */ @@ -1020,8 +1140,9 @@ static int semanage_direct_commit(semanage_handle_t * sh) semanage_module_info_t *modinfos = NULL; mode_t mask = umask(0077); struct stat sb; + char modules_checksum[CHECKSUM_CONTENT_SIZE]; - int do_rebuild, do_write_kernel, do_install; + int force_rebuild, do_rebuild, do_write_kernel, do_install; int fcontexts_modified, ports_modified, seusers_modified, disable_dontaudit, preserve_tunables, ibpkeys_modified, ibendports_modified; @@ -1053,16 +1174,24 @@ static int semanage_direct_commit(semanage_handle_t * sh) seusers_modified = seusers->dtable->is_modified(seusers->dbase); fcontexts_modified = fcontexts->dtable->is_modified(fcontexts->dbase); + /* Before we do anything else, flush the join to its component parts. + * This *does not* flush to disk automatically */ + if (users->dtable->is_modified(users->dbase)) { + retval = users->dtable->flush(sh, users->dbase); + if (retval < 0) + goto cleanup; + } + /* Rebuild if explicitly requested or any module changes occurred. */ - do_rebuild = sh->do_rebuild | sh->modules_modified; + force_rebuild = sh->modules_modified; /* Create or remove the disable_dontaudit flag file. */ path = semanage_path(SEMANAGE_TMP, SEMANAGE_DISABLE_DONTAUDIT); if (stat(path, &sb) == 0) - do_rebuild |= !(sepol_get_disable_dontaudit(sh->sepolh) == 1); + force_rebuild |= !(sepol_get_disable_dontaudit(sh->sepolh) == 1); else if (errno == ENOENT) { /* The file does not exist */ - do_rebuild |= (sepol_get_disable_dontaudit(sh->sepolh) == 1); + force_rebuild |= (sepol_get_disable_dontaudit(sh->sepolh) == 1); } else { ERR(sh, "Unable to access %s: %s\n", path, strerror(errno)); retval = -1; @@ -1090,10 +1219,10 @@ static int semanage_direct_commit(semanage_handle_t * sh) /* Create or remove the preserve_tunables flag file. */ path = semanage_path(SEMANAGE_TMP, SEMANAGE_PRESERVE_TUNABLES); if (stat(path, &sb) == 0) - do_rebuild |= !(sepol_get_preserve_tunables(sh->sepolh) == 1); + force_rebuild |= !(sepol_get_preserve_tunables(sh->sepolh) == 1); else if (errno == ENOENT) { /* The file does not exist */ - do_rebuild |= (sepol_get_preserve_tunables(sh->sepolh) == 1); + force_rebuild |= (sepol_get_preserve_tunables(sh->sepolh) == 1); } else { ERR(sh, "Unable to access %s: %s\n", path, strerror(errno)); retval = -1; @@ -1119,13 +1248,8 @@ static int semanage_direct_commit(semanage_handle_t * sh) } } - /* Before we do anything else, flush the join to its component parts. - * This *does not* flush to disk automatically */ - if (users->dtable->is_modified(users->dbase)) { - retval = users->dtable->flush(sh, users->dbase); - if (retval < 0) - goto cleanup; - } + if (force_rebuild) + goto rebuild; /* * This is for systems that have already migrated with an older version @@ -1135,70 +1259,66 @@ static int semanage_direct_commit(semanage_handle_t * sh) * in order to skip re-linking are present; otherwise, we force * a rebuild. */ - if (!do_rebuild) { - int files[] = {SEMANAGE_STORE_KERNEL, - SEMANAGE_STORE_FC, - SEMANAGE_STORE_SEUSERS, - SEMANAGE_LINKED, - SEMANAGE_SEUSERS_LINKED, - SEMANAGE_USERS_EXTRA_LINKED}; - - for (i = 0; i < (int) ARRAY_SIZE(files); i++) { - path = semanage_path(SEMANAGE_TMP, files[i]); - if (stat(path, &sb) != 0) { - if (errno != ENOENT) { - ERR(sh, "Unable to access %s: %s\n", path, strerror(errno)); - retval = -1; - goto cleanup; - } - - do_rebuild = 1; - goto rebuild; + for (i = 0; i < (int) ARRAY_SIZE(semanage_computed_files); i++) { + path = semanage_path(SEMANAGE_TMP, semanage_computed_files[i]); + if (stat(path, &sb) != 0) { + if (errno != ENOENT) { + ERR(sh, "Unable to access %s: %s\n", path, strerror(errno)); + retval = -1; + goto cleanup; } + + force_rebuild = 1; + goto rebuild; } } rebuild: - /* - * Now that we know whether or not a rebuild is required, - * we can determine what else needs to be done. - * We need to write the kernel policy if we are rebuilding - * or if any other policy component that lives in the kernel - * policy has been modified. - * We need to install the policy files if any of the managed files - * that live under /etc/selinux (kernel policy, seusers, file contexts) - * will be modified. - */ - do_write_kernel = do_rebuild | ports_modified | ibpkeys_modified | - ibendports_modified | - bools->dtable->is_modified(bools->dbase) | - ifaces->dtable->is_modified(ifaces->dbase) | - nodes->dtable->is_modified(nodes->dbase) | - users->dtable->is_modified(users_base->dbase); - do_install = do_write_kernel | seusers_modified | fcontexts_modified; - - /* - * If there were policy changes, or explicitly requested, or - * any required files are missing, rebuild the policy. - */ - if (do_rebuild) { - /* =================== Module expansion =============== */ + do_rebuild = 0; + if (force_rebuild || sh->do_rebuild || sh->check_ext_changes) { retval = semanage_get_active_modules(sh, &modinfos, &num_modinfos); if (retval < 0) { goto cleanup; } + /* No modules - nothing to rebuild. */ if (num_modinfos == 0) { goto cleanup; } - retval = semanage_compile_hll_modules(sh, modinfos, num_modinfos); + retval = semanage_compile_hll_modules(sh, modinfos, num_modinfos, + modules_checksum); if (retval < 0) { ERR(sh, "Failed to compile hll files into cil files.\n"); goto cleanup; } + if (force_rebuild) { + do_rebuild = 1; + } else if (sh->check_ext_changes) { + retval = semanage_compare_checksum(sh, modules_checksum); + if (retval < 0) + goto cleanup; + do_rebuild = retval; + } else if (sh->do_rebuild) { + do_rebuild = 1; + } + + retval = semanage_write_modules_checksum(sh, modules_checksum); + if (retval < 0) { + ERR(sh, "Failed to write module checksum file.\n"); + goto cleanup; + } + } + + /* + * If there were policy changes, or explicitly requested, or + * any required files are missing, rebuild the policy. + */ + if (do_rebuild) { + /* =================== Module expansion =============== */ + retval = semanage_get_cil_paths(sh, modinfos, num_modinfos, &mod_filenames); if (retval < 0) goto cleanup; @@ -1330,6 +1450,23 @@ rebuild: } } + /* + * Determine what else needs to be done. + * We need to write the kernel policy if we are rebuilding + * or if any other policy component that lives in the kernel + * policy has been modified. + * We need to install the policy files if any of the managed files + * that live under /etc/selinux (kernel policy, seusers, file contexts) + * will be modified. + */ + do_write_kernel = do_rebuild | ports_modified | ibpkeys_modified | + ibendports_modified | + bools->dtable->is_modified(bools->dbase) | + ifaces->dtable->is_modified(ifaces->dbase) | + nodes->dtable->is_modified(nodes->dbase) | + users->dtable->is_modified(users_base->dbase); + do_install = do_write_kernel | seusers_modified | fcontexts_modified; + /* Attach our databases to the policydb we just created or loaded. */ dbase_policydb_attach((dbase_policydb_t *) pusers_base->dbase, out); dbase_policydb_attach((dbase_policydb_t *) pports->dbase, out); @@ -1704,7 +1841,7 @@ static int semanage_direct_extract(semanage_handle_t * sh, goto cleanup; } - rc = semanage_compile_module(sh, _modinfo); + rc = semanage_compile_module(sh, _modinfo, NULL); if (rc < 0) { goto cleanup; } diff --git a/libsemanage/src/handle.c b/libsemanage/src/handle.c index bb1e6140..b2201ee3 100644 --- a/libsemanage/src/handle.c +++ b/libsemanage/src/handle.c @@ -116,20 +116,23 @@ semanage_handle_t *semanage_handle_create(void) void semanage_set_rebuild(semanage_handle_t * sh, int do_rebuild) { - assert(sh != NULL); sh->do_rebuild = do_rebuild; - return; } void semanage_set_reload(semanage_handle_t * sh, int do_reload) { - assert(sh != NULL); sh->do_reload = do_reload; - return; +} + +void semanage_set_check_ext_changes(semanage_handle_t * sh, int do_check) +{ + assert(sh != NULL); + + sh->check_ext_changes = do_check; } int semanage_get_hll_compiler_path(semanage_handle_t *sh, diff --git a/libsemanage/src/handle.h b/libsemanage/src/handle.h index e1ce83ba..4d2aae8f 100644 --- a/libsemanage/src/handle.h +++ b/libsemanage/src/handle.h @@ -61,6 +61,7 @@ struct semanage_handle { int is_in_transaction; int do_reload; /* whether to reload policy after commit */ int do_rebuild; /* whether to rebuild policy if there were no changes */ + int check_ext_changes; /* whether to rebuild if external changes are detected via checksum */ int commit_err; /* set by semanage_direct_commit() if there are * any errors when building or committing the * sandbox to kernel policy at /etc/selinux diff --git a/libsemanage/src/libsemanage.map b/libsemanage/src/libsemanage.map index 00259fc8..c8214b26 100644 --- a/libsemanage/src/libsemanage.map +++ b/libsemanage/src/libsemanage.map @@ -348,4 +348,5 @@ LIBSEMANAGE_1.1 { LIBSEMANAGE_3.4 { semanage_module_compute_checksum; + semanage_set_check_ext_changes; } LIBSEMANAGE_1.1; diff --git a/libsemanage/src/semanage_store.c b/libsemanage/src/semanage_store.c index 633ee731..767f05cb 100644 --- a/libsemanage/src/semanage_store.c +++ b/libsemanage/src/semanage_store.c @@ -115,6 +115,7 @@ static const char *semanage_sandbox_paths[SEMANAGE_STORE_NUM_PATHS] = { "/disable_dontaudit", "/preserve_tunables", "/modules/disabled", + "/modules_checksum", "/policy.kern", "/file_contexts.local", "/file_contexts.homedirs", diff --git a/libsemanage/src/semanage_store.h b/libsemanage/src/semanage_store.h index b9ec5664..1fc77da8 100644 --- a/libsemanage/src/semanage_store.h +++ b/libsemanage/src/semanage_store.h @@ -60,6 +60,7 @@ enum semanage_sandbox_defs { SEMANAGE_DISABLE_DONTAUDIT, SEMANAGE_PRESERVE_TUNABLES, SEMANAGE_MODULES_DISABLED, + SEMANAGE_MODULES_CHECKSUM, SEMANAGE_STORE_KERNEL, SEMANAGE_STORE_FC_LOCAL, SEMANAGE_STORE_FC_HOMEDIRS,
In Fedora/RHEL's selinux-policy package we ship a pre-built SELinux policy store in the RPMs. When updating the main policy RPM, care must be taken to rebuild the policy using `semodule -B` if there are any other SELinux modules installed (whether shipped via another RPM or manually installed locally). However, this way of shipping/managing the policy creates complications on systems, where system files are managed by rpm-ostree (such as Fedora CoreOS or Red Hat CoreOS), where the "package update" process is more sophisticated. (Disclaimer: The following is written according to my current limited understanding of rpm-ostree and may not be entirely accurate, but the gist of it should match the reality.) Basically, one can think of rpm-ostree as a kind of Git for system files. The package content is provided on a "branch", where each "commit" represents a set of package updates layered on top of the previous commit (i.e. it is a rolling release with some defined package content snapshots). The user can then maintain their own branch with additional package updates/installations/... and "rebase" it on top of the main branch as needed. On top of that, the user can also have additional configuration files (or modifications to existing files) in /etc, which represent an additional layer on top of the package content. When updating the system (i.e. rebasing on a new "commit" of the "main branch"), the files on the running system are not touched and the new system state is prepared under a new root directory, which is chrooted into on the next reboot. When an rpm-ostree system is updated, there are three moments when the SELinux module store needs to be rebuilt to ensure that all modules are included in the binary policy: 1. When the local RPM installations are applied on top of the base system snapshot. 2. When local user configuartion is applied on top of that. 3. On system shutdown, to ensure that any changes in local configuration performed since (2.) are reflected in the final new system image. Forcing a full rebuild at each step is not optimal and in many cases is not necessary, as the user may not have any custom modules installed. Thus, this patch extends libsemanage to compute a checksum of the content of all enabled modules, which is stored in the store, and adds a flag to the libsemanage handle that instructs it to skip rebuilding the policy when the module content cheksum matches the one from the last successful transaction and no other explicit changes are being done to modules in the transaction itself. This will allow rpm-ostree systems to potentially reduce delays when reconciling the module store when applying updates. I wasn't able to measure any noticeable overhead of the added hash computation for every transaction (both before and after this change a full policy rebuild took about 7 seconds on my test x86 VM). With the new option check_ext_changes enabled, rebuilding a policy store with unchanged modules took only about 0.96 seconds. Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com> --- libsemanage/include/semanage/handle.h | 5 + libsemanage/src/direct_api.c | 301 +++++++++++++++++++------- libsemanage/src/handle.c | 11 +- libsemanage/src/handle.h | 1 + libsemanage/src/libsemanage.map | 1 + libsemanage/src/semanage_store.c | 1 + libsemanage/src/semanage_store.h | 1 + 7 files changed, 235 insertions(+), 86 deletions(-)