CVE-2023-1672: Concurrent Execution using Shared Resource with Improper Synchronization ('Race Condition') (CWE-362)
value: A race condition exists in the Tang server functionality for key generation and key rotation. This flaw results in a small time window where Tang private keys become readable by other processes on the same host.
CWE description: The product contains a code sequence that can run concurrently with other code, and the code sequence requires temporary, exclusive access to a shared resource, but a timing window exists in which the shared resource can be modified by another code sequence that is operating concurrently.
CVSS v3 base score: 5.3
Published: 2023-07-11T12:15Z
Repository: https://github.com/latchset/tang
Language: Other
Files changed: src/keys.c, src/tangd-keygen.in, src/tangd-rotate-keys.in
Commit date: 2023-06-14 10:53:20 -0300
Commit message: Fix race condition when creating/rotating keys (#123)

When we create/rotate keys using either the tangd-keygen and
tangd-rotate-keys helpers, there is a small window between the
keys being created and then the proper ownership permissions being
set. This also happens when there are no keys and tang creates a
pair of keys itself.

In certain situations, such as the keys directory having wide open
permissions, a user with local access could exploit this race
condition and read the keys before they are set to more restrictive
permissions.

To prevent this issue, we now set the default umask to 0337 before
creating the files, so that they are already created with restrictive
permissions; afterwards, we set the proper ownership as usual.

Issue reported by Brian McDermott of CENSUS labs.

Fixes CVE-2023-1672


Reviewed-by: Sergio Arroutbi <sarroutb@redhat.com>
Signed-off-by: Sergio Correia <scorreia@redhat.com>
Diff: diff --git a/src/keys.c b/src/keys.c
index bbfc374..3403922 100644
--- a/src/keys.c
+++ b/src/keys.c
@@ -305,10 +305,13 @@ prepare_payload_and_sign(struct tang_keys_info* tki)
 static int
 create_new_keys(const char* jwkdir)
 {
     const char* alg[] = {"ES512", "ECMR", NULL};
     char path[PATH_MAX];
+
+    /* Set default umask for file creation. */
+    umask(0337);
     for (int i = 0; alg[i] != NULL; i++) {
         json_auto_t* jwk = jwk_generate(alg[i]);
         if (!jwk) {
             return 0;
         }
diff --git a/src/tangd-keygen.in b/src/tangd-keygen.in
index 522121c..6e0b033 100755
--- a/src/tangd-keygen.in
+++ b/src/tangd-keygen.in
@@ -36,10 +36,14 @@ set_perms() {
 [ -d "$1" ] || usage
 
 [ $# -eq 3 ] && sig=$2 && exc=$3
 
 THP_DEFAULT_HASH=S256     # SHA-256.
+
+# Set default umask for file creation.
+umask 0337
+
 jwe=$(jose jwk gen -i '{"alg":"ES512"}')
 [ -z "$sig" ] && sig=$(echo "$jwe" | jose jwk thp -i- -a "${THP_DEFAULT_HASH}")
 echo "$jwe" > "$1/$sig.jwk"
 set_perms "$1/$sig.jwk"
 
diff --git a/src/tangd-rotate-keys.in b/src/tangd-rotate-keys.in
index 601f475..a1929d3 100755
--- a/src/tangd-rotate-keys.in
+++ b/src/tangd-rotate-keys.in
@@ -77,10 +77,14 @@ cd "${JWKDIR}" || error "Unable to change to keys directory '${JWKDIR}'"
         log "Disabled advertisement of key ${key} -> .${key}" "${VERBOSE}"
     done
 
     # Create a new set of keys.
     DEFAULT_THP_HASH="S256"
+
+    # Set default umask for file creation.
+    umask 0337
+
     for alg in "ES512" "ECMR"; do
         json="$(printf '{"alg": "%s"}' "${alg}")"
         jwe="$(jose jwk gen --input "${json}")"
         thp="$(printf '%s' "${jwe}" | jose jwk thp --input=- \
                                            -a "${DEFAULT_THP_HASH}")"

Fixed code: 
    /* Set default umask for file creation. */
    umask(0337);

# Set default umask for file creation.
umask 0337


    # Set default umask for file creation.
    umask 0337