Skip to content

Commit

Permalink
Do not allow 64 bit ciphers for encryption without explicit option.
Browse files Browse the repository at this point in the history
  • Loading branch information
desvxx committed Aug 29, 2024
1 parent 08e277d commit 32242b1
Show file tree
Hide file tree
Showing 10 changed files with 101 additions and 14 deletions.
35 changes: 24 additions & 11 deletions src/lib/rnp.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1144,19 +1144,32 @@ get_feature_sec_value(
rnp_ffi_t ffi, const char *stype, const char *sname, rnp::FeatureType &type, int &value)
{
/* check type */
if (!rnp::str_case_eq(stype, RNP_FEATURE_HASH_ALG)) {
FFI_LOG(ffi, "Unsupported feature type: %s", stype);
return false;
if (rnp::str_case_eq(stype, RNP_FEATURE_HASH_ALG)) {
type = rnp::FeatureType::Hash;
/* check feature name */
pgp_hash_alg_t alg = PGP_HASH_UNKNOWN;
if (sname && !str_to_hash_alg(sname, &alg)) {
FFI_LOG(ffi, "Unknown hash algorithm: %s", sname);
return false;
}
value = alg;
return true;
}
type = rnp::FeatureType::Hash;
/* check feature name */
pgp_hash_alg_t alg = PGP_HASH_UNKNOWN;
if (sname && !str_to_hash_alg(sname, &alg)) {
FFI_LOG(ffi, "Unknown hash algorithm: %s", sname);
return false;

if (rnp::str_case_eq(stype, RNP_FEATURE_SYMM_ALG)) {
type = rnp::FeatureType::Cipher;
/* check feature name */
pgp_symm_alg_t alg = PGP_SA_UNKNOWN;
if (sname && !str_to_cipher(sname, &alg)) {
FFI_LOG(ffi, "Unknown cipher: %s", sname);
return false;
}
value = alg;
return true;
}
value = alg;
return true;

FFI_LOG(ffi, "Unsupported feature type: %s", stype);
return false;

Check warning on line 1172 in src/lib/rnp.cpp

View check run for this annotation

Codecov / codecov/patch

src/lib/rnp.cpp#L1171-L1172

Added lines #L1171 - L1172 were not covered by tests
}

static bool
Expand Down
5 changes: 5 additions & 0 deletions src/lib/sec_profile.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,11 @@ SecurityContext::SecurityContext() : time_(0), prov_state_(NULL), rng(RNG::Type:
SecurityAction::VerifyKey});
/* Mark MD5 insecure since 2012-01-01 */
profile.add_rule({FeatureType::Hash, PGP_HASH_MD5, SecurityLevel::Insecure, 1325376000});
/* Mark CAST5, 3DES, IDEA, BLOWFISH insecure since 2024-10-01*/ // TODO: tbd
profile.add_rule({FeatureType::Cipher, PGP_SA_CAST5, SecurityLevel::Insecure, 1727730000});
profile.add_rule({FeatureType::Cipher, PGP_SA_TRIPLEDES, SecurityLevel::Insecure, 1727730000});
profile.add_rule({FeatureType::Cipher, PGP_SA_IDEA, SecurityLevel::Insecure, 1727730000});
profile.add_rule({FeatureType::Cipher, PGP_SA_BLOWFISH, SecurityLevel::Insecure, 1727730000});
}

SecurityContext::~SecurityContext()
Expand Down
30 changes: 30 additions & 0 deletions src/rnp/fficli.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2898,6 +2898,36 @@ cli_rnp_check_weak_hash(cli_rnp_t *rnp)
return true;
}

