diff --git a/.codeql-config.yml b/.codeql-config.yml new file mode 100644 index 0000000..1489a54 --- /dev/null +++ b/.codeql-config.yml @@ -0,0 +1,4 @@ +name: "CodeQL config" + +paths-ignore: + - test/** diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index b7d572d..431e7b5 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -1,4 +1,4 @@ -name: Run CI +name: Build and Test permissions: contents: read pull-requests: read @@ -13,7 +13,7 @@ on: - main jobs: - compile: + build-test: runs-on: ubuntu-latest container: image: fedora:latest @@ -21,12 +21,7 @@ jobs: name: Test compile steps: - - name: checkout code tree - uses: actions/checkout@v6 - with: - fetch-depth: 0 - - - name: install dependencies + - name: Install build dependencies run: | dnf -y update dnf -y install \ @@ -36,12 +31,34 @@ jobs: cmake \ libcap-devel \ libseccomp-devel \ - asciidoc + asciidoc \ + checksec \ + shellcheck \ + bats \ + hostname \ + lcov - - name: run build + - name: Checkout + uses: actions/checkout@v6 + with: + fetch-depth: 0 + + - name: Configure + run: | + cmake -B build -S . -Wdev -Wdeprecated \ + -DCMAKE_BUILD_TYPE=RelWithDebInfo \ + -DUSE_SECCOMP=ON \ + -DUSE_LANDLOCK=ON + + - name: Build + run: | + export LDFLAGS="-Wl,--as-needed -Wl,-z,relro -Wl,-z,now -Wl,-z,combreloc" + cmake --build build --parallel --verbose + + - name: Test run: | - mkdir build cd build - cmake .. - cmake --build . --verbose - ctest --output-on-failure + ctest --output-on-failure --verbose + + - name: Coverage + run: make -C build coverage diff --git a/CMakeLists.txt b/CMakeLists.txt index f687f04..6cb542b 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -5,6 +5,11 @@ project(kcron C) set(LANGUAGES "C") +################################ +# Testing (CTest) +enable_testing() +include(CTest) + if(NOT CMAKE_BUILD_TYPE) set(CMAKE_BUILD_TYPE RelWithDebInfo CACHE STRING "Build type (Debug, Release, RelWithDebInfo, MinSizeRel)" FORCE @@ -53,6 +58,7 @@ endif(EXISTS "${LOC_PATH}") include(doc/CMakeLists.txt) include(src/C/CMakeLists.txt) include(src/shell/CMakeLists.txt) +add_subdirectory(test) #### # Print out feature summary diff --git a/Makefile b/Makefile index bec6448..e17cec4 100644 --- a/Makefile +++ b/Makefile @@ -7,7 +7,7 @@ _default: sources: @echo "You found my koji hook" @mkdir kcron - @cp -r doc src CMakeLists.txt LICENSE README.md kcron + @cp -r doc src test CMakeLists.txt LICENSE README.md kcron tar cf - kcron | gzip --best > $(current_dir)/kcron.tar.gz rm -rf kcron srpm: sources diff --git a/README.md b/README.md index 0e717d1..03471f4 100644 --- a/README.md +++ b/README.md @@ -65,4 +65,48 @@ You may change the `/var/kerberos/krb5/user/` to an alternate location at build The `Makefile` is not setting CAPIBILITIES on the binary. This is by design. +## Testing + +Tests are managed by CTest. Run the full suite after configuring: + +```bash +mkdir build && cd build +cmake .. +make test # builds and runs every test via ctest +# or: +ctest --output-on-failure +``` + +The suite (see `test/`) covers the areas requested in issues [#2] and [#9]: + + * **C unit tests** (`test_kcron_filename`, `test_kcron_keytab`, + `test_client_keytab_name`) use the minimal TAP 14 test framework ported from + `fermitools/fnal-vncpasswd`. Each test runs in its own process so a + failing assertion cannot corrupt the others. + * **Shell syntax** (`bash -n`) for `kcroninit`, `kcrondestroy`, and + `kcron.sysconfig`. + * **Sysconfig derivation** (`test_sysconfig.sh`) validates that + `REALM` / `WHOAMI` / `FULLPRINCIPAL` are derived as documented, offline. + * **Shell suite** via `bats` (`test/kcron.bats`) exercises the kcron shell + utilities' control flow with every Kerberos tooling command stubbed, so the + tests need no KDC (skipped automatically if `bats` is unavailable). + * **Static analysis** via `shellcheck` (skipped automatically if unavailable). + * **Hardening check** via `checksec` or `hardening-check` against the built + binaries (ported from the RPM spec `%check`, and skipped automatically if + neither auditor is installed). + +When `lcov` and `genhtml` are present, a coverage report can be generated: + +```bash +make coverage # writes coverage/index.html and prints a summary +``` + +All optional tools degrade gracefully: if `bats`/`checksec`/`hardening-check`/ +`shellcheck`/`lcov` are not installed on the toolchain, the corresponding tests +are registered only when the tool is available (or pass as a no-op) so that +minimal builds still configure and pass. + +[#2]: https://github.com/fermitools/kcron/issues/2 +[#9]: https://github.com/fermitools/kcron/issues/9 + See the [documentation](https://github.com/fermitools/kcron/tree/main/doc) folder for more information. diff --git a/fermilab-util_kcron.spec b/fermilab-util_kcron.spec index a13e3ea..c44041e 100644 --- a/fermilab-util_kcron.spec +++ b/fermilab-util_kcron.spec @@ -36,6 +36,8 @@ BuildRequires: kernel-devel BuildRequires: cmake >= 3.14 BuildRequires: redhat-rpm-config coreutils bash gcc BuildRequires: libcap libcap-devel +BuildRequires: bats +BuildRequires: shellcheck %if 0%{?rhel} < 10 @@ -101,31 +103,14 @@ make install DESTDIR=%{buildroot} %check -for code in $(ls %{buildroot}%{_bindir}); do - bash -n %{buildroot}%{_bindir}/${code} - if [[ $? -ne 0 ]]; then - exit 1 - fi -done -bash -n %{buildroot}%{_sysconfdir}/sysconfig/kcron -if [[ $? -ne 0 ]]; then - exit 1 -fi - -%if %{_hardened_build} -for code in $(ls %{buildroot}%{_libexecdir}/kcron); do - checksec --file=%{buildroot}%{_libexecdir}/kcron/${code} - if [[ $? -ne 0 ]]; then - exit 1 - fi - - checksec --fortify-file=%{buildroot}%{_libexecdir}/kcron/${code} - if [[ $? -ne 0 ]]; then - exit 1 - fi -done +%if 0%{?rhel} < 9 && 0%{?fedora} < 31 +cd build +ctest --output-on-failure +%else +%ctest --output-on-failure %endif + %post %{__mkdir_p} --mode=0755 %{_localstatedir}/kerberos/krb5/user %{__chmod} 0751 %{_localstatedir}/kerberos/krb5/user diff --git a/src/C/create-empty-client-keytab.c b/src/C/create-empty-client-keytab.c index d3d8960..e26df1f 100644 --- a/src/C/create-empty-client-keytab.c +++ b/src/C/create-empty-client-keytab.c @@ -58,6 +58,7 @@ #include "kcron_empty_keytab_file.h" #include "kcron_filename.h" +#include "kcron_keytab_file.h" #ifndef _0600 #define _0600 (S_IRUSR | S_IWUSR) @@ -82,139 +83,6 @@ static void free_buffers(char *keytab, char *keytab_dirname, char *keytab_filena } -static int validate_keytab_directory(const char *keytab_dirname) __attribute__((access(read_only, 1))) __attribute__((warn_unused_result)); -static int validate_keytab_directory(const char *keytab_dirname) { - struct stat lst = {0}; - struct stat st = {0}; - - if (keytab_dirname == NULL) { - (void)fprintf(stderr, "%s: Keytab directory pointer is NULL.\n", __PROGRAM_NAME); - return 1; - } - - /* Check symlink on keytab_dirname before stat to prevent symlink attacks */ - if (lstat(keytab_dirname, &lst) != 0) { - (void)fprintf(stderr, "%s: Keytab directory does not exist: %s.\n", __PROGRAM_NAME, keytab_dirname); - (void)fprintf(stderr, "%s: Run init-kerberos-client-dir first.\n", __PROGRAM_NAME); - return 1; - } - if (S_ISLNK(lst.st_mode)) { - (void)fprintf(stderr, "%s: Keytab directory %s is a symlink, not allowed.\n", __PROGRAM_NAME, keytab_dirname); - return 1; - } - - if (stat(keytab_dirname, &st) == -1) { - (void)fprintf(stderr, "%s: Keytab directory does not exist: %s.\n", __PROGRAM_NAME, keytab_dirname); - (void)fprintf(stderr, "%s: Run init-kerberos-client-dir first.\n", __PROGRAM_NAME); - return 1; - } - - if (!S_ISDIR(st.st_mode)) { - (void)fprintf(stderr, "%s: Keytab directory %s is not a directory.\n", __PROGRAM_NAME, keytab_dirname); - return 1; - } - - return 0; -} - -static int create_keytab_file(const char *keytab_dirname, const char *keytab_filename, const char *keytab) __attribute__((warn_unused_result)); -static int create_keytab_file(const char *keytab_dirname, const char *keytab_filename, const char *keytab) { - struct stat st = {0}; - struct stat lst = {0}; - DIR *keytab_dir = NULL; - int filedescriptor = -1; - - /* Validate non-null input pointers */ - if (keytab_dirname == NULL) { - (void)fprintf(stderr, "%s: keytab_dirname pointer is NULL.\n", __PROGRAM_NAME); - return 1; - } - if (keytab_filename == NULL) { - (void)fprintf(stderr, "%s: keytab_filename pointer is NULL.\n", __PROGRAM_NAME); - return 1; - } - if (keytab == NULL) { - (void)fprintf(stderr, "%s: keytab pointer is NULL.\n", __PROGRAM_NAME); - return 1; - } - - /* Check if keytab_dirname exists and is not a symlink (to avoid TOCTOU symlink attacks) */ - if (lstat(keytab_dirname, &lst) != 0) { - (void)fprintf(stderr, "%s: %s does not exist.\n", __PROGRAM_NAME, keytab_dirname); - return 1; - } - if (S_ISLNK(lst.st_mode)) { - (void)fprintf(stderr, "%s: %s is a symlink, not allowed.\n", __PROGRAM_NAME, keytab_dirname); - return 1; - } - - /* Open the directory handle to perform file operations safely on the directory inode */ - keytab_dir = opendir(keytab_dirname); - if (keytab_dir == NULL) { - (void)fprintf(stderr, "%s: Unable to open %s.\n", __PROGRAM_NAME, keytab_dirname); - (void)fprintf(stderr, "%s: This may be a permissions error?\n", __PROGRAM_NAME); - return 1; - } - - /* Verify the opened directory is indeed a directory */ - if (fstat(dirfd(keytab_dir), &st) != 0) { - (void)fprintf(stderr, "%s: %s could not be read.\n", __PROGRAM_NAME, keytab_dirname); - (void)closedir(keytab_dir); - return 1; - } - - if (!S_ISDIR(st.st_mode)) { - (void)fprintf(stderr, "%s: %s is not a directory.\n", __PROGRAM_NAME, keytab_dirname); - (void)closedir(keytab_dir); - return 1; - } - - /* Open the keytab file with O_NOFOLLOW to prevent symlink attacks, - * O_CREAT | O_EXCL to create only if missing and atomically detect existence, - * and O_CLOEXEC for descriptor safety. Permissions set to 0600 for security. - */ - filedescriptor = openat(dirfd(keytab_dir), keytab_filename, O_WRONLY | O_CREAT | O_EXCL | O_NOFOLLOW | O_CLOEXEC, _0600); - if (filedescriptor < 0) { - if (errno == EEXIST) { - (void)fprintf(stderr, "%s: %s already exists.\n", __PROGRAM_NAME, keytab); - } else { - (void)fprintf(stderr, "%s: %s cannot be created: %s\n", __PROGRAM_NAME, keytab, strerror(errno)); - } - (void)closedir(keytab_dir); - return 1; - } - (void)closedir(keytab_dir); - - /* Verify that the created file is a regular file */ - if (fstat(filedescriptor, &st) != 0) { - (void)close(filedescriptor); - (void)fprintf(stderr, "%s: %s could not be created.\n", __PROGRAM_NAME, keytab); - return 1; - } - if (!S_ISREG(st.st_mode)) { - (void)close(filedescriptor); - (void)fprintf(stderr, "%s: %s is not a regular file.\n", __PROGRAM_NAME, keytab); - return 1; - } - - /* Write empty keytab content to the file */ - if (write_empty_keytab(filedescriptor) != 0) { - (void)close(filedescriptor); - (void)fprintf(stderr, "%s: Cannot create keytab : %s.\n", __PROGRAM_NAME, keytab); - return 1; - } - - /* Ensure permissions are exactly 0600 */ - if (fchmod(filedescriptor, _0600) != 0) { - (void)close(filedescriptor); - (void)fprintf(stderr, "%s: Unable to chmod %o %s\n", __PROGRAM_NAME, _0600, keytab); - return 1; - } - - (void)close(filedescriptor); - return 0; -} - int main(void) { char *keytab = calloc(FILE_PATH_MAX_LENGTH + 3, sizeof(char)); char *keytab_dirname = calloc(FILE_PATH_MAX_LENGTH + 3, sizeof(char)); diff --git a/src/C/init-kerberos-client-dir.c b/src/C/init-kerberos-client-dir.c index edae75c..ce181ae 100644 --- a/src/C/init-kerberos-client-dir.c +++ b/src/C/init-kerberos-client-dir.c @@ -58,6 +58,7 @@ #include "kcron_caps.h" #include "kcron_filename.h" +#include "kcron_keytab_file.h" #include "kcron_setup.h" #ifndef _0600 @@ -182,35 +183,6 @@ static int mkdir_if_missing(const char *dir, uid_t owner, gid_t group, mode_t mo return 0; } -static int validate_client_dirname(char *client_keytab_dirname) __attribute__((warn_unused_result)); -static int validate_client_dirname(char *client_keytab_dirname) { - struct stat lst = {0}; - struct stat st = {0}; - - if (client_keytab_dirname == NULL) { - (void)fprintf(stderr, "%s: Client keytab directory pointer is NULL.\n", __PROGRAM_NAME); - return 1; - } - - /* Check symlink on client_keytab_dirname before stat */ - if (lstat(client_keytab_dirname, &lst) != 0) { - (void)fprintf(stderr, "%s: Client keytab directory does not exist: %s.\n", __PROGRAM_NAME, client_keytab_dirname); - (void)fprintf(stderr, "%s: Contact your admin to have it created correctly.\n", __PROGRAM_NAME); - return 1; - } - if (S_ISLNK(lst.st_mode)) { - (void)fprintf(stderr, "%s: Client keytab directory %s is a symlink, not allowed.\n", __PROGRAM_NAME, client_keytab_dirname); - return 1; - } - - if (stat(client_keytab_dirname, &st) == -1) { - (void)fprintf(stderr, "%s: Client keytab directory does not exist: %s.\n", __PROGRAM_NAME, client_keytab_dirname); - (void)fprintf(stderr, "%s: Contact your admin to have it created.\n", __PROGRAM_NAME); - return 1; - } - return 0; -} - void constructor(void) __attribute__((constructor)); void constructor(void) { /* Setup runtime hardening /before/ main() is even called */ diff --git a/src/C/kcron_keytab_file.h b/src/C/kcron_keytab_file.h new file mode 100644 index 0000000..b09bb98 --- /dev/null +++ b/src/C/kcron_keytab_file.h @@ -0,0 +1,247 @@ +/* SPDX-License-Identifier: MIT */ +/* + * + * On-disk validation and creation helpers for per-user client keytab files. + * + * These are the testable core of the create-empty-client-keytab and + * init-kerberos-client-dir programs: they operate on caller-supplied paths, + * return an integer status, and require no special privileges, so they can be + * exercised directly in unit tests the same way kcron_filename.h and + * kcron_empty_keytab_file.h logic is. The programs' main() functions stay thin + * wrappers that call these helpers and translate the result into an exit code. + * + */ +#include "autoconf.h" /* for our automatic config bits */ + +#ifndef KCRON_KEYTAB_FILE_H +#define KCRON_KEYTAB_FILE_H 1 + +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include "kcron_empty_keytab_file.h" + +#ifndef _0600 +#define _0600 (S_IRUSR | S_IWUSR) +#endif + +/* + * Validate the client keytab base directory. + * + * The directory must exist, be reachable, and not be a symlink (symlink + * indirection would let a caller substitute an unrelated path). This is the + * check the init-kerberos-client-dir program runs before creating anything. + * + * Parameters: + * client_keytab_dirname: Path of the base directory to validate. + * + * Returns: 0 if the path is an existing, non-symlink directory; 1 otherwise. + * + * Security Note: lstat() is used first (not stat()) so the check cannot be + * defeated by a TOCTOU symlink race. + */ +int validate_client_dirname(const char *client_keytab_dirname) __attribute__((warn_unused_result)); +int validate_client_dirname(const char *client_keytab_dirname) { + struct stat lst = {0}; + struct stat st = {0}; + + if (client_keytab_dirname == NULL) { + (void)fprintf(stderr, "%s: Client keytab directory pointer is NULL.\n", __PROGRAM_NAME); + return 1; + } + + /* Check symlink on client_keytab_dirname before stat */ + if (lstat(client_keytab_dirname, &lst) != 0) { + (void)fprintf(stderr, "%s: Client keytab directory does not exist: %s.\n", __PROGRAM_NAME, client_keytab_dirname); + (void)fprintf(stderr, "%s: Contact your admin to have it created correctly.\n", __PROGRAM_NAME); + return 1; + } + if (S_ISLNK(lst.st_mode)) { + (void)fprintf(stderr, "%s: Client keytab directory %s is a symlink, not allowed.\n", __PROGRAM_NAME, client_keytab_dirname); + return 1; + } + + if (stat(client_keytab_dirname, &st) == -1) { + (void)fprintf(stderr, "%s: Client keytab directory does not exist: %s.\n", __PROGRAM_NAME, client_keytab_dirname); + (void)fprintf(stderr, "%s: Contact your admin to have it created.\n", __PROGRAM_NAME); + return 1; + } + return 0; +} + +/* + * Validate the per-user keytab directory. + * + * The create-empty-client-keytab program writes into /, so the + * per-UID directory must already exist (created by init-kerberos-client-dir) + * and must not be a symlink. + * + * Parameters: + * keytab_dirname: Path of the per-user directory to validate. + * + * Returns: 0 if the path is an existing, non-symlink directory; 1 otherwise. + * + * Security Note: lstat() is used first (not stat()) to avoid a TOCTOU symlink + * race that could substitute an attacker-controlled path. + */ +int validate_keytab_directory(const char *keytab_dirname) __attribute__((access(read_only, 1))) __attribute__((warn_unused_result)); +int validate_keytab_directory(const char *keytab_dirname) { + struct stat lst = {0}; + struct stat st = {0}; + + if (keytab_dirname == NULL) { + (void)fprintf(stderr, "%s: Keytab directory pointer is NULL.\n", __PROGRAM_NAME); + return 1; + } + + /* Check symlink on keytab_dirname before stat to prevent symlink attacks */ + if (lstat(keytab_dirname, &lst) != 0) { + (void)fprintf(stderr, "%s: Keytab directory does not exist: %s.\n", __PROGRAM_NAME, keytab_dirname); + (void)fprintf(stderr, "%s: Run init-kerberos-client-dir first.\n", __PROGRAM_NAME); + return 1; + } + if (S_ISLNK(lst.st_mode)) { + (void)fprintf(stderr, "%s: Keytab directory %s is a symlink, not allowed.\n", __PROGRAM_NAME, keytab_dirname); + return 1; + } + + if (stat(keytab_dirname, &st) == -1) { + (void)fprintf(stderr, "%s: Keytab directory does not exist: %s.\n", __PROGRAM_NAME, keytab_dirname); + (void)fprintf(stderr, "%s: Run init-kerberos-client-dir first.\n", __PROGRAM_NAME); + return 1; + } + + if (!S_ISDIR(st.st_mode)) { + (void)fprintf(stderr, "%s: Keytab directory %s is not a directory.\n", __PROGRAM_NAME, keytab_dirname); + return 1; + } + + return 0; +} + +/* + * Create an empty client keytab file. + * + * The file is created atomically with O_CREAT|O_EXCL so a pre-existing entry + * is reported rather than clobbered, is opened with O_NOFOLLOW to reject + * symlink races, is pinned to mode 0600, and is given the valid empty keytab + * header by write_empty_keytab(). All operations are performed relative to the + * already-open directory inode to avoid a TOCTOU window between the directory + * check and the create. + * + * Parameters: + * keytab_dirname: Existing, non-symlink directory to create the file in. + * keytab_filename: Base name of the keytab file to create. + * keytab: Full path, used only for error messages. + * + * Returns: 0 on success; 1 on any failure. + * + * Security Note: Every failure path closes the directory handle and returns + * non-zero so the caller knows the keytab was not (re)created. + */ +int create_keytab_file(const char *keytab_dirname, const char *keytab_filename, const char *keytab) __attribute__((warn_unused_result)); +int create_keytab_file(const char *keytab_dirname, const char *keytab_filename, const char *keytab) { + struct stat st = {0}; + struct stat lst = {0}; + DIR *keytab_dir = NULL; + int filedescriptor = -1; + + /* Validate non-null input pointers */ + if (keytab_dirname == NULL) { + (void)fprintf(stderr, "%s: keytab_dirname pointer is NULL.\n", __PROGRAM_NAME); + return 1; + } + if (keytab_filename == NULL) { + (void)fprintf(stderr, "%s: keytab_filename pointer is NULL.\n", __PROGRAM_NAME); + return 1; + } + if (keytab == NULL) { + (void)fprintf(stderr, "%s: keytab pointer is NULL.\n", __PROGRAM_NAME); + return 1; + } + + /* Check if keytab_dirname exists and is not a symlink (to avoid TOCTOU symlink attacks) */ + if (lstat(keytab_dirname, &lst) != 0) { + (void)fprintf(stderr, "%s: %s does not exist.\n", __PROGRAM_NAME, keytab_dirname); + return 1; + } + if (S_ISLNK(lst.st_mode)) { + (void)fprintf(stderr, "%s: %s is a symlink, not allowed.\n", __PROGRAM_NAME, keytab_dirname); + return 1; + } + + /* Open the directory handle to perform file operations safely on the directory inode */ + keytab_dir = opendir(keytab_dirname); + if (keytab_dir == NULL) { + (void)fprintf(stderr, "%s: Unable to open %s.\n", __PROGRAM_NAME, keytab_dirname); + (void)fprintf(stderr, "%s: This may be a permissions error?\n", __PROGRAM_NAME); + return 1; + } + + /* Verify the opened directory is indeed a directory */ + if (fstat(dirfd(keytab_dir), &st) != 0) { + (void)fprintf(stderr, "%s: %s could not be read.\n", __PROGRAM_NAME, keytab_dirname); + (void)closedir(keytab_dir); + return 1; + } + + if (!S_ISDIR(st.st_mode)) { + (void)fprintf(stderr, "%s: %s is not a directory.\n", __PROGRAM_NAME, keytab_dirname); + (void)closedir(keytab_dir); + return 1; + } + + /* Open the keytab file with O_NOFOLLOW to prevent symlink attacks, + * O_CREAT | O_EXCL to create only if missing and atomically detect existence, + * and O_CLOEXEC for descriptor safety. Permissions set to 0600 for security. + */ + filedescriptor = openat(dirfd(keytab_dir), keytab_filename, O_WRONLY | O_CREAT | O_EXCL | O_NOFOLLOW | O_CLOEXEC, _0600); + if (filedescriptor < 0) { + if (errno == EEXIST) { + (void)fprintf(stderr, "%s: %s already exists.\n", __PROGRAM_NAME, keytab); + } else { + (void)fprintf(stderr, "%s: %s cannot be created: %s\n", __PROGRAM_NAME, keytab, strerror(errno)); + } + (void)closedir(keytab_dir); + return 1; + } + (void)closedir(keytab_dir); + + /* Verify that the created file is a regular file */ + if (fstat(filedescriptor, &st) != 0) { + (void)close(filedescriptor); + (void)fprintf(stderr, "%s: %s could not be created.\n", __PROGRAM_NAME, keytab); + return 1; + } + if (!S_ISREG(st.st_mode)) { + (void)close(filedescriptor); + (void)fprintf(stderr, "%s: %s is not a regular file.\n", __PROGRAM_NAME, keytab); + return 1; + } + + /* Write empty keytab content to the file */ + if (write_empty_keytab(filedescriptor) != 0) { + (void)close(filedescriptor); + (void)fprintf(stderr, "%s: Cannot create keytab : %s.\n", __PROGRAM_NAME, keytab); + return 1; + } + + /* Ensure permissions are exactly 0600 */ + if (fchmod(filedescriptor, _0600) != 0) { + (void)close(filedescriptor); + (void)fprintf(stderr, "%s: Unable to chmod %o %s\n", __PROGRAM_NAME, _0600, keytab); + return 1; + } + + (void)close(filedescriptor); + return 0; +} + +#endif /* KCRON_KEYTAB_FILE_H */ diff --git a/src/shell/CMakeLists.txt b/src/shell/CMakeLists.txt index f18b09e..ed515f2 100644 --- a/src/shell/CMakeLists.txt +++ b/src/shell/CMakeLists.txt @@ -6,8 +6,9 @@ include(GNUInstallDirs) install(FILES ${PROJECT_SOURCE_DIR}/src/shell/kcron.sysconfig DESTINATION ${CMAKE_INSTALL_FULL_SYSCONFDIR}/sysconfig RENAME kcron) install(FILES ${PROJECT_SOURCE_DIR}/src/shell/kcrondestroy ${PROJECT_SOURCE_DIR}/src/shell/kcroninit DESTINATION ${CMAKE_INSTALL_BINDIR}) -enable_testing() - -add_test(NAME Syntax:Config COMMAND bash -n ${PROJECT_SOURCE_DIR}/src/shell/kcron.sysconfig) -add_test(NAME Syntax:Init COMMAND bash -n ${PROJECT_SOURCE_DIR}/src/shell/kcroninit) -add_test(NAME Syntax:Destroy COMMAND bash -n ${PROJECT_SOURCE_DIR}/src/shell/kcrondestroy) +# +# All CTest registrations for the shell utilities (syntax checks, sourcing, +# hardening, shellcheck) live in the test tree: ${PROJECT_SOURCE_DIR}/test/. +# This file therefore owns only build and install directives, matching the +# separation used across the rest of the project. +# diff --git a/src/shell/kcron.sysconfig b/src/shell/kcron.sysconfig index 732ab08..c9aa534 100644 --- a/src/shell/kcron.sysconfig +++ b/src/shell/kcron.sysconfig @@ -1,16 +1,20 @@ -# SPDX-License-Identifier: MIT ########################################################### # Default settings for kcron ########################################################### +# shellcheck shell=bash KRB5_CONFIG=${KRB5_CONFIG:-'/etc/krb5.conf'} KRB5_CONF_D=${KRB5_CONF_D:-'/etc/krb5.conf.d/*'} +# shellcheck disable=SC2086 +# ${KRB5_CONF_D} is intentionally a glob; quoting would suppress the expansion. DEFAULT_REALM=$(grep default_realm ${KRB5_CONFIG} ${KRB5_CONF_D} 2>/dev/null | grep -v \# | cut -d '=' -f2 | tail -1 | tr -d ' ') REALM=${KCRON_REALM:-${DEFAULT_REALM}} WHOAMI=${KCRON_WHOAMI:-$(basename "$(whoami)")} NODENAME=${KCRON_NODENAME:-$(basename "$(hostname)")} +# shellcheck disable=SC2034 +# FULLPRINCIPAL is set for the sourcing shell (see kcroninit/kcrondestroy). FULLPRINCIPAL=${KCRON_FULLPRINCIPAL:-"${WHOAMI}/cron/${NODENAME}@${REALM}"} #KEYTAB_NAME_UTIL='/usr/libexec/kcron/client-keytab-name' diff --git a/src/shell/kcrondestroy b/src/shell/kcrondestroy index 126cd23..c81b8ae 100644 --- a/src/shell/kcrondestroy +++ b/src/shell/kcrondestroy @@ -1,6 +1,10 @@ #!/bin/bash -u # SPDX-License-Identifier: MIT +# shellcheck source=/dev/null disable=SC1090 +# The source lines below load site/host configuration supplied by the +# administrator; ShellCheck cannot follow those non-constant paths. + ########################################################### if [[ -r /etc/sysconfig/kcron ]]; then source /etc/sysconfig/kcron diff --git a/src/shell/kcroninit b/src/shell/kcroninit index 8af809d..f3198b9 100644 --- a/src/shell/kcroninit +++ b/src/shell/kcroninit @@ -1,6 +1,10 @@ #!/bin/bash -u # SPDX-License-Identifier: MIT +# shellcheck source=/dev/null disable=SC1090 +# The source lines below load site/host configuration supplied by the +# administrator; ShellCheck cannot follow those non-constant paths. + ########################################################### if [[ -r /etc/sysconfig/kcron ]]; then source /etc/sysconfig/kcron @@ -191,7 +195,7 @@ if [[ ! -w ${KEYTAB} ]]; then echo '' echo 'Keytab is not writable to this user:' >&2 id >&2 - ls -l ${KEYTAB} >&2 + ls -l "${KEYTAB}" >&2 exit 2 fi diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt new file mode 100644 index 0000000..21b2167 --- /dev/null +++ b/test/CMakeLists.txt @@ -0,0 +1,279 @@ +# SPDX-License-Identifier: MIT +# SPDX-FileCopyrightText: 2026 Fermi Forward Discovery Group +# test/CMakeLists.txt - Unit test executables for kcron +# +# Design rationale +# ---------------- +# Every test executable compiles the production headers directly rather than +# linking a shared OBJECT library. This ensures coverage instrumentation is +# applied to every translation unit under test; objects compiled without +# --coverage produce no .gcno data and silently drop out of reports. +# +# The production path/keytab logic lives in headers (kcron_filename.h, +# kcron_empty_keytab_file.h), so the tests include those headers directly. +# Each binary is also kept self-contained so coverage data is always emitted. +include(CTest) + +# ============================================================================ +# Shared include paths +# ============================================================================ +set(TEST_INCLUDES "${CMAKE_SOURCE_DIR}/src/C" + "${CMAKE_BINARY_DIR}/src/C" "${CMAKE_CURRENT_SOURCE_DIR}") + +# ============================================================================ +# Coverage detection +# ============================================================================ +set(HAVE_COVERAGE_TOOLS OFF) + +if(CMAKE_C_COMPILER_ID STREQUAL "GNU") + + find_program(LCOV_EXEC lcov) + find_program(GENHTML_EXEC genhtml) + + if(LCOV_EXEC AND GENHTML_EXEC) + set(HAVE_COVERAGE_TOOLS ON) + + set(COVERAGE_COMPILE_FLAGS -g --coverage -fdebug-prefix-map=${CMAKE_BINARY_DIR}=. + -fdebug-prefix-map=${CMAKE_SOURCE_DIR}=.) + + set(COVERAGE_LINK_FLAGS --coverage) + + message(STATUS "Coverage enabled: GCC + lcov") + endif() + +elseif(CMAKE_C_COMPILER_ID MATCHES "Clang") + + find_program(LLVM_PROFDATA_EXEC llvm-profdata) + find_program(LLVM_COV_EXEC llvm-cov) + + if(LLVM_PROFDATA_EXEC AND LLVM_COV_EXEC AND GENHTML_EXEC) + set(HAVE_COVERAGE_TOOLS ON) + + set(COVERAGE_COMPILE_FLAGS -g -fprofile-instr-generate -fcoverage-mapping + -fdebug-prefix-map=${CMAKE_BINARY_DIR}=. + -fdebug-prefix-map=${CMAKE_SOURCE_DIR}=.) + + set(COVERAGE_LINK_FLAGS -fprofile-instr-generate) + + message(STATUS "Coverage enabled: Clang + llvm-cov") + endif() + +endif() + +# Internal helper: apply coverage flags (and disable LTO, which would otherwise +# strip the instrumentation) to a target when tools are present. +macro(add_coverage_flags target) + if(HAVE_COVERAGE_TOOLS) + target_compile_options(${target} PRIVATE ${COVERAGE_COMPILE_FLAGS} -fno-lto) + target_link_options(${target} PRIVATE ${COVERAGE_LINK_FLAGS} -fno-lto) + endif() +endmacro() + +# ============================================================================ +# test_kcron_filename +# ============================================================================ +# Tests get_client_dirname() and get_filenames() from kcron_filename.h. + +add_executable(test_kcron_filename test_kcron_filename.c) + +target_include_directories(test_kcron_filename PRIVATE ${TEST_INCLUDES}) +add_coverage_flags(test_kcron_filename) + +add_test(NAME test_kcron_filename COMMAND test_kcron_filename) + +# ============================================================================ +# test_kcron_keytab +# ============================================================================ +# Tests write_empty_keytab() from kcron_empty_keytab_file.h. + +add_executable(test_kcron_keytab test_kcron_keytab.c) + +target_include_directories(test_kcron_keytab PRIVATE ${TEST_INCLUDES}) +add_coverage_flags(test_kcron_keytab) + +add_test(NAME test_kcron_keytab COMMAND test_kcron_keytab) + +# ============================================================================ +# test_client_keytab_name +# ============================================================================ +# Integration test: run the built client-keytab-name binary and verify its +# stdout matches the path derived from the calling UID. + +add_executable(test_client_keytab_name test_client_keytab_name.c) + +target_include_directories(test_client_keytab_name PRIVATE ${TEST_INCLUDES}) +target_compile_definitions(test_client_keytab_name PRIVATE + "CLIENT_KEYTAB_NAME_BIN=\"$\"") +add_coverage_flags(test_client_keytab_name) + +add_test(NAME test_client_keytab_name COMMAND test_client_keytab_name) + +# Instrument the client-keytab-name binary itself so its main() and the path +# helpers it calls are captured by the coverage report when the integration +# test executes it. Gated on the coverage toolchain just like the test binaries. +if(HAVE_COVERAGE_TOOLS) + add_coverage_flags(client-keytab-name) +endif(HAVE_COVERAGE_TOOLS) + + +# ============================================================================ +# test_kcron_caps +# ============================================================================ +# Tests the libcap(3) helpers from kcron_caps.h: disable_capabilities(), +# print_cap_error() and enable_capabilities(). The privilege-free helpers are +# called directly; enable_capabilities() is checked via a grandchild so its +# documented exit-on-failure contract is verified without failing the test. + +add_executable(test_kcron_caps test_kcron_caps.c) + +target_include_directories(test_kcron_caps PRIVATE ${TEST_INCLUDES}) +add_coverage_flags(test_kcron_caps) +target_link_libraries(test_kcron_caps PRIVATE cap) + +add_test(NAME test_kcron_caps COMMAND test_kcron_caps) + +# ============================================================================ +# test_kcron_keytab_file +# ============================================================================ +# Tests the on-disk validation and creation helpers from kcron_keytab_file.h: +# validate_client_dirname(), validate_keytab_directory() and create_keytab_file(). +# These are pure, privilege-free, and return a status code, so the tests run +# them directly in a private temporary directory. create_keytab_file() writes +# the real empty-keytab header, so the success path is verified by reading the +# bytes back and checking the resulting file mode. + +add_executable(test_kcron_keytab_file test_kcron_keytab_file.c) + +target_include_directories(test_kcron_keytab_file PRIVATE ${TEST_INCLUDES}) +add_coverage_flags(test_kcron_keytab_file) + +add_test(NAME test_kcron_keytab_file COMMAND test_kcron_keytab_file) + +# ============================================================================ +# Shell tests (bats) +# ============================================================================ +# bats is the shell test framework for the kcron utilities. It is optional at +# configure time so a minimal toolchain without bats still configures and runs +# the C tests; when present it is added like any other test so it runs under +# "make test"/"ctest". +find_program(BATS_EXEC bats) +if(BATS_EXEC) + + add_test( + NAME Shell:bats + COMMAND ${BATS_EXEC} ${CMAKE_CURRENT_SOURCE_DIR}/kcron.bats) + +endif() + +# ============================================================================ +# Shell source tests +# ============================================================================ +# The kcron shell utilities live under src/shell; their tests are registered +# here so that every CTest test (C, bats, shell) is registered from the test +# tree. Source scripts referenced by name stay where the source owns them. + +add_test(NAME Syntax:Config COMMAND bash -n ${PROJECT_SOURCE_DIR}/src/shell/kcron.sysconfig) +add_test(NAME Syntax:Init COMMAND bash -n ${PROJECT_SOURCE_DIR}/src/shell/kcroninit) +add_test(NAME Syntax:Destroy COMMAND bash -n ${PROJECT_SOURCE_DIR}/src/shell/kcrondestroy) + +add_test( + NAME Sysconfig:Sourcing + COMMAND bash ${PROJECT_SOURCE_DIR}/test/test_sysconfig.sh + ${PROJECT_SOURCE_DIR}/src/shell/kcron.sysconfig) + +# Hardening check (ported from the RPM spec %check section) +# Determine which hardening auditor is available; prefer checksec, fall back to +# hardening-check. When neither is installed the check is a no-op so that +# minimal toolchains do not fail the build. +find_program(CHECKSEC_EXEC checksec) +find_program(HARDENING_CHECK_EXEC hardening-check) + +if(CHECKSEC_EXEC OR HARDENING_CHECK_EXEC) + add_test( + NAME Hardening:Checksec + COMMAND bash ${PROJECT_SOURCE_DIR}/test/hardening.sh + $ + $ + $) +endif() + +# Static analysis of shell scripts (ported from issue #9) +find_program(SHELLCHECK_EXEC shellcheck) +if(SHELLCHECK_EXEC) + add_test( + NAME Shell:Shellcheck + COMMAND ${SHELLCHECK_EXEC} -x + ${PROJECT_SOURCE_DIR}/src/shell/kcroninit + ${PROJECT_SOURCE_DIR}/src/shell/kcrondestroy + ${PROJECT_SOURCE_DIR}/src/shell/kcron.sysconfig) +endif() + +# ============================================================================ +# Coverage report target +# ============================================================================ +set(UNIT_TEST_BINARIES $ + $ + $ + $ + $) + +add_custom_target(test_binaries DEPENDS test_kcron_filename test_kcron_keytab + test_kcron_caps + test_kcron_keytab_file + test_client_keytab_name + COMMENT "Building every unit test binary") + +if(BUILD_TESTING AND HAVE_COVERAGE_TOOLS) + + set(COVERAGE_OUTPUT_DIR ${CMAKE_BINARY_DIR}/coverage) + set_property(DIRECTORY APPEND PROPERTY ADDITIONAL_CLEAN_FILES ${COVERAGE_OUTPUT_DIR}) + + if(CMAKE_C_COMPILER_ID STREQUAL "GNU") + + set(COVERAGE_INFO ${CMAKE_BINARY_DIR}/coverage.info) + set(COVERAGE_INFO_CLEAN ${CMAKE_BINARY_DIR}/coverage.info.cleaned) + set_property(DIRECTORY APPEND PROPERTY ADDITIONAL_CLEAN_FILES ${COVERAGE_INFO} + ${COVERAGE_INFO_CLEAN}) + + add_custom_target( + coverage + COMMAND ${LCOV_EXEC} --directory ${CMAKE_BINARY_DIR} --zerocounters + COMMAND ${CMAKE_CTEST_COMMAND} --output-on-failure + COMMAND ${LCOV_EXEC} --directory ${CMAKE_BINARY_DIR} --capture --rc + branch_coverage=1 --output-file ${COVERAGE_INFO} + COMMAND + ${LCOV_EXEC} --remove ${COVERAGE_INFO} '${CMAKE_SOURCE_DIR}/test/*' --rc + branch_coverage=1 --output-file ${COVERAGE_INFO_CLEAN} + COMMAND ${GENHTML_EXEC} --rc branch_coverage=1 -o ${COVERAGE_OUTPUT_DIR} + ${COVERAGE_INFO_CLEAN} + COMMAND ${LCOV_EXEC} --summary ${COVERAGE_INFO_CLEAN} --rc + branch_coverage=1 + WORKING_DIRECTORY ${CMAKE_BINARY_DIR} + DEPENDS test_binaries + COMMENT + "Running tests and generating GCC coverage report in ${COVERAGE_OUTPUT_DIR}/index.html" + ) + + elseif(CMAKE_C_COMPILER_ID MATCHES "Clang") + + set(PROFILE_DATA ${CMAKE_BINARY_DIR}/default.profdata) + set_property(DIRECTORY APPEND PROPERTY ADDITIONAL_CLEAN_FILES ${PROFILE_DATA}) + + add_custom_target( + coverage + COMMAND ${CMAKE_COMMAND} -E env LLVM_PROFILE_FILE=default.profraw + ${CMAKE_CTEST_COMMAND} --output-on-failure + COMMAND ${LLVM_PROFDATA_EXEC} merge -sparse default.profraw -o ${PROFILE_DATA} + COMMAND ${LLVM_COV_EXEC} show ${UNIT_TEST_BINARIES} -instr-profile=${PROFILE_DATA} + -format=html -output-dir=${COVERAGE_OUTPUT_DIR} + COMMAND ${LLVM_COV_EXEC} report ${UNIT_TEST_BINARIES} + -instr-profile=${PROFILE_DATA} -show-region-summary=false + WORKING_DIRECTORY ${CMAKE_BINARY_DIR} + DEPENDS test_binaries + COMMENT + "Running tests and generating Clang coverage report in ${COVERAGE_OUTPUT_DIR}/index.html" + ) + + endif() + +endif() diff --git a/test/hardening.sh b/test/hardening.sh new file mode 100755 index 0000000..6be205a --- /dev/null +++ b/test/hardening.sh @@ -0,0 +1,57 @@ +#!/bin/bash +# SPDX-License-Identifier: MIT +# SPDX-FileCopyrightText: 2026 Fermi Forward Discovery Group +# +# hardening.sh - Verify build hardening of kcron binaries. +# +# It is driven by CTest instead of rpmbuild. It prefers checksec and falls back +# to hardening-check when checksec is unavailable. When neither tool is +# installed the test is a no-op so minimal toolchains do not break. + +set -u + +if [[ $# -eq 0 ]]; then + echo "no binaries provided" + exit 1 +fi + +if command -v checksec >/dev/null 2>&1; then + TOOL=checksec +elif command -v hardening-check >/dev/null 2>&1; then + TOOL=hardening-check +else + echo "SKIP: neither checksec nor hardening-check is installed" + exit 0 +fi + +FAILED=0 +for binary in "$@"; do + if [[ ! -f "$binary" ]]; then + echo "SKIP: binary not found: $binary" + continue + fi + + echo "==> $TOOL on $(basename "$binary")" + if [[ "$TOOL" == "checksec" ]]; then + if ! checksec --file="$binary"; then + echo "checksec --file reported issues for $binary" + FAILED=1 + fi + if ! checksec --fortify-file="$binary"; then + echo "checksec --fortify-file reported issues for $binary" + FAILED=1 + fi + else + if ! hardening-check "$binary"; then + echo "hardening-check reported issues for $binary" + FAILED=1 + fi + fi +done + +if [[ "$FAILED" -ne 0 ]]; then + echo "hardening checks failed" + exit 1 +fi +echo "hardening checks passed" +exit 0 diff --git a/test/kcron.bats b/test/kcron.bats new file mode 100644 index 0000000..7908ed5 --- /dev/null +++ b/test/kcron.bats @@ -0,0 +1,247 @@ +#!/usr/bin/env bash +# SPDX-License-Identifier: MIT +# SPDX-FileCopyrightText: 2026 Fermi Forward Discovery Group +# +# cron.bats - shell tests for the kcron utilities. +# +# These tests exercise everything that can be tested hermetically: +# * kcron.sysconfig variable derivation (offline, deterministic) +# * the control flow of kcroninit / kcrondestroy when all external commands +# (kadmin, kinit, ...) are stubbed, so no Kerberos tooling or KDC is needed. +# +# End-to-end keytab creation requires a live KDC and is intentionally out of +# scope for automated tests. +# +# This suite deliberately ships its own tiny assertion helpers rather than +# pulling in bats-assert. The distro bats-core installed here does not provide +# the `batslib_*` / `fail` helpers that bats-assert depends on, so the upstream +# library cannot load. A handful of self-contained helpers keeps the suite +# dependency-free, deterministic, and easy to audit. + +# shellcheck shell=bats + +# Resolve paths relative to this file so the suite works from any CWD. +: "${BATS_TEST_DIRNAME:=$PWD}" +KCRON_SRC_DIR="$BATS_TEST_DIRNAME/../src/shell" +SYSKCONFIG="$KCRON_SRC_DIR/kcron.sysconfig" +KCRONINIT="$KCRON_SRC_DIR/kcroninit" +KCRONDESTROY="$KCRON_SRC_DIR/kcrondestroy" + +# A fixed identity keeps derivations deterministic regardless of the caller. +readonly REALM=EXAMPLE.ORG +readonly WHOAMI=alice +readonly NODENAME=node01 + +# --------------------------------------------------------------------------- +# Minimal assertion helpers (assert_success, assert_failure, assert_output, +# refute_output, assert_equal). Each returns non-zero on failure, which is how +# bats marks a test "not ok"; assert functions also print a diagnostic. +# --------------------------------------------------------------------------- + +__assert_fail() { + # $1 = human-readable reason + printf -- '-- assertion failed: %s --\n' "$1" >&2 + printf '%s\n' "$output" >&2 + return 1 +} + +assert_success() { + if [[ $status -ne 0 ]]; then + __assert_fail "expected status 0, got $status" + fi +} + +assert_failure() { + if [[ $status -eq 0 ]]; then + __assert_fail "expected non-zero status, got 0" + fi +} + +# assert_output [--partial] +assert_output() { + local partial=0 expected="" + while [[ ${1:-} == --* ]]; do + case "$1" in + --partial) partial=1 ;; + --) shift; break ;; + *) shift ;; + esac + shift + done + expected="${1:-}" + if [[ $partial -eq 1 ]]; then + [[ "$output" == *"$expected"* ]] \ + || __assert_fail "output did not contain: $expected" + else + [[ "$output" == "$expected" ]] \ + || __assert_fail "output did not equal: $expected" + fi +} + +# refute_output [--partial] +refute_output() { + local partial=0 unexpected="" + while [[ ${1:-} == --* ]]; do + case "$1" in + --partial) partial=1 ;; + --) shift; break ;; + *) shift ;; + esac + shift + done + unexpected="${1:-}" + if [[ $partial -eq 1 ]]; then + [[ "$output" != *"$unexpected"* ]] \ + || __assert_fail "output unexpectedly contained: $unexpected" + else + [[ "$output" != "$unexpected" ]] \ + || __assert_fail "output unexpectedly equal: $unexpected" + fi +} + +# assert_equal [actual] [expected] +assert_equal() { + if [[ "$2" != "$1" ]]; then + __assert_fail "expected '$2' but got '$1'" + fi +} + +setup() { + # Build a hermetic stub for every external the scripts may call, then run + # them with that stub directory first on PATH. This makes the control flow + # deterministic and removes the Kerberos dependency from the test. + STUBDIR="$(mktemp -d)" + local tool + for tool in kadmin kinit klist kdestroy logger md5sum id date \ + basename hostname cut whoami grep tr tail cat printf; do + printf '#!/bin/bash\nexit 0\n' >"$STUBDIR/$tool" + chmod +x "$STUBDIR/$tool" + done + + # The kcron shell utilities invoke the kcron-internal helpers through + # env-overridable absolute-path variables. Point those at tiny stubs so the + # real (seccomp/landlock-restricted) /usr/libexec binaries never run. + printf '#!/bin/bash\nexit 0\n' >"$STUBDIR/kdir" + printf '#!/bin/bash\nexit 0\n' >"$STUBDIR/kcreate" + # shellcheck disable=SC2016 + printf '#!/bin/bash\necho "${STUBDIR}/client.keytab"\nexit 0\n' >"$STUBDIR/kname" + for tool in kdir kcreate kname; do + chmod +x "$STUBDIR/$tool" + done + touch "$STUBDIR/client.keytab" + + export STUBDIR + STUBPATH="$STUBDIR:/usr/bin:/bin" + export STUBPATH + export KEYTAB_DIR_INIT="$STUBDIR/kdir" + export KEYTAB_NAME="$STUBDIR/kname" + export KEYTAB_CREATE="$STUBDIR/kcreate" + export KEYTAB_NAME_UTIL="$STUBDIR/kname" +} + +teardown() { + rm -rf "$STUBDIR" +} + +# sysconfig_get [ENV=value ...] +# Source the sysconfig under the given environment and print one variable. +sysconfig_get() { + local var="$1" + shift + env "$@" bash -c "source '$SYSKCONFIG'; printf '%s' \"\${$var}\"" +} + +# run_cli