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
…2266)

* Do not allow 64 bit ciphers for encryption without explicit option.

* Add old ciphers check on library level

* Fix broken tests(time-machine)

* Fix coverage

* Settle 'mark insecure' date
  • Loading branch information
desvxx authored Nov 28, 2024
1 parent 24895aa commit ddcbaa9
Show file tree
Hide file tree
Showing 14 changed files with 401 additions and 18 deletions.
63 changes: 52 additions & 11 deletions src/lib/rnp.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1161,19 +1161,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;
}

static bool
Expand Down Expand Up @@ -2740,13 +2753,41 @@ try {
}
FFI_GUARD

static bool
rnp_check_old_ciphers(rnp_ffi_t ffi, const char *cipher)
{
uint32_t security_level = 0;

if (rnp_get_security_rule(ffi,
RNP_FEATURE_SYMM_ALG,
cipher,
ffi->context.time(),
NULL,
NULL,
&security_level)) {
FFI_LOG(ffi, "Failed to get security rules for cipher algorithm \'%s\'!", cipher);
return false;
}

if (security_level < RNP_SECURITY_DEFAULT) {
FFI_LOG(ffi, "Cipher algorithm \'%s\' is cryptographically weak!", cipher);
return false;
}
/* TODO: check other weak algorithms and key sizes */
return true;
}

rnp_result_t
rnp_op_encrypt_set_cipher(rnp_op_encrypt_t op, const char *cipher)
try {
// checks
if (!op || !cipher) {
return RNP_ERROR_NULL_POINTER;
}
if (!rnp_check_old_ciphers(op->ffi, cipher)) {
FFI_LOG(op->ffi, "Deprecated cipher: %s", cipher);
return RNP_ERROR_BAD_PARAMETERS;
}
if (!str_to_cipher(cipher, &op->rnpctx.ealg)) {
FFI_LOG(op->ffi, "Invalid cipher: %s", cipher);
return RNP_ERROR_BAD_PARAMETERS;
Expand Down
7 changes: 7 additions & 0 deletions src/lib/sec_profile.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,13 @@ 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*/
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
65 changes: 65 additions & 0 deletions src/rnp/fficli.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -662,6 +662,41 @@ cli_rnp_t::init(const rnp_cfg &cfg)
RNP_SECURITY_DEFAULT);
}

if (cfg_.has(CFG_ALLOW_OLD_CIPHERS)) {
auto now = time(NULL);
uint64_t from = 0;
uint32_t level = 0;
rnp_get_security_rule(ffi, RNP_FEATURE_SYMM_ALG, "CAST5", now, NULL, &from, &level);
rnp_add_security_rule(ffi,
RNP_FEATURE_SYMM_ALG,
"CAST5",
RNP_SECURITY_OVERRIDE,
from,
RNP_SECURITY_DEFAULT);
rnp_get_security_rule(
ffi, RNP_FEATURE_SYMM_ALG, "TRIPLEDES", now, NULL, &from, &level);
rnp_add_security_rule(ffi,
RNP_FEATURE_SYMM_ALG,
"TRIPLEDES",
RNP_SECURITY_OVERRIDE,
from,
RNP_SECURITY_DEFAULT);
rnp_get_security_rule(ffi, RNP_FEATURE_SYMM_ALG, "IDEA", now, NULL, &from, &level);
rnp_add_security_rule(ffi,
RNP_FEATURE_SYMM_ALG,
"IDEA",
RNP_SECURITY_OVERRIDE,
from,
RNP_SECURITY_DEFAULT);
rnp_get_security_rule(ffi, RNP_FEATURE_SYMM_ALG, "BLOWFISH", now, NULL, &from, &level);
rnp_add_security_rule(ffi,
RNP_FEATURE_SYMM_ALG,
"BLOWFISH",
RNP_SECURITY_OVERRIDE,
from,
RNP_SECURITY_DEFAULT);
}

// by default use stdin password provider
if (rnp_ffi_set_pass_provider(ffi, ffi_pass_callback_stdin, this)) {
goto done;
Expand Down Expand Up @@ -2911,6 +2946,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;
}

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\'!",
rnp->cfg().get_cipher().c_str());
return false;
}

