Skip to content

Commit

Permalink
Add autoupgrade of weak cipher to the more secure one when expiration…
Browse files Browse the repository at this point in the history 
… date changes
  • Loading branch information
@desvxx
desvxx committed Dec 4, 2024
1 parent ddcbaa9 commit cd8d936
Show file tree
Hide file tree
Showing 3 changed files with 104 additions and 10 deletions.
31 changes: 22 additions & 9 deletions src/lib/pgp-key.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -271,10 +271,23 @@ static bool
update_sig_expiration(pgp_signature_t * dst,
const pgp_signature_t *src,
uint64_t create,
uint32_t expiry)
uint32_t expiry,
rnp::SecurityContext & ctx)
{
try {
*dst = *src;
// Upgrade old hashes to the more secure one
rnp::SecurityRule rule(rnp::FeatureType::Hash, dst->halg, ctx.profile.def_level());
if (ctx.profile.has_rule(
rnp::FeatureType::Hash, dst->halg, ctx.time(), rnp::SecurityAction::Any)) {
rule = ctx.profile.get_rule(
rnp::FeatureType::Hash, dst->halg, ctx.time(), rnp::SecurityAction::Any);
}

if (rule.level != rnp::SecurityLevel::Default) {
RNP_LOG("Warning: Weak hash algorithm, authomatically upgrading to SHA256");
dst->halg = PGP_HASH_SHA256;
}
if (!expiry) {
dst->remove_subpkt(dst->find_subpkt(pgp::pkt::sigsub::Type::KeyExpirationTime));
;
Expand Down Expand Up @@ -304,12 +317,12 @@ pgp_key_set_expiration(pgp_key_t * key,
std::vector<pgp_sig_id_t> sigs;
/* update expiration for the latest direct-key signature and self-signature for each userid
*/
pgp_subsig_t *sig = key->latest_selfsig(PGP_UID_NONE);
pgp_subsig_t *sig = key->latest_selfsig(PGP_UID_NONE, false);
if (sig) {
sigs.push_back(sig->sigid);
}
for (size_t uid = 0; uid < key->uid_count(); uid++) {
sig = key->latest_selfsig(uid);
sig = key->latest_selfsig(uid, false);
if (sig) {
sigs.push_back(sig->sigid);
}
Expand All @@ -335,7 +348,7 @@ pgp_key_set_expiration(pgp_key_t * key,

pgp_signature_t newsig;
pgp_sig_id_t oldsigid = sigid;
if (!update_sig_expiration(&newsig, &sig.sig, ctx.time(), expiry)) {
if (!update_sig_expiration(&newsig, &sig.sig, ctx.time(), expiry, ctx)) {
return false;
}
try {
Expand Down Expand Up @@ -387,7 +400,7 @@ pgp_subkey_set_expiration(pgp_key_t * sub,
}

/* find the latest valid subkey binding */
pgp_subsig_t *subsig = sub->latest_binding();
pgp_subsig_t *subsig = sub->latest_binding(false);
if (!subsig) {
RNP_LOG("No valid subkey binding");
return false;
Expand All @@ -412,7 +425,7 @@ pgp_subkey_set_expiration(pgp_key_t * sub,
/* update signature and re-sign */
pgp_signature_t newsig;
pgp_sig_id_t oldsigid = subsig->sigid;
if (!update_sig_expiration(&newsig, &subsig->sig, ctx.time(), expiry)) {
if (!update_sig_expiration(&newsig, &subsig->sig, ctx.time(), expiry, ctx)) {
return false;
}
primsec->sign_subkey_binding(*secsub, newsig, ctx);
Expand Down Expand Up @@ -1766,14 +1779,14 @@ pgp_key_t::write_vec() const
#define PGP_UID_ANY ((uint32_t) -3)

pgp_subsig_t *
pgp_key_t::latest_selfsig(uint32_t uid)
pgp_key_t::latest_selfsig(uint32_t uid, bool validated)
{
uint32_t latest = 0;
pgp_subsig_t *res = nullptr;

for (auto &sigid : sigs_) {
auto &sig = get_sig(sigid);
if (!sig.valid()) {
if (validated && !sig.valid()) {
continue;
}
bool skip = false;
Expand Down Expand Up @@ -1807,7 +1820,7 @@ pgp_key_t::latest_selfsig(uint32_t uid)

/* if there is later self-sig for the same uid without primary flag, then drop res */
if ((uid == PGP_UID_PRIMARY) && res) {
pgp_subsig_t *overres = latest_selfsig(res->uid);
pgp_subsig_t *overres = latest_selfsig(res->uid, validated);
if (overres && (overres->sig.creation() > res->sig.creation())) {
res = nullptr;
}
Expand Down
3 changes: 2 additions & 1 deletion src/lib/pgp-key.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -366,9 +366,10 @@ struct pgp_key_t {
* PGP_UID_NONE for direct-key signature,
* PGP_UID_PRIMARY for any primary key,
* PGP_UID_ANY for any uid.
* @param validated set to true whether signature must be validated
* @return pointer to signature object or NULL if failed/not found.
*/
pgp_subsig_t *latest_selfsig(uint32_t uid);
pgp_subsig_t *latest_selfsig(uint32_t uid, bool validated = true);

/**
* @brief Get the latest valid subkey binding. Should be called on subkey.
Expand Down
80 changes: 80 additions & 0 deletions src/tests/ffi-key-prop.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -852,6 +852,86 @@ TEST_F(rnp_tests, test_ffi_key_set_expiry)
assert_rnp_success(rnp_ffi_destroy(ffi));
}

TEST_F(rnp_tests, test_ffi_key_upgrade_hash_on_set_expiry)
{
rnp_ffi_t ffi = NULL;

assert_rnp_success(rnp_ffi_create(&ffi, "GPG", "GPG"));

assert_true(import_pub_keys(ffi, "data/keyrings/7/pubring.gpg"));
assert_true(import_sec_keys(ffi, "data/keyrings/7/secring.gpg"));

rnp_key_handle_t key = NULL;
rnp_key_handle_t sub = NULL;
uint32_t expiry = 0;
const uint32_t new_expiry = 10 * 365 * 24 * 60 * 60;

assert_rnp_success(rnp_locate_key(ffi, "keyid", "07f90cc9ea074d53", &key));
assert_rnp_success(rnp_key_get_expiration(key, &expiry));
assert_int_equal(expiry, 0);
assert_int_equal(key->pub->get_sig(0).sig.halg, PGP_HASH_SHA1);
assert_rnp_success(rnp_key_set_expiration(key, new_expiry));
assert_rnp_success(rnp_key_get_expiration(key, &expiry));
assert_int_equal(expiry, new_expiry);
assert_int_equal(key->pub->get_sig(0).sig.halg, PGP_HASH_SHA256);

assert_rnp_success(rnp_locate_key(ffi, "keyid", "0265f8e2594f8e7b", &sub));
assert_rnp_success(rnp_key_get_expiration(sub, &expiry));
assert_int_equal(expiry, 0);
assert_int_equal(sub->pub->get_sig(0).sig.halg, PGP_HASH_SHA1);
assert_rnp_success(rnp_key_set_expiration(sub, new_expiry));
assert_rnp_success(rnp_key_get_expiration(sub, &expiry));
assert_int_equal(expiry, new_expiry);
assert_int_equal(sub->pub->get_sig(0).sig.halg, PGP_HASH_SHA256);

assert_rnp_success(rnp_key_handle_destroy(key));
assert_rnp_success(rnp_key_handle_destroy(sub));

rnp_ffi_destroy(ffi);

/* allow SHA1 and check that hash alg is not changed */

assert_rnp_success(rnp_ffi_create(&ffi, "GPG", "GPG"));

// Allow SHA1
auto now = time(NULL);
uint64_t from = 0;
uint32_t level = 0;
rnp_get_security_rule(ffi, RNP_FEATURE_HASH_ALG, "SHA1", now, NULL, &from, &level);
rnp_add_security_rule(ffi,
RNP_FEATURE_HASH_ALG,
"SHA1",
RNP_SECURITY_OVERRIDE | RNP_SECURITY_VERIFY_KEY,
from,
RNP_SECURITY_DEFAULT);

assert_true(import_pub_keys(ffi, "data/keyrings/7/pubring.gpg"));
assert_true(import_sec_keys(ffi, "data/keyrings/7/secring.gpg"));

assert_rnp_success(rnp_locate_key(ffi, "keyid", "07f90cc9ea074d53", &key));
assert_rnp_success(rnp_key_get_expiration(key, &expiry));
assert_int_equal(expiry, 0);
assert_int_equal(key->pub->get_sig(0).sig.halg, PGP_HASH_SHA1);
assert_rnp_success(rnp_key_set_expiration(key, new_expiry));
assert_rnp_success(rnp_key_get_expiration(key, &expiry));
assert_int_equal(expiry, new_expiry);
assert_int_equal(key->pub->get_sig(0).sig.halg, PGP_HASH_SHA1);

assert_rnp_success(rnp_locate_key(ffi, "keyid", "0265f8e2594f8e7b", &sub));
assert_rnp_success(rnp_key_get_expiration(sub, &expiry));
assert_int_equal(expiry, 0);
assert_int_equal(sub->pub->get_sig(0).sig.halg, PGP_HASH_SHA1);
assert_rnp_success(rnp_key_set_expiration(sub, new_expiry));
assert_rnp_success(rnp_key_get_expiration(sub, &expiry));
assert_int_equal(expiry, new_expiry);
assert_int_equal(sub->pub->get_sig(0).sig.halg, PGP_HASH_SHA1);

assert_rnp_success(rnp_key_handle_destroy(key));
assert_rnp_success(rnp_key_handle_destroy(sub));

rnp_ffi_destroy(ffi);
}

TEST_F(rnp_tests, test_ffi_key_get_protection_info)
{
rnp_ffi_t ffi = NULL;
Expand Down

0 comments on commit cd8d936

Please sign in to comment.