d227e41497
* Drop dependency on libpath_utils * Minor fixes and code cleanups Related: RHEL-123675 Signed-off-by: Stephen Gallagher <sgallagh@redhat.com>
362 lines
11 KiB
Diff
362 lines
11 KiB
Diff
From ad7ec4c30d8b0911574131157c7c7aeee9b4c570 Mon Sep 17 00:00:00 2001
|
|
From: Stephen Gallagher <sgallagh@redhat.com>
|
|
Date: Mon, 1 Dec 2025 11:31:43 -0500
|
|
Subject: [PATCH 2/2] Handle opening the same file
|
|
|
|
We introduced a regression in 4.0.1 when closing the TOCTOU issue. We
|
|
incorrectly refuse to open the same file twice to save both a CA and
|
|
cert into the same location.
|
|
|
|
This changes the opening logic to always open for both reading and
|
|
writing to check if it is zero-length.
|
|
|
|
Also switches the EC curve tests to output the CA and service cert to a
|
|
common path to ensure we are testing this case properly.
|
|
|
|
Updates some incorrect expected behaviors in the test_dhparams_creation
|
|
test as well.
|
|
|
|
Signed-off-by: Stephen Gallagher <sgallagh@redhat.com>
|
|
---
|
|
src/io_utils.c | 85 +++++++++++++++++++++++++++++---
|
|
src/sscg.c | 44 ++++++++++++-----
|
|
test/test_dhparams_creation.sh | 37 +++++++-------
|
|
test/test_ecdsa_cert_validity.sh | 12 ++---
|
|
4 files changed, 135 insertions(+), 43 deletions(-)
|
|
|
|
diff --git a/src/io_utils.c b/src/io_utils.c
|
|
index 1cc855bb8f394fe6a11faf28eb127a54ab8df3f5..10c5f6818115a0c8219bca586e710ac9f3266bd5 100644
|
|
--- a/src/io_utils.c
|
|
+++ b/src/io_utils.c
|
|
@@ -37,6 +37,7 @@
|
|
#include <string.h>
|
|
#include <talloc.h>
|
|
#include <sys/stat.h>
|
|
+#include <unistd.h>
|
|
|
|
#include "config.h"
|
|
#ifdef HAVE_GETTEXT
|
|
@@ -102,17 +103,85 @@ static int
|
|
sscg_io_utils_open_file (const char *path, bool overwrite, FILE **fp)
|
|
{
|
|
FILE *_fp = NULL;
|
|
- if (overwrite)
|
|
- _fp = fopen (path, "w");
|
|
- else
|
|
- _fp = fopen (path, "wx");
|
|
+ struct stat st;
|
|
+ int ret;
|
|
+ int fd;
|
|
+
|
|
+ /* Try to open with r+ mode (file must exist) */
|
|
+ _fp = fopen (path, "r+");
|
|
if (!_fp)
|
|
{
|
|
- SSCG_ERROR ("Could not open file %s: %s\n", path, strerror (errno));
|
|
- return errno;
|
|
+ /* If file doesn't exist, create it with w+ mode */
|
|
+ if (errno == ENOENT)
|
|
+ {
|
|
+ _fp = fopen (path, "w+");
|
|
+ if (!_fp)
|
|
+ {
|
|
+ SSCG_ERROR (
|
|
+ "Could not create file %s: %s\n", path, strerror (errno));
|
|
+ return errno;
|
|
+ }
|
|
+ /* New file has size 0, so we can proceed */
|
|
+ ret = EOK;
|
|
+ goto done;
|
|
+ }
|
|
+ else
|
|
+ {
|
|
+ SSCG_ERROR ("Could not open file %s: %s\n", path, strerror (errno));
|
|
+ ret = errno;
|
|
+ goto done;
|
|
+ }
|
|
}
|
|
- *fp = _fp;
|
|
- return EOK;
|
|
+
|
|
+ /* File exists and was opened successfully, check its size */
|
|
+ fd = fileno (_fp);
|
|
+ ret = fstat (fd, &st);
|
|
+ if (ret != 0)
|
|
+ {
|
|
+ SSCG_ERROR ("Could not stat file %s: %s\n", path, strerror (errno));
|
|
+ ret = errno;
|
|
+ goto done;
|
|
+ }
|
|
+
|
|
+ /* Check if file size is greater than zero */
|
|
+ if (st.st_size > 0)
|
|
+ {
|
|
+ if (overwrite)
|
|
+ {
|
|
+ /* Truncate the file */
|
|
+ ret = ftruncate (fd, 0);
|
|
+ if (ret != 0)
|
|
+ {
|
|
+ SSCG_ERROR (
|
|
+ "Could not truncate file %s: %s\n", path, strerror (errno));
|
|
+ ret = errno;
|
|
+ goto done;
|
|
+ }
|
|
+ /* Rewind to beginning after truncation */
|
|
+ rewind (_fp);
|
|
+ }
|
|
+ else
|
|
+ {
|
|
+ /* File exists and overwrite is false - return error */
|
|
+ SSCG_ERROR ("File %s already exists\n", path);
|
|
+ ret = EEXIST;
|
|
+ goto done;
|
|
+ }
|
|
+ }
|
|
+
|
|
+ /* File size is zero or has been truncated */
|
|
+ ret = EOK;
|
|
+
|
|
+done:
|
|
+ if (ret == EOK)
|
|
+ {
|
|
+ *fp = _fp;
|
|
+ _fp = NULL;
|
|
+ }
|
|
+
|
|
+ if (_fp)
|
|
+ fclose (_fp);
|
|
+ return ret;
|
|
}
|
|
|
|
|
|
diff --git a/src/sscg.c b/src/sscg.c
|
|
index 760e9ed258f06c09a09b3e1cdf1d237f792f7e5f..7ce0a331e56d32c5ebf94e580f9bd9836a909028 100644
|
|
--- a/src/sscg.c
|
|
+++ b/src/sscg.c
|
|
@@ -75,10 +75,11 @@ main (int argc, const char **argv)
|
|
|
|
struct sscg_stream *stream = NULL;
|
|
|
|
- /* Always use umask 0577 for generating certificates and keys
|
|
- This means that it's opened as write-only by the effective
|
|
- user. */
|
|
- umask (0577);
|
|
+ /* Always use umask 0177 for generating certificates and keys
|
|
+ This means that it's opened as read/write only by the effective
|
|
+ user.
|
|
+ */
|
|
+ umask (0177);
|
|
|
|
#ifdef HAVE_GETTEXT
|
|
/* Initialize internationalization */
|
|
@@ -170,19 +171,38 @@ main (int argc, const char **argv)
|
|
if (options->dhparams_file)
|
|
{
|
|
dhparams_file = talloc_strdup (main_ctx, options->dhparams_file);
|
|
+ CHECK_MEM (dhparams_file);
|
|
+
|
|
+ ret = sscg_io_utils_add_output_file (options->streams,
|
|
+ SSCG_FILE_TYPE_DHPARAMS,
|
|
+ dhparams_file,
|
|
+ options->overwrite,
|
|
+ options->dhparams_mode);
|
|
+ CHECK_OK (ret);
|
|
}
|
|
else
|
|
{
|
|
dhparams_file = talloc_strdup (main_ctx, "./dhparams.pem");
|
|
- }
|
|
- CHECK_MEM (dhparams_file);
|
|
+ CHECK_MEM (dhparams_file);
|
|
|
|
- ret = sscg_io_utils_add_output_file (options->streams,
|
|
- SSCG_FILE_TYPE_DHPARAMS,
|
|
- dhparams_file,
|
|
- options->overwrite,
|
|
- options->dhparams_mode);
|
|
- CHECK_OK (ret);
|
|
+ ret = sscg_io_utils_add_output_file (options->streams,
|
|
+ SSCG_FILE_TYPE_DHPARAMS,
|
|
+ dhparams_file,
|
|
+ options->overwrite,
|
|
+ options->dhparams_mode);
|
|
+ if (ret == EACCES || ret == EEXIST)
|
|
+ {
|
|
+ SSCG_LOG (SSCG_VERBOSE,
|
|
+ "Could not open dhparams file %s: %s\n",
|
|
+ dhparams_file,
|
|
+ strerror (ret));
|
|
+ ret = EOK;
|
|
+ }
|
|
+ else
|
|
+ {
|
|
+ CHECK_OK (ret);
|
|
+ }
|
|
+ }
|
|
}
|
|
/* Validate and open the file paths */
|
|
ret = sscg_io_utils_open_BIOs (options->streams);
|
|
diff --git a/test/test_dhparams_creation.sh b/test/test_dhparams_creation.sh
|
|
index ca6fc77a268d2ef5d3d49309190ddd05a087f89e..49f2b08d23246c90663eb7d2e5078817eb42139b 100755
|
|
--- a/test/test_dhparams_creation.sh
|
|
+++ b/test/test_dhparams_creation.sh
|
|
@@ -50,6 +50,7 @@ set -e
|
|
DHPARAMS_TMPDIR=$(mktemp --directory --tmpdir=$GITHUB_WORKSPACE sscg_dhparams_test_XXXXXX)
|
|
WRITABLE_DIR="$DHPARAMS_TMPDIR/writable"
|
|
READONLY_DIR="$DHPARAMS_TMPDIR/readonly"
|
|
+READONLY_WITH_DHPARAMS_DIR="$DHPARAMS_TMPDIR/readonly_with_dhparams"
|
|
DHPARAMS_DIR="$DHPARAMS_TMPDIR/dhparams"
|
|
|
|
function cleanup {
|
|
@@ -65,14 +66,16 @@ trap cleanup EXIT
|
|
# Set up test directories
|
|
mkdir -p "$WRITABLE_DIR"
|
|
mkdir -p "$READONLY_DIR"
|
|
+mkdir -p "$READONLY_WITH_DHPARAMS_DIR"
|
|
mkdir -p "$DHPARAMS_DIR"
|
|
|
|
# Create a pre-existing dhparams.pem file for some tests
|
|
-touch "$DHPARAMS_DIR/dhparams.pem"
|
|
+echo "preexisting dhparams.pem" > "$DHPARAMS_DIR/dhparams.pem"
|
|
|
|
# Copy pre-existing dhparams.pem to readonly directory before making it readonly
|
|
-cp "$DHPARAMS_DIR/dhparams.pem" "$READONLY_DIR/dhparams.pem"
|
|
-chmod 555 "$READONLY_DIR"
|
|
+cp "$DHPARAMS_DIR/dhparams.pem" "$READONLY_WITH_DHPARAMS_DIR/dhparams.pem"
|
|
+chmod 0555 "$READONLY_DIR"
|
|
+chmod 0555 "$READONLY_WITH_DHPARAMS_DIR"
|
|
|
|
failed_tests=0
|
|
total_tests=8
|
|
@@ -86,9 +89,9 @@ function run_test {
|
|
local expected_file="$6"
|
|
local should_create_file="$7"
|
|
local output_dir="$8"
|
|
-
|
|
+
|
|
echo "Test $test_num: $description"
|
|
-
|
|
+
|
|
pushd "$work_dir" >/dev/null
|
|
|
|
# Check if the expected file exists before running sscg
|
|
@@ -102,7 +105,7 @@ function run_test {
|
|
if [ -z "$output_dir" ]; then
|
|
output_dir="."
|
|
fi
|
|
-
|
|
+
|
|
# Run sscg with the specified arguments
|
|
local cmd_args=(
|
|
"${MESON_BUILD_ROOT}/sscg"
|
|
@@ -111,23 +114,23 @@ function run_test {
|
|
--cert-file "${output_dir}/service.pem"
|
|
--cert-key-file "${output_dir}/service-key.pem"
|
|
)
|
|
-
|
|
+
|
|
if [ -n "$dhparams_output_file" ]; then
|
|
cmd_args+=("--dhparams-file=$dhparams_output_file")
|
|
fi
|
|
echo "${cmd_args[@]}"
|
|
-
|
|
+
|
|
local exit_code=0
|
|
"${cmd_args[@]}" >/dev/null 2>&1 || exit_code=$?
|
|
-
|
|
+
|
|
local test_passed=true
|
|
-
|
|
+
|
|
# Check exit code
|
|
if [ "$exit_code" -ne "$expected_exit_code" ]; then
|
|
echo " FAIL: Expected exit code $expected_exit_code, got $exit_code"
|
|
test_passed=false
|
|
fi
|
|
-
|
|
+
|
|
# Check file creation
|
|
if [ "$should_create_file" = "true" ]; then
|
|
if [ ! -f "$expected_file" ]; then
|
|
@@ -147,17 +150,17 @@ function run_test {
|
|
test_passed=false
|
|
fi
|
|
fi
|
|
-
|
|
+
|
|
if [ "$test_passed" = "true" ]; then
|
|
echo " PASS"
|
|
else
|
|
((failed_tests++))
|
|
fi
|
|
-
|
|
+
|
|
# Clean up any created files for next test
|
|
rm -f "${output_dir}/ca.crt" "${output_dir}/service.pem" "${output_dir}/service-key.pem"
|
|
rm -f "$expected_file" || true # Ignore errors
|
|
-
|
|
+
|
|
popd >/dev/null
|
|
echo
|
|
}
|
|
@@ -250,15 +253,15 @@ run_test \
|
|
"false" \
|
|
"$WRITABLE_DIR"
|
|
|
|
-# Test 8: --dhparams-file to non-writable path, existing file
|
|
+# Test 8: --dhparams-file to non-writable path, existing file
|
|
# Arguments: test_num description work_dir dhparams_output_file expected_exit_code expected_file should_create_file output_dir
|
|
run_test \
|
|
8 \
|
|
"--dhparams-file to non-writable path, existing file" \
|
|
"$WRITABLE_DIR" \
|
|
- "$READONLY_DIR/dhparams.pem" \
|
|
+ "$READONLY_WITH_DHPARAMS_DIR/dhparams.pem" \
|
|
17 \
|
|
- "$READONLY_DIR/dhparams.pem" \
|
|
+ "$READONLY_WITH_DHPARAMS_DIR/dhparams.pem" \
|
|
"false" \
|
|
"$WRITABLE_DIR"
|
|
|
|
diff --git a/test/test_ecdsa_cert_validity.sh b/test/test_ecdsa_cert_validity.sh
|
|
index e7a58c543b692099fc6aeb965e19a6cbe7841975..996be7d1628204433a342f322c72d0b3359fd109 100755
|
|
--- a/test/test_ecdsa_cert_validity.sh
|
|
+++ b/test/test_ecdsa_cert_validity.sh
|
|
@@ -185,18 +185,18 @@ pushd "$TMPDIR"
|
|
--key-type ecdsa \
|
|
--ec-curve "$_arg_ec_curve" \
|
|
--cert-key-password mypassword \
|
|
- --dhparams-file "dhparams.pem"
|
|
+ --dhparams-file "dhparams.pem" \
|
|
+ --ca-file "combined.crt" \
|
|
+ --cert-file "combined.crt"
|
|
|
|
# Verify that the expected files were created
|
|
test -e dhparams.pem
|
|
-test -e ca.crt
|
|
-test -e service.pem
|
|
+test -e combined.crt
|
|
test -e service-key.pem
|
|
|
|
# Verify that they have the correct file format
|
|
openssl dhparam -noout -in dhparams.pem
|
|
-openssl x509 -noout -in ca.crt
|
|
-openssl x509 -noout -in service.pem
|
|
+openssl x509 -noout -in combined.crt
|
|
|
|
grep "ENCRYPTED PRIVATE KEY" service-key.pem
|
|
openssl pkey -noout -in service-key.pem -passin pass:mypassword
|
|
@@ -210,7 +210,7 @@ curve_info=$(openssl pkey -text -noout -in service-key.pem -passin pass:mypasswo
|
|
echo "$curve_info" | grep "$_arg_ec_curve"
|
|
|
|
# Validate the certificates
|
|
-openssl verify -x509_strict -CAfile ca.crt service.pem
|
|
+openssl verify -x509_strict -CAfile combined.crt combined.crt
|
|
|
|
popd # $TMPDIR
|
|
|
|
--
|
|
2.52.0
|
|
|