bool
cli_rnp_check_old_ciphers(cli_rnp_t *rnp)
{
if (rnp->cfg().has(CFG_ALLOW_OLD_CIPHERS)) {
return true;

Check warning on line 2905 in src/rnp/fficli.cpp

View check run for this annotation

Codecov / codecov/patch

src/rnp/fficli.cpp#L2905

Added line #L2905 was not covered by tests
}

uint32_t security_level = 0;

if (rnp_get_security_rule(rnp->ffi,
RNP_FEATURE_SYMM_ALG,
rnp->cfg().get_cipher().c_str(),
rnp->cfg().time(),
NULL,
NULL,
&security_level)) {
ERR_MSG("Failed to get security rules for cipher algorithm \'%s\'!",

Check warning on line 2917 in src/rnp/fficli.cpp

View check run for this annotation

Codecov / codecov/patch

src/rnp/fficli.cpp#L2917

Added line #L2917 was not covered by tests
rnp->cfg().get_cipher().c_str());
return false;

Check warning on line 2919 in src/rnp/fficli.cpp

View check run for this annotation

Codecov / codecov/patch

src/rnp/fficli.cpp#L2919

Added line #L2919 was not covered by tests
}

if (security_level < RNP_SECURITY_DEFAULT) {
ERR_MSG("Cipher algorithm \'%s\' is cryptographically weak!",

Check warning on line 2923 in src/rnp/fficli.cpp

View check run for this annotation

Codecov / codecov/patch

src/rnp/fficli.cpp#L2923

Added line #L2923 was not covered by tests
rnp->cfg().get_cipher().c_str());
return false;

Check warning on line 2925 in src/rnp/fficli.cpp

View check run for this annotation

Codecov / codecov/patch

src/rnp/fficli.cpp#L2925

Added line #L2925 was not covered by tests
}
/* TODO: check other weak algorithms and key sizes */
return true;
}

