You can not select more than 25 topics
Topics must start with a letter or number, can include dashes ('-') and can be up to 35 characters long.
495 lines
17 KiB
495 lines
17 KiB
8 months ago
|
From a86b58ed1419b9af8a486dcaf95608eb07241965 Mon Sep 17 00:00:00 2001
|
||
|
From: Lennart Poettering <lennart@poettering.net>
|
||
|
Date: Tue, 10 Jan 2023 12:39:58 +0100
|
||
|
Subject: [PATCH] tree-wide: use CLEANUP_ERASE() at various places
|
||
|
|
||
|
Let's use this new macro wherever it makes sense, as it allows us to
|
||
|
shorten or clean-up paths, and makes it less likely to miss a return
|
||
|
path.
|
||
|
|
||
|
(cherry picked from commit 692597c84395ad2b3f8e221bb1eca55a9dfc544f)
|
||
|
|
||
|
Related: RHEL-16182
|
||
|
---
|
||
|
src/home/homework-fscrypt.c | 58 ++++++++++--------------------
|
||
|
src/shared/ask-password-api.c | 66 +++++++++++++----------------------
|
||
|
src/shared/creds-util.c | 24 ++++++-------
|
||
|
src/shared/ethtool-util.c | 11 +++---
|
||
|
src/shared/tpm2-util.c | 15 ++++----
|
||
|
5 files changed, 67 insertions(+), 107 deletions(-)
|
||
|
|
||
|
diff --git a/src/home/homework-fscrypt.c b/src/home/homework-fscrypt.c
|
||
|
index bd32393d93..455a4c88fc 100644
|
||
|
--- a/src/home/homework-fscrypt.c
|
||
|
+++ b/src/home/homework-fscrypt.c
|
||
|
@@ -58,10 +58,10 @@ static int fscrypt_upload_volume_key(
|
||
|
};
|
||
|
memcpy(key.raw, volume_key, volume_key_size);
|
||
|
|
||
|
+ CLEANUP_ERASE(key);
|
||
|
+
|
||
|
/* Upload to the kernel */
|
||
|
serial = add_key("logon", description, &key, sizeof(key), where);
|
||
|
- explicit_bzero_safe(&key, sizeof(key));
|
||
|
-
|
||
|
if (serial < 0)
|
||
|
return log_error_errno(errno, "Failed to install master key in keyring: %m");
|
||
|
|
||
|
@@ -124,20 +124,18 @@ static int fscrypt_slot_try_one(
|
||
|
* resulting hash.
|
||
|
*/
|
||
|
|
||
|
+ CLEANUP_ERASE(derived);
|
||
|
+
|
||
|
if (PKCS5_PBKDF2_HMAC(
|
||
|
password, strlen(password),
|
||
|
salt, salt_size,
|
||
|
0xFFFF, EVP_sha512(),
|
||
|
- sizeof(derived), derived) != 1) {
|
||
|
- r = log_error_errno(SYNTHETIC_ERRNO(ENOTRECOVERABLE), "PBKDF2 failed");
|
||
|
- goto finish;
|
||
|
- }
|
||
|
+ sizeof(derived), derived) != 1)
|
||
|
+ return log_error_errno(SYNTHETIC_ERRNO(ENOTRECOVERABLE), "PBKDF2 failed");
|
||
|
|
||
|
context = EVP_CIPHER_CTX_new();
|
||
|
- if (!context) {
|
||
|
- r = log_oom();
|
||
|
- goto finish;
|
||
|
- }
|
||
|
+ if (!context)
|
||
|
+ return log_oom();
|
||
|
|
||
|
/* We use AES256 in counter mode */
|
||
|
assert_se(cc = EVP_aes_256_ctr());
|
||
|
@@ -145,13 +143,8 @@ static int fscrypt_slot_try_one(
|
||
|
/* We only use the first half of the derived key */
|
||
|
assert(sizeof(derived) >= (size_t) EVP_CIPHER_key_length(cc));
|
||
|
|
||
|
- if (EVP_DecryptInit_ex(context, cc, NULL, derived, NULL) != 1) {
|
||
|
- r = log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Failed to initialize decryption context.");
|
||
|
- goto finish;
|
||
|
- }
|
||
|
-
|
||
|
- /* Flush out the derived key now, we don't need it anymore */
|
||
|
- explicit_bzero_safe(derived, sizeof(derived));
|
||
|
+ if (EVP_DecryptInit_ex(context, cc, NULL, derived, NULL) != 1)
|
||
|
+ return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Failed to initialize decryption context.");
|
||
|
|
||
|
decrypted_size = encrypted_size + EVP_CIPHER_key_length(cc) * 2;
|
||
|
decrypted = malloc(decrypted_size);
|
||
|
@@ -184,10 +177,6 @@ static int fscrypt_slot_try_one(
|
||
|
*ret_decrypted_size = decrypted_size;
|
||
|
|
||
|
return 0;
|
||
|
-
|
||
|
-finish:
|
||
|
- explicit_bzero_safe(derived, sizeof(derived));
|
||
|
- return r;
|
||
|
}
|
||
|
|
||
|
static int fscrypt_slot_try_many(
|
||
|
@@ -414,20 +403,18 @@ static int fscrypt_slot_set(
|
||
|
if (r < 0)
|
||
|
return log_error_errno(r, "Failed to generate salt: %m");
|
||
|
|
||
|
+ CLEANUP_ERASE(derived);
|
||
|
+
|
||
|
if (PKCS5_PBKDF2_HMAC(
|
||
|
password, strlen(password),
|
||
|
salt, sizeof(salt),
|
||
|
0xFFFF, EVP_sha512(),
|
||
|
- sizeof(derived), derived) != 1) {
|
||
|
- r = log_error_errno(SYNTHETIC_ERRNO(ENOTRECOVERABLE), "PBKDF2 failed");
|
||
|
- goto finish;
|
||
|
- }
|
||
|
+ sizeof(derived), derived) != 1)
|
||
|
+ return log_error_errno(SYNTHETIC_ERRNO(ENOTRECOVERABLE), "PBKDF2 failed");
|
||
|
|
||
|
context = EVP_CIPHER_CTX_new();
|
||
|
- if (!context) {
|
||
|
- r = log_oom();
|
||
|
- goto finish;
|
||
|
- }
|
||
|
+ if (!context)
|
||
|
+ return log_oom();
|
||
|
|
||
|
/* We use AES256 in counter mode */
|
||
|
cc = EVP_aes_256_ctr();
|
||
|
@@ -435,13 +422,8 @@ static int fscrypt_slot_set(
|
||
|
/* We only use the first half of the derived key */
|
||
|
assert(sizeof(derived) >= (size_t) EVP_CIPHER_key_length(cc));
|
||
|
|
||
|
- if (EVP_EncryptInit_ex(context, cc, NULL, derived, NULL) != 1) {
|
||
|
- r = log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Failed to initialize encryption context.");
|
||
|
- goto finish;
|
||
|
- }
|
||
|
-
|
||
|
- /* Flush out the derived key now, we don't need it anymore */
|
||
|
- explicit_bzero_safe(derived, sizeof(derived));
|
||
|
+ if (EVP_EncryptInit_ex(context, cc, NULL, derived, NULL) != 1)
|
||
|
+ return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Failed to initialize encryption context.");
|
||
|
|
||
|
encrypted_size = volume_key_size + EVP_CIPHER_key_length(cc) * 2;
|
||
|
encrypted = malloc(encrypted_size);
|
||
|
@@ -478,10 +460,6 @@ static int fscrypt_slot_set(
|
||
|
log_info("Written key slot %s.", label);
|
||
|
|
||
|
return 0;
|
||
|
-
|
||
|
-finish:
|
||
|
- explicit_bzero_safe(derived, sizeof(derived));
|
||
|
- return r;
|
||
|
}
|
||
|
|
||
|
int home_create_fscrypt(
|
||
|
diff --git a/src/shared/ask-password-api.c b/src/shared/ask-password-api.c
|
||
|
index 871af2ec99..5f05271caa 100644
|
||
|
--- a/src/shared/ask-password-api.c
|
||
|
+++ b/src/shared/ask-password-api.c
|
||
|
@@ -253,6 +253,8 @@ int ask_password_plymouth(
|
||
|
if (r < 0)
|
||
|
return r;
|
||
|
|
||
|
+ CLEANUP_ERASE(buffer);
|
||
|
+
|
||
|
pollfd[POLL_SOCKET].fd = fd;
|
||
|
pollfd[POLL_SOCKET].events = POLLIN;
|
||
|
pollfd[POLL_INOTIFY].fd = notify;
|
||
|
@@ -266,20 +268,16 @@ int ask_password_plymouth(
|
||
|
else
|
||
|
timeout = USEC_INFINITY;
|
||
|
|
||
|
- if (flag_file && access(flag_file, F_OK) < 0) {
|
||
|
- r = -errno;
|
||
|
- goto finish;
|
||
|
- }
|
||
|
+ if (flag_file && access(flag_file, F_OK) < 0)
|
||
|
+ return -errno;
|
||
|
|
||
|
r = ppoll_usec(pollfd, notify >= 0 ? 2 : 1, timeout);
|
||
|
if (r == -EINTR)
|
||
|
continue;
|
||
|
if (r < 0)
|
||
|
- goto finish;
|
||
|
- if (r == 0) {
|
||
|
- r = -ETIME;
|
||
|
- goto finish;
|
||
|
- }
|
||
|
+ return r;
|
||
|
+ if (r == 0)
|
||
|
+ return -ETIME;
|
||
|
|
||
|
if (notify >= 0 && pollfd[POLL_INOTIFY].revents != 0)
|
||
|
(void) flush_fd(notify);
|
||
|
@@ -292,13 +290,10 @@ int ask_password_plymouth(
|
||
|
if (ERRNO_IS_TRANSIENT(errno))
|
||
|
continue;
|
||
|
|
||
|
- r = -errno;
|
||
|
- goto finish;
|
||
|
- }
|
||
|
- if (k == 0) {
|
||
|
- r = -EIO;
|
||
|
- goto finish;
|
||
|
+ return -errno;
|
||
|
}
|
||
|
+ if (k == 0)
|
||
|
+ return -EIO;
|
||
|
|
||
|
p += k;
|
||
|
|
||
|
@@ -310,14 +305,12 @@ int ask_password_plymouth(
|
||
|
* with a normal password request */
|
||
|
packet = mfree(packet);
|
||
|
|
||
|
- if (asprintf(&packet, "*\002%c%s%n", (int) (strlen(message) + 1), message, &n) < 0) {
|
||
|
- r = -ENOMEM;
|
||
|
- goto finish;
|
||
|
- }
|
||
|
+ if (asprintf(&packet, "*\002%c%s%n", (int) (strlen(message) + 1), message, &n) < 0)
|
||
|
+ return -ENOMEM;
|
||
|
|
||
|
r = loop_write(fd, packet, n+1, true);
|
||
|
if (r < 0)
|
||
|
- goto finish;
|
||
|
+ return r;
|
||
|
|
||
|
flags &= ~ASK_PASSWORD_ACCEPT_CACHED;
|
||
|
p = 0;
|
||
|
@@ -325,8 +318,7 @@ int ask_password_plymouth(
|
||
|
}
|
||
|
|
||
|
/* No password, because UI not shown */
|
||
|
- r = -ENOENT;
|
||
|
- goto finish;
|
||
|
+ return -ENOENT;
|
||
|
|
||
|
} else if (IN_SET(buffer[0], 2, 9)) {
|
||
|
uint32_t size;
|
||
|
@@ -338,35 +330,25 @@ int ask_password_plymouth(
|
||
|
|
||
|
memcpy(&size, buffer+1, sizeof(size));
|
||
|
size = le32toh(size);
|
||
|
- if (size + 5 > sizeof(buffer)) {
|
||
|
- r = -EIO;
|
||
|
- goto finish;
|
||
|
- }
|
||
|
+ if (size + 5 > sizeof(buffer))
|
||
|
+ return -EIO;
|
||
|
|
||
|
if (p-5 < size)
|
||
|
continue;
|
||
|
|
||
|
l = strv_parse_nulstr(buffer + 5, size);
|
||
|
- if (!l) {
|
||
|
- r = -ENOMEM;
|
||
|
- goto finish;
|
||
|
- }
|
||
|
+ if (!l)
|
||
|
+ return -ENOMEM;
|
||
|
|
||
|
*ret = l;
|
||
|
break;
|
||
|
|
||
|
- } else {
|
||
|
+ } else
|
||
|
/* Unknown packet */
|
||
|
- r = -EIO;
|
||
|
- goto finish;
|
||
|
- }
|
||
|
+ return -EIO;
|
||
|
}
|
||
|
|
||
|
- r = 0;
|
||
|
-
|
||
|
-finish:
|
||
|
- explicit_bzero_safe(buffer, sizeof(buffer));
|
||
|
- return r;
|
||
|
+ return 0;
|
||
|
}
|
||
|
|
||
|
#define NO_ECHO "(no echo) "
|
||
|
@@ -428,6 +410,8 @@ int ask_password_tty(
|
||
|
return -errno;
|
||
|
}
|
||
|
|
||
|
+ CLEANUP_ERASE(passphrase);
|
||
|
+
|
||
|
/* If the caller didn't specify a TTY, then use the controlling tty, if we can. */
|
||
|
if (ttyfd < 0)
|
||
|
ttyfd = cttyfd = open("/dev/tty", O_RDWR|O_NOCTTY|O_CLOEXEC);
|
||
|
@@ -631,7 +615,6 @@ int ask_password_tty(
|
||
|
}
|
||
|
|
||
|
x = strndup(passphrase, p);
|
||
|
- explicit_bzero_safe(passphrase, sizeof(passphrase));
|
||
|
if (!x) {
|
||
|
r = -ENOMEM;
|
||
|
goto finish;
|
||
|
@@ -891,6 +874,8 @@ int ask_password_agent(
|
||
|
goto finish;
|
||
|
}
|
||
|
|
||
|
+ CLEANUP_ERASE(passphrase);
|
||
|
+
|
||
|
cmsg_close_all(&msghdr);
|
||
|
|
||
|
if (n == 0) {
|
||
|
@@ -915,7 +900,6 @@ int ask_password_agent(
|
||
|
l = strv_new("");
|
||
|
else
|
||
|
l = strv_parse_nulstr(passphrase+1, n-1);
|
||
|
- explicit_bzero_safe(passphrase, n);
|
||
|
if (!l) {
|
||
|
r = -ENOMEM;
|
||
|
goto finish;
|
||
|
diff --git a/src/shared/creds-util.c b/src/shared/creds-util.c
|
||
|
index ecf90e2084..9ac0320c58 100644
|
||
|
--- a/src/shared/creds-util.c
|
||
|
+++ b/src/shared/creds-util.c
|
||
|
@@ -165,7 +165,6 @@ static int make_credential_host_secret(
|
||
|
void **ret_data,
|
||
|
size_t *ret_size) {
|
||
|
|
||
|
- struct credential_host_secret_format buf;
|
||
|
_cleanup_free_ char *t = NULL;
|
||
|
_cleanup_close_ int fd = -1;
|
||
|
int r;
|
||
|
@@ -189,21 +188,23 @@ static int make_credential_host_secret(
|
||
|
if (r < 0)
|
||
|
log_debug_errno(r, "Failed to set file attributes for secrets file, ignoring: %m");
|
||
|
|
||
|
- buf = (struct credential_host_secret_format) {
|
||
|
+ struct credential_host_secret_format buf = {
|
||
|
.machine_id = machine_id,
|
||
|
};
|
||
|
|
||
|
+ CLEANUP_ERASE(buf);
|
||
|
+
|
||
|
r = crypto_random_bytes(buf.data, sizeof(buf.data));
|
||
|
if (r < 0)
|
||
|
- goto finish;
|
||
|
+ goto fail;
|
||
|
|
||
|
r = loop_write(fd, &buf, sizeof(buf), false);
|
||
|
if (r < 0)
|
||
|
- goto finish;
|
||
|
+ goto fail;
|
||
|
|
||
|
if (fsync(fd) < 0) {
|
||
|
r = -errno;
|
||
|
- goto finish;
|
||
|
+ goto fail;
|
||
|
}
|
||
|
|
||
|
warn_not_encrypted(fd, flags, dirname, fn);
|
||
|
@@ -211,17 +212,17 @@ static int make_credential_host_secret(
|
||
|
if (t) {
|
||
|
r = rename_noreplace(dfd, t, dfd, fn);
|
||
|
if (r < 0)
|
||
|
- goto finish;
|
||
|
+ goto fail;
|
||
|
|
||
|
t = mfree(t);
|
||
|
} else if (linkat(fd, "", dfd, fn, AT_EMPTY_PATH) < 0) {
|
||
|
r = -errno;
|
||
|
- goto finish;
|
||
|
+ goto fail;
|
||
|
}
|
||
|
|
||
|
if (fsync(dfd) < 0) {
|
||
|
r = -errno;
|
||
|
- goto finish;
|
||
|
+ goto fail;
|
||
|
}
|
||
|
|
||
|
if (ret_data) {
|
||
|
@@ -230,7 +231,7 @@ static int make_credential_host_secret(
|
||
|
copy = memdup(buf.data, sizeof(buf.data));
|
||
|
if (!copy) {
|
||
|
r = -ENOMEM;
|
||
|
- goto finish;
|
||
|
+ goto fail;
|
||
|
}
|
||
|
|
||
|
*ret_data = copy;
|
||
|
@@ -239,13 +240,12 @@ static int make_credential_host_secret(
|
||
|
if (ret_size)
|
||
|
*ret_size = sizeof(buf.data);
|
||
|
|
||
|
- r = 0;
|
||
|
+ return 0;
|
||
|
|
||
|
-finish:
|
||
|
+fail:
|
||
|
if (t && unlinkat(dfd, t, 0) < 0)
|
||
|
log_debug_errno(errno, "Failed to remove temporary credential key: %m");
|
||
|
|
||
|
- explicit_bzero_safe(&buf, sizeof(buf));
|
||
|
return r;
|
||
|
}
|
||
|
|
||
|
diff --git a/src/shared/ethtool-util.c b/src/shared/ethtool-util.c
|
||
|
index e39b2f754b..1900537917 100644
|
||
|
--- a/src/shared/ethtool-util.c
|
||
|
+++ b/src/shared/ethtool-util.c
|
||
|
@@ -434,6 +434,8 @@ int ethtool_set_wol(
|
||
|
|
||
|
strscpy(ifr.ifr_name, sizeof(ifr.ifr_name), ifname);
|
||
|
|
||
|
+ CLEANUP_ERASE(ecmd);
|
||
|
+
|
||
|
if (ioctl(*ethtool_fd, SIOCETHTOOL, &ifr) < 0)
|
||
|
return -errno;
|
||
|
|
||
|
@@ -466,16 +468,11 @@ int ethtool_set_wol(
|
||
|
need_update = true;
|
||
|
}
|
||
|
|
||
|
- if (!need_update) {
|
||
|
- explicit_bzero_safe(&ecmd, sizeof(ecmd));
|
||
|
+ if (!need_update)
|
||
|
return 0;
|
||
|
- }
|
||
|
|
||
|
ecmd.cmd = ETHTOOL_SWOL;
|
||
|
- r = RET_NERRNO(ioctl(*ethtool_fd, SIOCETHTOOL, &ifr));
|
||
|
-
|
||
|
- explicit_bzero_safe(&ecmd, sizeof(ecmd));
|
||
|
- return r;
|
||
|
+ return RET_NERRNO(ioctl(*ethtool_fd, SIOCETHTOOL, &ifr));
|
||
|
}
|
||
|
|
||
|
int ethtool_set_nic_buffer_size(int *ethtool_fd, const char *ifname, const netdev_ring_param *ring) {
|
||
|
diff --git a/src/shared/tpm2-util.c b/src/shared/tpm2-util.c
|
||
|
index d1a4e9cd11..7e98ec851b 100644
|
||
|
--- a/src/shared/tpm2-util.c
|
||
|
+++ b/src/shared/tpm2-util.c
|
||
|
@@ -778,13 +778,14 @@ static void hash_pin(const char *pin, size_t len, TPM2B_AUTH *auth) {
|
||
|
|
||
|
assert(auth);
|
||
|
assert(pin);
|
||
|
+
|
||
|
auth->size = SHA256_DIGEST_SIZE;
|
||
|
|
||
|
+ CLEANUP_ERASE(hash);
|
||
|
+
|
||
|
sha256_init_ctx(&hash);
|
||
|
sha256_process_bytes(pin, len, &hash);
|
||
|
sha256_finish_ctx(&hash, auth->buffer);
|
||
|
-
|
||
|
- explicit_bzero_safe(&hash, sizeof(hash));
|
||
|
}
|
||
|
|
||
|
static int tpm2_make_encryption_session(
|
||
|
@@ -816,11 +817,11 @@ static int tpm2_make_encryption_session(
|
||
|
if (pin) {
|
||
|
TPM2B_AUTH auth = {};
|
||
|
|
||
|
+ CLEANUP_ERASE(auth);
|
||
|
+
|
||
|
hash_pin(pin, strlen(pin), &auth);
|
||
|
|
||
|
rc = sym_Esys_TR_SetAuth(c, bind_key, &auth);
|
||
|
- /* ESAPI knows about it, so clear it from our memory */
|
||
|
- explicit_bzero_safe(&auth, sizeof(auth));
|
||
|
if (rc != TSS2_RC_SUCCESS)
|
||
|
return log_error_errno(
|
||
|
SYNTHETIC_ERRNO(ENOTRECOVERABLE),
|
||
|
@@ -1412,8 +1413,8 @@ int tpm2_seal(const char *device,
|
||
|
static const TPML_PCR_SELECTION creation_pcr = {};
|
||
|
_cleanup_(erase_and_freep) void *secret = NULL;
|
||
|
_cleanup_free_ void *blob = NULL, *hash = NULL;
|
||
|
- TPM2B_SENSITIVE_CREATE hmac_sensitive;
|
||
|
ESYS_TR primary = ESYS_TR_NONE, session = ESYS_TR_NONE;
|
||
|
+ TPM2B_SENSITIVE_CREATE hmac_sensitive;
|
||
|
TPMI_ALG_PUBLIC primary_alg;
|
||
|
TPM2B_PUBLIC hmac_template;
|
||
|
TPMI_ALG_HASH pcr_bank;
|
||
|
@@ -1453,6 +1454,8 @@ int tpm2_seal(const char *device,
|
||
|
|
||
|
start = now(CLOCK_MONOTONIC);
|
||
|
|
||
|
+ CLEANUP_ERASE(hmac_sensitive);
|
||
|
+
|
||
|
r = tpm2_context_init(device, &c);
|
||
|
if (r < 0)
|
||
|
return r;
|
||
|
@@ -1541,7 +1544,6 @@ int tpm2_seal(const char *device,
|
||
|
}
|
||
|
|
||
|
secret = memdup(hmac_sensitive.sensitive.data.buffer, hmac_sensitive.sensitive.data.size);
|
||
|
- explicit_bzero_safe(hmac_sensitive.sensitive.data.buffer, hmac_sensitive.sensitive.data.size);
|
||
|
if (!secret) {
|
||
|
r = log_oom();
|
||
|
goto finish;
|
||
|
@@ -1602,7 +1604,6 @@ int tpm2_seal(const char *device,
|
||
|
r = 0;
|
||
|
|
||
|
finish:
|
||
|
- explicit_bzero_safe(&hmac_sensitive, sizeof(hmac_sensitive));
|
||
|
primary = tpm2_flush_context_verbose(c.esys_context, primary);
|
||
|
session = tpm2_flush_context_verbose(c.esys_context, session);
|
||
|
return r;
|