Start work on self tests

.codeql-config.yml

@@ -0,0 +1,4 @@
+name: "CodeQL config"
+
+paths-ignore:
+  - test/**

.github/workflows/ci.yaml

@@ -1,4 +1,4 @@
-name: Run CI
+name: Build and Test
 permissions:
   contents: read
   pull-requests: read
@@ -13,20 +13,15 @@ on:
       - main
 
 jobs:
-  compile:
+  build-test:
     runs-on: ubuntu-latest
     container:
       image: fedora:latest
 
     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

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

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

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.

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

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));

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 */