bool
cli_rnp_protect_file(cli_rnp_t *rnp)
{
Expand Down
1 change: 1 addition & 0 deletions src/rnp/fficli.h
Original file line number Diff line number Diff line change
Expand Up @@ -233,6 +233,7 @@ bool cli_rnp_dump_file(cli_rnp_t *rnp);
bool cli_rnp_armor_file(cli_rnp_t *rnp);
bool cli_rnp_dearmor_file(cli_rnp_t *rnp);
bool cli_rnp_check_weak_hash(cli_rnp_t *rnp);
bool cli_rnp_check_old_ciphers(cli_rnp_t *rnp);
bool cli_rnp_setup(cli_rnp_t *rnp);
bool cli_rnp_protect_file(cli_rnp_t *rnp);
bool cli_rnp_process_file(cli_rnp_t *rnp);
Expand Down
11 changes: 11 additions & 0 deletions src/rnp/rnp.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,7 @@ enum optdefs {
OPT_SOURCE,
OPT_NOWRAP,
OPT_CURTIME,
OPT_ALLOW_OLD_CIPHERS,
OPT_SETFNAME,
OPT_ALLOW_HIDDEN,
OPT_S2K_ITER,
Expand Down Expand Up @@ -233,6 +234,7 @@ static struct option options[] = {
{"source", required_argument, NULL, OPT_SOURCE},
{"no-wrap", no_argument, NULL, OPT_NOWRAP},
{"current-time", required_argument, NULL, OPT_CURTIME},
{"allow-old-ciphers", no_argument, NULL, OPT_ALLOW_OLD_CIPHERS},
{"set-filename", required_argument, NULL, OPT_SETFNAME},
{"allow-hidden", no_argument, NULL, OPT_ALLOW_HIDDEN},
{"s2k-iterations", required_argument, NULL, OPT_S2K_ITER},
Expand Down Expand Up @@ -523,6 +525,9 @@ setoption(rnp_cfg &cfg, int val, const char *arg)
case OPT_CURTIME:
cfg.set_str(CFG_CURTIME, arg);
return true;
case OPT_ALLOW_OLD_CIPHERS:
cfg.set_bool(CFG_ALLOW_OLD_CIPHERS, true);
return true;

Check warning on line 530 in src/rnp/rnp.cpp

View check run for this annotation

Codecov / codecov/patch

src/rnp/rnp.cpp#L528-L530

Added lines #L528 - L530 were not covered by tests
case OPT_SETFNAME:
cfg.set_str(CFG_SETFNAME, arg);
return true;
Expand Down Expand Up @@ -681,6 +686,12 @@ rnp_main(int argc, char **argv)
return EXIT_ERROR;
}

if (!cli_rnp_check_old_ciphers(&rnp)) {
ERR_MSG("Old cipher detected. Pass --allow-old-ciphers option if you really "

Check warning on line 690 in src/rnp/rnp.cpp

View check run for this annotation

Codecov / codecov/patch

src/rnp/rnp.cpp#L690

Added line #L690 was not covered by tests
"want to use it.");
return EXIT_ERROR;

Check warning on line 692 in src/rnp/rnp.cpp

View check run for this annotation

Codecov / codecov/patch

src/rnp/rnp.cpp#L692

Added line #L692 was not covered by tests
}

bool disable_ks = rnp.cfg().get_bool(CFG_KEYSTORE_DISABLED);
if (!disable_ks && !rnp.load_keyrings(rnp.cfg().get_bool(CFG_NEEDSSECKEY))) {
ERR_MSG("fatal: failed to load keys");
Expand Down
10 changes: 10 additions & 0 deletions src/rnp/rnpcfg.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -312,6 +312,16 @@ rnp_cfg::get_hashalg() const
return DEFAULT_HASH_ALG;
}

const std::string
rnp_cfg::get_cipher() const
{
const std::string cipher_alg = get_str(CFG_CIPHER);
if (!cipher_alg.empty()) {
return cipher_alg;
}
return DEFAULT_SYMM_ALG;

Check warning on line 322 in src/rnp/rnpcfg.cpp

View check run for this annotation

Codecov / codecov/patch

src/rnp/rnpcfg.cpp#L322

Added line #L322 was not covered by tests
}

bool
rnp_cfg::get_expiration(const std::string &key, uint32_t &seconds) const
{
Expand Down
3 changes: 3 additions & 0 deletions src/rnp/rnpcfg.h
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@
#define CFG_SOURCE "source" /* source for the detached signature */
#define CFG_NOWRAP "no-wrap" /* do not wrap the output in a literal data packet */
#define CFG_CURTIME "curtime" /* date or timestamp to override the system's time */
#define CFG_ALLOW_OLD_CIPHERS "allow-old-ciphers" /* Allow to use 64-bit ciphers (CAST5, 3DES, IDEA, BLOWFISH) */
#define CFG_ALLOW_HIDDEN "allow-hidden" /* allow hidden recipients */

/* rnp keyring setup variables */
Expand Down Expand Up @@ -195,6 +196,8 @@ class rnp_cfg {
int get_pswdtries() const;
/** @brief get hash algorithm */
const std::string get_hashalg() const;
/** @brief get cipher algorithm */
const std::string get_cipher() const;

/** @brief Get expiration time from the cfg variable, as value relative to the current
* time. As per OpenPGP standard it should fit in 32 bit value, otherwise error is
Expand Down
11 changes: 11 additions & 0 deletions src/rnpkeys/rnpkeys.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ const char *usage =
" --overwrite Overwrite output file without a prompt.\n"
" --notty Do not write anything to the TTY.\n"
" --current-time Override system's time.\n"
" --allow-old-ciphers Allow to use 64-bit ciphers (CAST5, 3DES, IDEA, BLOWFISH).\n"
"\n"
"See man page for a detailed listing and explanation.\n"
"\n";
Expand Down Expand Up @@ -141,6 +142,7 @@ struct option options[] = {
{"add-subkey", no_argument, NULL, OPT_ADD_SUBKEY},
{"set-expire", required_argument, NULL, OPT_SET_EXPIRE},
{"current-time", required_argument, NULL, OPT_CURTIME},
{"allow-old-ciphers", no_argument, NULL, OPT_ALLOW_OLD_CIPHERS},
{"allow-weak-hash", no_argument, NULL, OPT_ALLOW_WEAK_HASH},
{"keyfile", required_argument, NULL, OPT_KEYFILE},
{NULL, 0, NULL, 0},
Expand Down Expand Up @@ -606,6 +608,9 @@ setoption(rnp_cfg &cfg, optdefs_t *cmd, int val, const char *arg)
case OPT_CURTIME:
cfg.set_str(CFG_CURTIME, arg);
return true;
case OPT_ALLOW_OLD_CIPHERS:
cfg.set_bool(CFG_ALLOW_OLD_CIPHERS, true);
return true;

Check warning on line 613 in src/rnpkeys/rnpkeys.cpp

View check run for this annotation

Codecov / codecov/patch

src/rnpkeys/rnpkeys.cpp#L611-L613

Added lines #L611 - L613 were not covered by tests
case OPT_ADD_SUBKEY:
cfg.set_bool(CFG_ADD_SUBKEY, true);
return true;
Expand Down Expand Up @@ -645,6 +650,12 @@ rnpkeys_init(cli_rnp_t &rnp, const rnp_cfg &cfg)
return false;
}

if (!cli_rnp_check_old_ciphers(&rnp)) {
ERR_MSG("Old cipher detected. Pass --allow-old-ciphers option if you really "

Check warning on line 654 in src/rnpkeys/rnpkeys.cpp

View check run for this annotation

Codecov / codecov/patch

src/rnpkeys/rnpkeys.cpp#L654

Added line #L654 was not covered by tests
"want to use it.");
return false;

Check warning on line 656 in src/rnpkeys/rnpkeys.cpp

View check run for this annotation

Codecov / codecov/patch

src/rnpkeys/rnpkeys.cpp#L656

Added line #L656 was not covered by tests
}

bool disable_ks = rnp.cfg().get_bool(CFG_KEYSTORE_DISABLED);
if (!disable_ks && !rnp.load_keyrings(true)) {
ERR_MSG("fatal: failed to load keys");
Expand Down
1 change: 1 addition & 0 deletions src/rnpkeys/rnpkeys.h
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ typedef enum {
OPT_FIX_25519_BITS,
OPT_CHK_25519_BITS,
OPT_CURTIME,
OPT_ALLOW_OLD_CIPHERS,
OPT_ADD_SUBKEY,
OPT_SET_EXPIRE,
OPT_KEYFILE,
Expand Down
8 changes: 5 additions & 3 deletions src/tests/ffi.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5793,7 +5793,7 @@ TEST_F(rnp_tests, test_ffi_security_profile)
assert_rnp_failure(
rnp_get_security_rule(NULL, RNP_FEATURE_HASH_ALG, "SHA1", 0, &flags, &from, &level));
assert_rnp_failure(rnp_get_security_rule(ffi, NULL, "SHA1", 0, &flags, &from, &level));
assert_rnp_failure(
assert_rnp_success(
rnp_get_security_rule(ffi, RNP_FEATURE_SYMM_ALG, "AES256", 0, &flags, &from, &level));
assert_rnp_failure(
rnp_get_security_rule(ffi, RNP_FEATURE_HASH_ALG, "Unknown", 0, &flags, &from, &level));
Expand Down Expand Up @@ -5954,12 +5954,14 @@ TEST_F(rnp_tests, test_ffi_security_profile)
removed = 0;
assert_rnp_failure(rnp_remove_security_rule(ffi, NULL, NULL, 0, 0x17, 0, &removed));
assert_rnp_success(rnp_remove_security_rule(ffi, NULL, NULL, 0, 0, 0, &removed));
assert_int_equal(removed, 3);
assert_int_equal(removed, 3 /*HASH*/ + 4 /*SYMM*/);
rnp_ffi_destroy(ffi);
rnp_ffi_create(&ffi, "GPG", "GPG");
/* Remove all rules for hash */
assert_rnp_failure(
removed = 0;
assert_rnp_success(
rnp_remove_security_rule(ffi, RNP_FEATURE_SYMM_ALG, NULL, 0, 0, 0, &removed));
assert_int_equal(removed, 4);
removed = 0;
assert_rnp_success(
rnp_remove_security_rule(ffi, RNP_FEATURE_HASH_ALG, NULL, 0, 0, 0, &removed));
Expand Down

0 comments on commit 32242b1

Please sign in to comment.