From: Lennart Poettering Date: Tue, 7 Oct 2025 16:37:07 +0000 (+0200) Subject: creds: add explicit control on whether to allow null key decryption X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=e921d6d40fda2e4aa2d77b4659e01e8ff7de7a92;p=thirdparty%2Fsystemd.git creds: add explicit control on whether to allow null key decryption The ability to encrypt/authenticate encryption with a null key was originally just a fallback concept for cases where during early boot we have no host key, but the local system has no TPM2. Nowadays it is used for other stuff as well, such as pcrlock data propagation (i.e. data that needs no protection itself and required to properly to TPM key derivation). Let's give better, explicit control over null key usage, i.e. let's make it a tristate both on the systemd-creds command line and in the Varlink IPC to control three cases: - the default that we allow it only if SecureBoot is off - explicitly allowed - explicitly refused (this is new) Ideally systemd-creds --allow-null switch would take a boolean argument to control this as a tristate. Alas, that would be a compat break, hence I added --refuse-null instead (which also maps to the low-level flag for this). This also normalizes that the null key is always called "null key" in messages, and not sometimes "empty key" or "fallback key". --- diff --git a/man/systemd-creds.xml b/man/systemd-creds.xml index 2d62faafda0..7b9ce14e5f4 100644 --- a/man/systemd-creds.xml +++ b/man/systemd-creds.xml @@ -424,11 +424,19 @@ - Allow decrypting credentials that use an empty key. + Allow decrypting credentials that use a null key. By default decryption of credentials encrypted/authenticated with a null key is only allowed if UEFI SecureBoot is off. + + + + Refuse decrypting credentials that use a null key, regardless of the UEFI SecureBoot state (see above). + + + + diff --git a/src/creds/creds.c b/src/creds/creds.c index 7c8731fbf6a..98206f719df 100644 --- a/src/creds/creds.c +++ b/src/creds/creds.c @@ -72,7 +72,7 @@ static bool arg_pretty = false; static bool arg_quiet = false; static bool arg_varlink = false; static uid_t arg_uid = UID_INVALID; -static bool arg_allow_null = false; +static CredentialFlags arg_credential_flags = 0; static bool arg_ask_password = true; STATIC_DESTRUCTOR_REGISTER(arg_tpm2_public_key, freep); @@ -526,7 +526,7 @@ static int verb_cat(int argc, char **argv, void *userdata) { timestamp, uid_is_valid(arg_uid) ? arg_uid : getuid(), &IOVEC_MAKE(data, size), - CREDENTIAL_ANY_SCOPE, + arg_credential_flags | CREDENTIAL_ANY_SCOPE, &plaintext); else r = decrypt_credential_and_warn( @@ -536,7 +536,7 @@ static int verb_cat(int argc, char **argv, void *userdata) { arg_tpm2_signature, uid_is_valid(arg_uid) ? arg_uid : getuid(), &IOVEC_MAKE(data, size), - CREDENTIAL_ANY_SCOPE, + arg_credential_flags | CREDENTIAL_ANY_SCOPE, &plaintext); if (r < 0) return r; @@ -608,7 +608,7 @@ static int verb_encrypt(int argc, char **argv, void *userdata) { arg_not_after, arg_uid, &plaintext, - arg_ask_password ? CREDENTIAL_IPC_ALLOW_INTERACTIVE : 0, + arg_credential_flags, &output); } else r = encrypt_credential_and_warn( @@ -622,7 +622,7 @@ static int verb_encrypt(int argc, char **argv, void *userdata) { arg_tpm2_public_key_pcr_mask, arg_uid, &plaintext, - /* flags= */ 0, + arg_credential_flags, &output); if (r < 0) return r; @@ -713,7 +713,7 @@ static int verb_decrypt(int argc, char **argv, void *userdata) { timestamp, arg_uid, &input, - arg_ask_password ? CREDENTIAL_IPC_ALLOW_INTERACTIVE : 0, + arg_credential_flags, &plaintext); } else r = decrypt_credential_and_warn( @@ -723,7 +723,7 @@ static int verb_decrypt(int argc, char **argv, void *userdata) { arg_tpm2_signature, arg_uid, &input, - arg_allow_null ? CREDENTIAL_ALLOW_NULL : 0, + arg_credential_flags, &plaintext); if (r < 0) return r; @@ -815,7 +815,8 @@ static int verb_help(int argc, char **argv, void *userdata) { " Specify signature for public key PCR policy\n" " --user Select user-scoped credential encryption\n" " --uid=UID Select user for scoped credentials\n" - " --allow-null Allow decrypting credentials with empty key\n" + " --allow-null Allow decrypting credentials with null key\n" + " --refuse-null Refuse decrypting credentials with null key\n" "\nSee the %2$s for details.\n", program_invocation_short_name, link, @@ -849,6 +850,7 @@ static int parse_argv(int argc, char *argv[]) { ARG_USER, ARG_UID, ARG_ALLOW_NULL, + ARG_REFUSE_NULL, ARG_NO_ASK_PASSWORD, }; @@ -875,6 +877,7 @@ static int parse_argv(int argc, char *argv[]) { { "user", no_argument, NULL, ARG_USER }, { "uid", required_argument, NULL, ARG_UID }, { "allow-null", no_argument, NULL, ARG_ALLOW_NULL }, + { "refuse-null", no_argument, NULL, ARG_REFUSE_NULL }, { "no-ask-password", no_argument, NULL, ARG_NO_ASK_PASSWORD }, {} }; @@ -1068,7 +1071,13 @@ static int parse_argv(int argc, char *argv[]) { break; case ARG_ALLOW_NULL: - arg_allow_null = true; + arg_credential_flags &= ~CREDENTIAL_REFUSE_NULL; + arg_credential_flags |= CREDENTIAL_ALLOW_NULL; + break; + + case ARG_REFUSE_NULL: + arg_credential_flags |= CREDENTIAL_REFUSE_NULL; + arg_credential_flags &= ~CREDENTIAL_ALLOW_NULL; break; case ARG_NO_ASK_PASSWORD: @@ -1087,6 +1096,8 @@ static int parse_argv(int argc, char *argv[]) { } } + SET_FLAG(arg_credential_flags, CREDENTIAL_IPC_ALLOW_INTERACTIVE, arg_ask_password); + if (uid_is_valid(arg_uid)) { /* If a UID is specified, then switch to scoped credentials */ @@ -1391,6 +1402,7 @@ typedef struct MethodDecryptParameters { uint64_t timestamp; CredentialScope scope; uid_t uid; + int allow_null; } MethodDecryptParameters; static void method_decrypt_parameters_done(MethodDecryptParameters *p) { @@ -1402,11 +1414,12 @@ static void method_decrypt_parameters_done(MethodDecryptParameters *p) { static int vl_method_decrypt(sd_varlink *link, sd_json_variant *parameters, sd_varlink_method_flags_t flags, void *userdata) { static const sd_json_dispatch_field dispatch_table[] = { - { "name", SD_JSON_VARIANT_STRING, sd_json_dispatch_const_string, offsetof(MethodDecryptParameters, name), 0 }, - { "blob", SD_JSON_VARIANT_STRING, json_dispatch_unbase64_iovec, offsetof(MethodDecryptParameters, blob), SD_JSON_MANDATORY }, - { "timestamp", _SD_JSON_VARIANT_TYPE_INVALID, sd_json_dispatch_uint64, offsetof(MethodDecryptParameters, timestamp), 0 }, - { "scope", SD_JSON_VARIANT_STRING, dispatch_credential_scope, offsetof(MethodDecryptParameters, scope), 0 }, - { "uid", _SD_JSON_VARIANT_TYPE_INVALID, sd_json_dispatch_uid_gid, offsetof(MethodDecryptParameters, uid), 0 }, + { "name", SD_JSON_VARIANT_STRING, sd_json_dispatch_const_string, offsetof(MethodDecryptParameters, name), 0 }, + { "blob", SD_JSON_VARIANT_STRING, json_dispatch_unbase64_iovec, offsetof(MethodDecryptParameters, blob), SD_JSON_MANDATORY }, + { "timestamp", _SD_JSON_VARIANT_TYPE_INVALID, sd_json_dispatch_uint64, offsetof(MethodDecryptParameters, timestamp), 0 }, + { "scope", SD_JSON_VARIANT_STRING, dispatch_credential_scope, offsetof(MethodDecryptParameters, scope), 0 }, + { "uid", _SD_JSON_VARIANT_TYPE_INVALID, sd_json_dispatch_uid_gid, offsetof(MethodDecryptParameters, uid), 0 }, + { "allowNull", SD_JSON_VARIANT_BOOLEAN, sd_json_dispatch_tristate, offsetof(MethodDecryptParameters, allow_null), SD_JSON_NULLABLE }, VARLINK_DISPATCH_POLKIT_FIELD, {} }; @@ -1414,6 +1427,7 @@ static int vl_method_decrypt(sd_varlink *link, sd_json_variant *parameters, sd_v .timestamp = UINT64_MAX, .scope = _CREDENTIAL_SCOPE_INVALID, .uid = UID_INVALID, + .allow_null = -1, }; bool timestamp_fresh, any_scope_after_polkit = false; _cleanup_(iovec_done_erase) struct iovec output = {}; @@ -1440,6 +1454,11 @@ static int vl_method_decrypt(sd_varlink *link, sd_json_variant *parameters, sd_v if (r < 0) return r; + if (p.allow_null > 0) + cflags |= CREDENTIAL_ALLOW_NULL; + else if (p.allow_null == 0) + cflags |= CREDENTIAL_REFUSE_NULL; + r = sd_varlink_get_peer_uid(link, &peer_uid); if (r < 0) return r; diff --git a/src/shared/creds-util.c b/src/shared/creds-util.c index 7f7c04470f2..fe3fce3576b 100644 --- a/src/shared/creds-util.c +++ b/src/shared/creds-util.c @@ -812,6 +812,9 @@ int encrypt_credential_and_warn( assert(iovec_is_valid(input)); assert(ret); + /* Only one of these two flags may be set at the same time */ + assert(!FLAGS_SET(flags, CREDENTIAL_ALLOW_NULL) || !FLAGS_SET(flags, CREDENTIAL_REFUSE_NULL)); + if (!sd_id128_in_set(with_key, _CRED_AUTO, _CRED_AUTO_INITRD, @@ -1008,8 +1011,12 @@ int encrypt_credential_and_warn( } else id = with_key; - if (sd_id128_equal(id, CRED_AES256_GCM_BY_NULL) && !FLAGS_SET(flags, CREDENTIAL_ALLOW_NULL)) - log_warning("Using a null key for encryption and signing. Confidentiality or authenticity will not be provided."); + if (sd_id128_equal(id, CRED_AES256_GCM_BY_NULL)) { + if (FLAGS_SET(flags, CREDENTIAL_REFUSE_NULL)) + return log_error_errno(SYNTHETIC_ERRNO(EHWPOISON), "Attempted to encrypt with null key, but this is disallowed."); + if (!FLAGS_SET(flags, CREDENTIAL_ALLOW_NULL)) + log_warning("Using a null key for encryption and signing. Confidentiality or authenticity will not be provided."); + } /* Let's now take the host key and the TPM2 key and hash it together, to use as encryption key for the data */ r = sha256_hash_host_and_tpm2_key(&host_key, &tpm2_key, md); @@ -1201,12 +1208,15 @@ int decrypt_credential_and_warn( assert(iovec_is_valid(input)); assert(ret); + /* Only one of these two flags may be set at the same time */ + assert(!FLAGS_SET(flags, CREDENTIAL_ALLOW_NULL) || !FLAGS_SET(flags, CREDENTIAL_REFUSE_NULL)); + /* Relevant error codes: * * -EBADMSG → Corrupted file * -EOPNOTSUPP → Unsupported file type (could be: requires TPM but we have no TPM) * -EHOSTDOWN → Need PCR signature file, but couldn't find it - * -EHWPOISON → Attempt to decode NULL key (and CREDENTIAL_ALLOW_NULL is off), but the system has a TPM and SecureBoot is on + * -EHWPOISON → Attempt to unlock with NULL key and either CREDENTIAL_ALLOW_REFUSE is on, or CREDENTIAL_ALLOW_NULL is off, but the system has a TPM and SecureBoot is on * -EMEDIUMTYPE → File has unexpected scope, i.e. user-scoped credential is attempted to be unlocked in system scope, or vice versa * -EDESTADDRREQ → Credential is incorrectly named (i.e. the authenticated name does not match the actual name) * -ESTALE → Credential's validity has passed @@ -1237,24 +1247,30 @@ int decrypt_credential_and_warn( return log_error_errno(r, "Failed to load PCR signature: %m"); } - if (with_null && !FLAGS_SET(flags, CREDENTIAL_ALLOW_NULL)) { - /* So this is a credential encrypted with a zero length key. We support this to cover for the - * case where neither a host key not a TPM2 are available (specifically: initrd environments - * where the host key is not yet accessible and no TPM2 chip exists at all), to minimize - * different codeflow for TPM2 and non-TPM2 codepaths. Of course, credentials encoded this - * way offer no confidentiality nor authenticity. Because of that it's important we refuse to - * use them on systems that actually *do* have a TPM2 chip – if we are in SecureBoot - * mode. Otherwise an attacker could hand us credentials like this and we'd use them thinking - * they are trusted, even though they are not. */ - - if (efi_has_tpm2()) { - if (is_efi_secure_boot()) - return log_error_errno(SYNTHETIC_ERRNO(EHWPOISON), - "Credential uses fixed key for fallback use when TPM2 is absent — but TPM2 is present, and SecureBoot is enabled, refusing."); - - log_warning("Credential uses fixed key for use when TPM2 is absent, but TPM2 is present! Accepting anyway, since SecureBoot is disabled."); - } else - log_debug("Credential uses fixed key for use when TPM2 is absent, and TPM2 indeed is absent. Accepting."); + if (with_null) { + if (FLAGS_SET(flags, CREDENTIAL_REFUSE_NULL)) + return log_error_errno(SYNTHETIC_ERRNO(EHWPOISON), + "Credential uses null key, but that's not allowed, refusing."); + + if (!FLAGS_SET(flags, CREDENTIAL_ALLOW_NULL)) { + /* So this is a credential encrypted with a zero length key. We support this to cover for the + * case where neither a host key not a TPM2 are available (specifically: initrd environments + * where the host key is not yet accessible and no TPM2 chip exists at all), to minimize + * different codeflow for TPM2 and non-TPM2 codepaths. Of course, credentials encoded this + * way offer no confidentiality nor authenticity. Because of that it's important we refuse to + * use them on systems that actually *do* have a TPM2 chip – if we are in SecureBoot + * mode. Otherwise an attacker could hand us credentials like this and we'd use them thinking + * they are trusted, even though they are not. */ + + if (efi_has_tpm2()) { + if (is_efi_secure_boot()) + return log_error_errno(SYNTHETIC_ERRNO(EHWPOISON), + "Credential uses null key intended for fallback use when TPM2 is absent — but TPM2 is present, and SecureBoot is enabled, refusing."); + + log_warning("Credential uses null key intended for use when TPM2 is absent, but TPM2 is present! Accepting anyway, since SecureBoot is disabled."); + } else + log_debug("Credential uses null key intended for use when TPM2 is absent, and TPM2 indeed is absent. Accepting."); + } } if (with_scope) { @@ -1590,6 +1606,7 @@ int ipc_encrypt_credential(const char *name, usec_t timestamp, usec_t not_after, SD_JSON_BUILD_PAIR_CONDITION(not_after != USEC_INFINITY, "notAfter", SD_JSON_BUILD_UNSIGNED(not_after)), SD_JSON_BUILD_PAIR_CONDITION(!FLAGS_SET(flags, CREDENTIAL_ANY_SCOPE), "scope", SD_JSON_BUILD_STRING(uid_is_valid(uid) ? "user" : "system")), SD_JSON_BUILD_PAIR_CONDITION(uid_is_valid(uid), "uid", SD_JSON_BUILD_UNSIGNED(uid)), + SD_JSON_BUILD_PAIR_CONDITION((flags & (CREDENTIAL_ALLOW_NULL|CREDENTIAL_REFUSE_NULL)) != 0, "allowNull", SD_JSON_BUILD_BOOLEAN(flags & CREDENTIAL_ALLOW_NULL)), SD_JSON_BUILD_PAIR_BOOLEAN("allowInteractiveAuthentication", FLAGS_SET(flags, CREDENTIAL_IPC_ALLOW_INTERACTIVE))); if (r < 0) return log_error_errno(r, "Failed to call Encrypt() varlink call."); diff --git a/src/shared/creds-util.h b/src/shared/creds-util.h index 058c8cabc7b..8991d0678c7 100644 --- a/src/shared/creds-util.h +++ b/src/shared/creds-util.h @@ -54,11 +54,12 @@ int get_credential_host_secret(CredentialSecretFlags flags, struct iovec *ret); int get_credential_user_password(const char *username, char **ret_password, bool *ret_is_hashed); typedef enum CredentialFlags { - CREDENTIAL_ALLOW_NULL = 1 << 0, /* allow decryption of NULL key, even if TPM is around */ - CREDENTIAL_ANY_SCOPE = 1 << 1, /* allow decryption of both system and user credentials */ + CREDENTIAL_ALLOW_NULL = 1 << 0, /* allow decryption with NULL key, even if TPM is around */ + CREDENTIAL_REFUSE_NULL = 1 << 1, /* deny decryption with NULL key, even if SecureBoot is off */ + CREDENTIAL_ANY_SCOPE = 1 << 2, /* allow decryption of both system and user credentials */ /* Only used by ipc_{encrypt,decrypt}_credential */ - CREDENTIAL_IPC_ALLOW_INTERACTIVE = 1 << 2, + CREDENTIAL_IPC_ALLOW_INTERACTIVE = 1 << 3, } CredentialFlags; /* The four modes we support: keyed only by on-disk key, only by TPM2 HMAC key, and by the combination of diff --git a/src/shared/varlink-io.systemd.Credentials.c b/src/shared/varlink-io.systemd.Credentials.c index c774604d3f2..975e992f169 100644 --- a/src/shared/varlink-io.systemd.Credentials.c +++ b/src/shared/varlink-io.systemd.Credentials.c @@ -63,6 +63,8 @@ static SD_VARLINK_DEFINE_METHOD( SD_VARLINK_DEFINE_INPUT_BY_TYPE(scope, Scope, SD_VARLINK_NULLABLE), SD_VARLINK_FIELD_COMMENT("If the 'user' scope is selected, specifies the numeric UNIX UID of the user the credential is associated with. If not specified this is automatically derived from the UID of the calling user, if that can be determined."), SD_VARLINK_DEFINE_INPUT(uid, SD_VARLINK_INT, SD_VARLINK_NULLABLE), + SD_VARLINK_FIELD_COMMENT("If true allows decryption of credentials encrypted with the null key, if false does not allow it, if unset/null the default depends on whether a TPM device exists and SecureBoot is enabled."), + SD_VARLINK_DEFINE_INPUT(allowNull, SD_VARLINK_BOOL, SD_VARLINK_NULLABLE), VARLINK_DEFINE_POLKIT_INPUT, SD_VARLINK_FIELD_COMMENT("The decrypted plaintext data in Base64 encoding."), SD_VARLINK_DEFINE_OUTPUT(data, SD_VARLINK_STRING, 0)); diff --git a/test/units/TEST-54-CREDS.sh b/test/units/TEST-54-CREDS.sh index 75332892f93..7024adace45 100755 --- a/test/units/TEST-54-CREDS.sh +++ b/test/units/TEST-54-CREDS.sh @@ -499,11 +499,17 @@ cmp /tmp/vlcredsdata /tmp/vlcredsdata2 rm /tmp/vlcredsdata2 varlinkctl call /run/systemd/io.systemd.Credentials io.systemd.Credentials.Encrypt "{\"data\":\"$DATA\",\"withKey\":\"null\"}" | \ + jq '.["allowNull"] = true' | varlinkctl call --json=short /run/systemd/io.systemd.Credentials io.systemd.Credentials.Decrypt > /tmp/vlcredsdata2 cmp /tmp/vlcredsdata /tmp/vlcredsdata2 rm /tmp/vlcredsdata /tmp/vlcredsdata2 +# Ensure allowNull works +(! varlinkctl call /run/systemd/io.systemd.Credentials io.systemd.Credentials.Encrypt "{\"data\":\"$DATA\",\"withKey\":\"null\"}" | \ + jq '.["allowNull"] = false' | + varlinkctl call --json=short /run/systemd/io.systemd.Credentials io.systemd.Credentials.Decrypt ) + clean_usertest() { rm -f /tmp/usertest.data /tmp/usertest.data /tmp/brummbaer.data }