if (security_level < RNP_SECURITY_DEFAULT) {
ERR_MSG("Cipher algorithm \'%s\' is cryptographically weak!",
rnp->cfg().get_cipher().c_str());
return false;
}
/* 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 @@ -165,6 +165,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 @@ -235,6 +236,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 @@ -529,6 +531,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;
case OPT_SETFNAME:
cfg.set_str(CFG_SETFNAME, arg);
return true;
Expand Down Expand Up @@ -687,6 +692,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 "
"want to use it.");
return EXIT_ERROR;
}

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;
}

bool
rnp_cfg::get_expiration(const std::string &key, uint32_t &seconds) const
{
Expand Down
8 changes: 6 additions & 2 deletions src/rnp/rnpcfg.h
Original file line number Diff line number Diff line change
Expand Up @@ -96,8 +96,10 @@
#define CFG_ADD_SUBKEY "add-subkey" /* add subkey to existing primary */
#define CFG_SET_KEY_EXPIRE "key-expire" /* set/update key expiration time */
#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_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 @@ -196,6 +198,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 @@ -86,6 +86,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 @@ -142,6 +143,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},
{"allow-sha1-key-sigs", no_argument, NULL, OPT_ALLOW_SHA1},
{"keyfile", required_argument, NULL, OPT_KEYFILE},
Expand Down Expand Up @@ -611,6 +613,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;
case OPT_ADD_SUBKEY:
cfg.set_bool(CFG_ADD_SUBKEY, true);
return true;
Expand Down Expand Up @@ -650,6 +655,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 "
"want to use it.");
return false;
}

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 @@ -57,6 +57,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
30 changes: 28 additions & 2 deletions src/tests/cli_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -354,7 +354,7 @@ def rnp_params_insert_aead(params, pos, aead):

def rnp_encrypt_file_ex(src, dst, recipients=None, passwords=None, aead=None, cipher=None,
z=None, armor=False, s2k_iter=False, s2k_msec=False):
params = ['--homedir', RNPDIR, src, '--output', dst]
params = ['--homedir', RNPDIR, src, '--output', dst, '--allow-old-ciphers']
# Recipients. None disables PK encryption, [] to use default key. Otherwise list of ids.
if recipients != None:
params[2:2] = ['--encrypt']
Expand Down Expand Up @@ -3117,7 +3117,7 @@ def test_alg_aliases(self):
self.assertRegex(out,r'(?s)^.*Symmetric-key encrypted session key packet.*symmetric algorithm: 7 \(AES-128\).*$')
remove_files(enc)
# Encrypt file using the 3DES instead of tripledes
ret, _, err = run_proc(RNP, ['-c', src, '--cipher', '3DES', '--password', 'password'])
ret, _, err = run_proc(RNP, ['-c', src, '--cipher', '3DES', '--password', 'password', "--allow-old-ciphers"])
self.assertEqual(ret, 0)
self.assertNotRegex(err, r'(?s)^.*Warning, unsupported encryption algorithm: 3DES.*$')
self.assertNotRegex(err, r'(?s)^.*Unsupported encryption algorithm: 3DES.*$')
Expand Down Expand Up @@ -4227,6 +4227,32 @@ def test_allow_sha1_key_sigs(self):

clear_workfiles()

def test_allow_old_ciphers(self):
RNP2 = RNPDIR + '2'
os.mkdir(RNP2, 0o700)
if RNP_CAST5:
ret, _, err = run_proc(RNPK, ['--cipher', 'CAST5', '--homedir', RNP2, '--password', 'password', '--current-time', '2030-01-01',
'--userid', 'test_user', '--generate-key'])
self.assertNotEqual(ret, 0)
self.assertRegex(err, r'(?s)^.*Cipher algorithm \'CAST5\' is cryptographically weak!.*Old cipher detected. Pass --allow-old-ciphers option if you really want to use it\..*')
ret, _, err = run_proc(RNPK, ['--cipher', 'CAST5', '--homedir', RNP2, '--password', 'password', '--current-time', '2030-01-01',
'--userid', 'test_user', '--generate-key', '--allow-old-ciphers'])
self.assertEqual(ret, 0)

src, sig = reg_workfiles('cleartext', '.txt', '.sig')
random_text(src, 120)

if RNP_CAST5:
ret, _, err = run_proc(RNP, ['-c', src, '--output', sig, '--cipher', 'CAST5', '--password', 'password', '--current-time', '2030-01-01'])
self.assertNotEqual(ret, 0)
self.assertRegex(err, r'(?s)^.*Cipher algorithm \'CAST5\' is cryptographically weak!.*Old cipher detected. Pass --allow-old-ciphers option if you really want to use it\..*')
ret, _, err = run_proc(RNP, ['-c', src, '--output', sig, '--cipher', 'CAST5', '--password', 'password', '--current-time', '2030-01-01', '--allow-old-ciphers'])
self.assertEqual(ret, 0)

remove_files(sig)
clear_workfiles()
shutil.rmtree(RNP2, ignore_errors=True)

def test_armored_detection_on_cleartext(self):
ret, out, err = run_proc(RNP, ['--keyfile', data_path(SECRING_1), '--password', PASSWORD, '--clearsign'], 'Hello\n')
self.assertEqual(ret, 0)
Expand Down
Loading

0 comments on commit ddcbaa9

Please sign in to comment.