Skip to content

Commit

Permalink
Add silent autoupgrade of weak cipher to the more secure one when exp…
Browse files Browse the repository at this point in the history 
…iration date changes
  • Loading branch information
@desvxx
desvxx committed Dec 4, 2024
1 parent ddcbaa9 commit a426e64
Show file tree
Hide file tree
Showing 3 changed files with 98 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
74 changes: 74 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,80 @@ 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);

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

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 a426e64

Please sign in to comment.