Fix g10 key algorithm extraction and refactor secret key decryption.

src/lib/crypto/ecdsa.cpp

@@ -45,7 +45,8 @@ ecdsa_load_public_key(botan_pubkey_t *pubkey, const pgp_ec_key_t *keydata)
  const size_t curve_order = BITS_TO_BYTES(curve->bitlen);
 
  if (!mpi_bytes(&keydata->p) || (keydata->p.mpi[0] != 0x04)) {
- RNP_LOG("Failed to load public key");
+ RNP_LOG(
+ "Failed to load public key: %zu, %02x", mpi_bytes(&keydata->p), keydata->p.mpi[0]);
  return false;
  }
 

src/lib/pgp-key.cpp

@@ -74,15 +74,14 @@
 #include "defaults.h"
 
 pgp_key_pkt_t *
-pgp_decrypt_seckey_pgp(const uint8_t * data,
- size_t data_len,
- const pgp_key_pkt_t *pubkey,
- const char * password)
+pgp_decrypt_seckey_pgp(const pgp_rawpacket_t &raw,
+ const pgp_key_pkt_t & pubkey,
+ const char * password)
 {
  pgp_source_t src = {0};
  pgp_key_pkt_t *res = NULL;
 
- if (init_mem_src(&src, data, data_len, false)) {
+ if (init_mem_src(&src, raw.raw.data(), raw.raw.size(), false)) {
  return NULL;
  }
  try {
@@ -113,41 +112,32 @@ pgp_decrypt_seckey_pgp(const uint8_t * data,
  * key->key.seckey by parsing the key data in packets[0].
  */
 pgp_key_pkt_t *
-pgp_decrypt_seckey(const pgp_key_t * key,
- const pgp_password_provider_t *provider,
- const pgp_password_ctx_t * ctx)
+pgp_decrypt_seckey(const pgp_key_t & key,
+ const pgp_password_provider_t &provider,
+ const pgp_password_ctx_t & ctx)
 {
- typedef struct pgp_key_pkt_t *pgp_seckey_decrypt_t(
- const uint8_t *data, size_t data_len, const pgp_key_pkt_t *pubkey, const char *password);
- pgp_seckey_decrypt_t *decryptor = NULL;
-
  // sanity checks
- if (!key || !key->is_secret() || !provider) {
+ if (!key.is_secret()) {
  RNP_LOG("invalid args");
  return NULL;
  }
- switch (key->format) {
+ // ask the provider for a password
+ rnp::secure_array<char, MAX_PASSWORD_LENGTH> password;
+ if (key.is_protected() &&
+ !pgp_request_password(&provider, &ctx, password.data(), password.size())) {
+ return NULL;
+ }
+ // attempt to decrypt with the provided password
+ switch (key.format) {
  case PGP_KEY_STORE_GPG:
  case PGP_KEY_STORE_KBX:
- decryptor = pgp_decrypt_seckey_pgp;
- break;
+ return pgp_decrypt_seckey_pgp(key.rawpkt(), key.pkt(), password.data());
  case PGP_KEY_STORE_G10:
- decryptor = g10_decrypt_seckey;
- break;
+ return g10_decrypt_seckey(key.rawpkt(), key.pkt(), password.data());
  default:
- RNP_LOG("unexpected format: %d", key->format);
+ RNP_LOG("unexpected format: %d", key.format);
  return NULL;
  }
-
- // ask the provider for a password
- rnp::secure_array<char, MAX_PASSWORD_LENGTH> password;
- if (key->is_protected() &&
- !pgp_request_password(provider, ctx, password.data(), password.size())) {
- return NULL;
- }
- // attempt to decrypt with the provided password
- const pgp_rawpacket_t &pkt = key->rawpkt();
- return decryptor(pkt.raw.data(), pkt.raw.size(), &key->pkt(), password.data());
 }
 
 pgp_key_t *
@@ -1512,7 +1502,7 @@ pgp_key_t::unlock(const pgp_password_provider_t &provider, pgp_op_t op)
  }
 
  pgp_password_ctx_t ctx = {.op = (uint8_t) op, .key = this};
- pgp_key_pkt_t * decrypted_seckey = pgp_decrypt_seckey(this, &provider, &ctx);
+ pgp_key_pkt_t * decrypted_seckey = pgp_decrypt_seckey(*this, provider, ctx);
  if (!decrypted_seckey) {
  return false;
  }
@@ -1626,7 +1616,7 @@ pgp_key_t::unprotect(const pgp_password_provider_t &password_provider, rnp::RNG
  ctx.op = PGP_OP_UNPROTECT;
  ctx.key = this;
 
- pgp_key_pkt_t *decrypted_seckey = pgp_decrypt_seckey(this, &password_provider, &ctx);
+ pgp_key_pkt_t *decrypted_seckey = pgp_decrypt_seckey(*this, password_provider, ctx);
  if (!decrypted_seckey) {
  return false;
  }

src/lib/pgp-key.h

@@ -576,14 +576,13 @@ class KeyLocker {
 };
 }; // namespace rnp
 
-pgp_key_pkt_t *pgp_decrypt_seckey_pgp(const uint8_t *,
- size_t,
- const pgp_key_pkt_t *,
- const char *);
-
-pgp_key_pkt_t *pgp_decrypt_seckey(const pgp_key_t *,
- const pgp_password_provider_t *,
- const pgp_password_ctx_t *);
+pgp_key_pkt_t *pgp_decrypt_seckey_pgp(const pgp_rawpacket_t &raw,
+ const pgp_key_pkt_t & key,
+ const char * password);
+
+pgp_key_pkt_t *pgp_decrypt_seckey(const pgp_key_t &,
+ const pgp_password_provider_t &,
+ const pgp_password_ctx_t &);
 
 /**
  * @brief Get the signer's key for signature

src/lib/rnp.cpp

@@ -7130,7 +7130,7 @@ try {
  const std::string pass = password;
  if (key->encrypted()) {
  pgp_password_ctx_t ctx = {.op = PGP_OP_PROTECT, .key = key};
- decrypted_key = pgp_decrypt_seckey(key, &handle->ffi->pass_provider, &ctx);
+ decrypted_key = pgp_decrypt_seckey(*key, handle->ffi->pass_provider, ctx);
  if (!decrypted_key) {
  return RNP_ERROR_GENERIC;
  }

src/librekey/key_store_g10.cpp

@@ -874,20 +874,21 @@ g10_parse_seckey(pgp_key_pkt_t &seckey,
 }
 
 pgp_key_pkt_t *
-g10_decrypt_seckey(const uint8_t * data,
- size_t data_len,
- const pgp_key_pkt_t *pubkey,
- const char * password)
+g10_decrypt_seckey(const pgp_rawpacket_t &raw,
+ const pgp_key_pkt_t & pubkey,
+ const char * password)
 {
  if (!password) {
  return NULL;
  }
-
- auto seckey = std::unique_ptr<pgp_key_pkt_t>(pubkey ? new pgp_key_pkt_t(*pubkey, false) :
- new pgp_key_pkt_t());
- if (!g10_parse_seckey(*seckey, data, data_len, password)) {
+ auto seckey = std::unique_ptr<pgp_key_pkt_t>(new pgp_key_pkt_t(pubkey, false));
+ if (!g10_parse_seckey(*seckey, raw.raw.data(), raw.raw.size(), password)) {
  return NULL;
  }
+ /* g10 has the same 'ecc' algo for ECDSA/ECDH/EDDSA. Probably should be better place to fix
+ * this. */
+ seckey->alg = pubkey.alg;
+ seckey->material.alg = pubkey.material.alg;
  return seckey.release();
 }
 

src/librekey/key_store_g10.h

@@ -35,9 +35,8 @@ bool g10_write_seckey(pgp_dest_t * dst,
  pgp_key_pkt_t *seckey,
  const char * password,
  rnp::RNG & rng);
-pgp_key_pkt_t *g10_decrypt_seckey(const uint8_t * data,
- size_t data_len,
- const pgp_key_pkt_t *pubkey,
- const char * password);
+pgp_key_pkt_t *g10_decrypt_seckey(const pgp_rawpacket_t &raw,
+ const pgp_key_pkt_t & pubkey,
+ const char * password);
 
 #endif // RNP_KEY_STORE_G10_H

src/librepgp/stream-parse.cpp

@@ -2096,7 +2096,7 @@ init_encrypted_src(pgp_parse_handler_t *handler, pgp_source_t *src, pgp_source_t
  if (seckey->encrypted()) {
  pgp_password_ctx_t pass_ctx{.op = PGP_OP_DECRYPT, .key = seckey};
  decrypted_seckey =
- pgp_decrypt_seckey(seckey, handler->password_provider, &pass_ctx);
+ pgp_decrypt_seckey(*seckey, *handler->password_provider, pass_ctx);
  if (!decrypted_seckey) {
  errcode = RNP_ERROR_BAD_PASSWORD;
  continue;

src/tests/load-pgp.cpp

@@ -83,9 +83,7 @@ TEST_F(rnp_tests, test_load_v3_keyring_pgp)
  assert_true(key->is_locked());
 
  // decrypt the key
- const pgp_rawpacket_t &pkt = key->rawpkt();
- pgp_key_pkt_t * seckey =
- pgp_decrypt_seckey_pgp(pkt.raw.data(), pkt.raw.size(), &key->pkt(), "password");
+ pgp_key_pkt_t *seckey = pgp_decrypt_seckey_pgp(key->rawpkt(), key->pkt(), "password");
  assert_non_null(seckey);
 
  // cleanup