Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add silent autoupgrade of weak cipher to the more secure one when exp… #2295

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Add silent autoupgrade of weak cipher to the more secure one when exp… #2295

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 22 additions & 9 deletions src/lib/pgp-key.cpp
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Loading