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.

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

Refactor encrypted_add_password(). #2309

Merged
merged 9 commits into from
Jan 29, 2025
2 changes: 1 addition & 1 deletion .github/workflows/codeql.yml
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ jobs:
if: ${{ matrix.language == 'cpp' }}
run: |
sudo apt-get update
sudo apt-get install --yes libjson-c-dev libgtest-dev
sudo apt-get install --yes libbz2-dev libjson-c-dev libgtest-dev

- name: After Prepare (cpp)
if: ${{ matrix.language == 'cpp' }}
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/coverity.yml
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ jobs:
- name: Install dependencies
run: |
sudo apt-get -y update
sudo apt-get -y install cmake libjson-c-dev libbotan-2-dev asciidoctor
sudo apt-get -y install cmake libbz2-dev libjson-c-dev libbotan-2-dev asciidoctor

- name: Configure
run: |
Expand Down
12 changes: 6 additions & 6 deletions .github/workflows/ubuntu.yml
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ jobs:
# Already installed on GHA: build-essential libbz2-dev zlib1g-dev
run: |
sudo apt-get -y update
sudo apt-get -y install cmake libjson-c-dev ${{ matrix.backend.package }} asciidoctor
sudo apt-get -y install cmake libbz2-dev libjson-c-dev ${{ matrix.backend.package }} asciidoctor

- name: Configure
run: |
Expand Down Expand Up @@ -133,7 +133,7 @@ jobs:
- name: Install dependencies
run: |
sudo apt-get -y update
sudo apt-get -y install cmake libjson-c-dev libbotan-2-dev asciidoctor googletest
sudo apt-get -y install cmake libbz2-dev libjson-c-dev libbotan-2-dev asciidoctor googletest

- name: Configure
run: |
Expand Down Expand Up @@ -174,7 +174,7 @@ jobs:
- name: Install dependencies
run: |
sudo apt-get -y update
sudo apt-get -y install cmake libjson-c-dev libbotan-2-dev asciidoctor googletest
sudo apt-get -y install cmake libbz2-dev libjson-c-dev libbotan-2-dev asciidoctor googletest

- name: Build googletest
run: |
Expand Down Expand Up @@ -221,7 +221,7 @@ jobs:
- name: Install dependencies
run: |
sudo apt-get -y update
sudo apt-get -y install cmake libjson-c-dev libbotan-2-dev asciidoctor
sudo apt-get -y install cmake libbz2-dev libjson-c-dev libbotan-2-dev asciidoctor

- name: Checkout sexpp
uses: actions/checkout@v4
Expand Down Expand Up @@ -286,7 +286,7 @@ jobs:
- name: Install dependencies
run: |
sudo apt-get -y update
sudo apt-get -y install cmake libjson-c-dev libbotan-2-dev asciidoctor
sudo apt-get -y install cmake libbz2-dev libjson-c-dev libbotan-2-dev asciidoctor

- name: Configure
run: |
Expand Down Expand Up @@ -323,7 +323,7 @@ jobs:
- name: Install dependencies
run: |
sudo apt-get -y update
sudo apt-get -y install cmake libjson-c-dev libbotan-2-dev asciidoctor
sudo apt-get -y install cmake libbz2-dev libjson-c-dev libbotan-2-dev asciidoctor

- name: Download source package
uses: actions/download-artifact@v4
Expand Down
6 changes: 6 additions & 0 deletions src/lib/crypto/mem.h
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,12 @@ template <typename T, std::size_t N> struct secure_array {
return &data_[0];
}

const T *
data() const
{
return &data_[0];
}

std::size_t
size() const
{
Expand Down
42 changes: 29 additions & 13 deletions src/lib/ffi-priv-types.h
Original file line number Diff line number Diff line change
Expand Up @@ -187,11 +187,17 @@ struct rnp_op_sign_signature_st {
typedef std::list<rnp_op_sign_signature_st> rnp_op_sign_signatures_t;

struct rnp_op_sign_st {
rnp_ffi_t ffi{};
rnp_input_t input{};
rnp_output_t output{};
rnp_ctx_t rnpctx{};
rnp_op_sign_signatures_t signatures{};
rnp_ffi_t ffi;
rnp_input_t input;
rnp_output_t output;
rnp_ctx_t rnpctx;
rnp_op_sign_signatures_t signatures;

rnp_op_sign_st(rnp_ffi_t affi, rnp_input_t in, rnp_output_t out)
: ffi(affi), input(in), output(out),
rnpctx(ffi->context, ffi->key_provider, ffi->pass_provider)
{
}
};

struct rnp_op_verify_signature_st {
Expand All @@ -201,11 +207,11 @@ struct rnp_op_verify_signature_st {
};

struct rnp_op_verify_st {
rnp_ffi_t ffi{};
rnp_input_t input{};
rnp_ffi_t ffi;
rnp_input_t input;
rnp_input_t detached_input{}; /* for detached signature will be source file/data */
rnp_output_t output{};
rnp_ctx_t rnpctx{};
rnp_ctx_t rnpctx;
/* these fields are filled after operation execution */
std::vector<rnp_op_verify_signature_st> signatures_;
pgp_literal_hdr_t lithdr{};
Expand All @@ -225,15 +231,25 @@ struct rnp_op_verify_st {
rnp_symenc_handle_t used_symenc{};
size_t encrypted_layers{};

rnp_op_verify_st(rnp_ffi_t affi, rnp_input_t in)
: ffi(affi), input(in), rnpctx(ffi->context, ffi->key_provider, ffi->pass_provider)
{
}
~rnp_op_verify_st();
};

struct rnp_op_encrypt_st {
rnp_ffi_t ffi{};
rnp_input_t input{};
rnp_output_t output{};
rnp_ctx_t rnpctx{};
rnp_op_sign_signatures_t signatures{};
rnp_ffi_t ffi;
rnp_input_t input;
rnp_output_t output;
rnp_ctx_t rnpctx;
rnp_op_sign_signatures_t signatures;

rnp_op_encrypt_st(rnp_ffi_t affi, rnp_input_t in, rnp_output_t out)
: ffi(affi), input(in), output(out),
rnpctx(ffi->context, ffi->key_provider, ffi->pass_provider)
{
}
};

#define RNP_LOCATOR_MAX_SIZE (MAX_ID_LENGTH + 1)
Expand Down
3 changes: 3 additions & 0 deletions src/lib/logging.h
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,9 @@ class LogStop {
} // namespace rnp

/* remove "src" */
#ifndef SOURCE_PATH_SIZE
#define SOURCE_PATH_SIZE 0
#endif
#define __SOURCE_PATH_FILE__ (&(__FILE__[SOURCE_PATH_SIZE + 3]))

#define RNP_LOG_FD(fd, ...) \
Expand Down
58 changes: 8 additions & 50 deletions src/lib/rnp.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -120,15 +120,6 @@ ffi_key_provider(const pgp_key_request_ctx_t *ctx, void *userdata)
return find_key(ffi, ctx->search, ctx->secret, true);
}

static void
rnp_ctx_init_ffi(rnp_ctx_t &ctx, rnp_ffi_t ffi)
{
ctx.ctx = &ffi->context;
ctx.ealg = DEFAULT_PGP_SYMM_ALG;
ctx.aalg = PGP_AEAD_NONE;
ctx.abits = DEFAULT_AEAD_CHUNK_BITS;
}

static const id_str_pair sig_type_map[] = {{PGP_SIG_BINARY, "binary"},
{PGP_SIG_TEXT, "text"},
{PGP_SIG_STANDALONE, "standalone"},
Expand Down Expand Up @@ -2589,11 +2580,7 @@ try {
return RNP_ERROR_NULL_POINTER;
}

*op = new rnp_op_encrypt_st();
rnp_ctx_init_ffi((*op)->rnpctx, ffi);
(*op)->ffi = ffi;
(*op)->input = input;
(*op)->output = output;
*op = new rnp_op_encrypt_st(ffi, input, output);
return RNP_SUCCESS;
}
FFI_GUARD
Expand Down Expand Up @@ -2876,20 +2863,6 @@ try {
}
FFI_GUARD

static pgp_write_handler_t
pgp_write_handler(pgp_password_provider_t *pass_provider,
rnp_ctx_t * rnpctx,
void * param,
rnp::KeyProvider * key_provider)
{
pgp_write_handler_t handler{};
handler.password_provider = pass_provider;
handler.ctx = rnpctx;
handler.param = param;
handler.key_provider = key_provider;
return handler;
}

static rnp_result_t
rnp_op_add_signatures(rnp_op_sign_signatures_t &opsigs, rnp_ctx_t &ctx)
{
Expand Down Expand Up @@ -2925,14 +2898,12 @@ try {
if (!op->rnpctx.halg) {
op->rnpctx.halg = DEFAULT_PGP_HASH_ALG;
}
pgp_write_handler_t handler =
pgp_write_handler(&op->ffi->pass_provider, &op->rnpctx, NULL, &op->ffi->key_provider);

rnp_result_t ret;
rnp_result_t ret = RNP_ERROR_GENERIC;
if (!op->signatures.empty() && (ret = rnp_op_add_signatures(op->signatures, op->rnpctx))) {
return ret;
}
ret = rnp_encrypt_sign_src(&handler, &op->input->src, &op->output->dst);
ret = rnp_encrypt_sign_src(op->rnpctx, op->input->src, op->output->dst);

dst_flush(&op->output->dst);
op->output->keep = ret == RNP_SUCCESS;
Expand All @@ -2958,11 +2929,7 @@ try {
return RNP_ERROR_NULL_POINTER;
}

*op = new rnp_op_sign_st();
rnp_ctx_init_ffi((*op)->rnpctx, ffi);
(*op)->ffi = ffi;
(*op)->input = input;
(*op)->output = output;
*op = new rnp_op_sign_st(ffi, input, output);
return RNP_SUCCESS;
}
FFI_GUARD
Expand Down Expand Up @@ -3126,14 +3093,11 @@ try {
if (!op->rnpctx.halg) {
op->rnpctx.halg = DEFAULT_PGP_HASH_ALG;
}
pgp_write_handler_t handler =
pgp_write_handler(&op->ffi->pass_provider, &op->rnpctx, NULL, &op->ffi->key_provider);

rnp_result_t ret;
rnp_result_t ret = RNP_ERROR_GENERIC;
if ((ret = rnp_op_add_signatures(op->signatures, op->rnpctx))) {
return ret;
}
ret = rnp_sign_src(&handler, &op->input->src, &op->output->dst);
ret = rnp_sign_src(op->rnpctx, op->input->src, op->output->dst);

dst_flush(&op->output->dst);
op->output->keep = ret == RNP_SUCCESS;
Expand Down Expand Up @@ -3320,10 +3284,7 @@ try {
return RNP_ERROR_NULL_POINTER;
}

*op = new rnp_op_verify_st();
rnp_ctx_init_ffi((*op)->rnpctx, ffi);
(*op)->ffi = ffi;
(*op)->input = input;
*op = new rnp_op_verify_st(ffi, input);
(*op)->output = output;

return RNP_SUCCESS;
Expand All @@ -3340,11 +3301,8 @@ try {
return RNP_ERROR_NULL_POINTER;
}

*op = new rnp_op_verify_st();
rnp_ctx_init_ffi((*op)->rnpctx, ffi);
*op = new rnp_op_verify_st(ffi, signature);
(*op)->rnpctx.detached = true;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe it will be more consistent to add these params to constructor as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree, thanks. Will address via another PR as this one is already merged.

(*op)->ffi = ffi;
(*op)->input = signature;
(*op)->detached_input = input;

return RNP_SUCCESS;
Expand Down
16 changes: 16 additions & 0 deletions src/librepgp/stream-common.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@
#include <rnp/rnp_def.h>
#include "rnp.h"
#include "stream-common.h"
#include "stream-packet.h"
#include "types.h"
#include "file-utils.h"
#include "crypto/mem.h"
Expand Down Expand Up @@ -1230,3 +1231,18 @@ check_enforce_aes_v3_pkesk(pgp_pubkey_alg_t alg, pgp_symm_alg_t salg, pgp_pkesk_
return (ver != PGP_PKSK_V3) || have_pkesk_checksum(alg) || pgp_is_sa_aes(salg);
}
#endif

#if defined(ENABLE_AEAD)
bool
encrypted_sesk_set_ad(pgp_crypt_t &crypt, pgp_sk_sesskey_t &skey)
{
uint8_t ad_data[4];

ad_data[0] = PGP_PKT_SK_SESSION_KEY | PGP_PTAG_ALWAYS_SET | PGP_PTAG_NEW_FORMAT;
ad_data[1] = skey.version;
ad_data[2] = skey.alg;
ad_data[3] = skey.aalg;

return pgp_cipher_aead_set_ad(&crypt, ad_data, 4);
}
#endif
4 changes: 4 additions & 0 deletions src/librepgp/stream-common.h
Original file line number Diff line number Diff line change
Expand Up @@ -563,5 +563,9 @@ bool check_enforce_aes_v3_pkesk(pgp_pubkey_alg_t alg,
pgp_symm_alg_t salg,
pgp_pkesk_version_t ver);
#endif
#if defined(ENABLE_AEAD)
typedef struct pgp_sk_sesskey_t pgp_sk_sesskey_t;
bool encrypted_sesk_set_ad(pgp_crypt_t &crypt, pgp_sk_sesskey_t &skey);
#endif

#endif
4 changes: 2 additions & 2 deletions src/librepgp/stream-ctx.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,9 @@ rnp_ctx_t::add_encryption_password(const std::string &password,
info.s2k.usage = PGP_S2KU_ENCRYPTED_AND_HASHED;
info.s2k.specifier = PGP_S2KS_ITERATED_AND_SALTED;
info.s2k.hash_alg = halg;
ctx->rng.get(info.s2k.salt, sizeof(info.s2k.salt));
sec_ctx.rng.get(info.s2k.salt, sizeof(info.s2k.salt));
if (!iterations) {
iterations = ctx->s2k_iterations(halg);
iterations = sec_ctx.s2k_iterations(halg);
}
if (!iterations) {
return RNP_ERROR_BAD_PARAMETERS;
Expand Down
30 changes: 20 additions & 10 deletions src/librepgp/stream-ctx.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@
#include <list>
#include "pgp-key.h"
#include "crypto/mem.h"
#include "key-provider.h"
#include "pass-provider.h"
#include "sec_profile.hpp"

/* signature info structure */
Expand Down Expand Up @@ -93,18 +95,18 @@ typedef struct rnp_symmetric_pass_info_t {
*/

typedef struct rnp_ctx_t {
std::string filename{}; /* name of the input file to store in literal data packet */
std::string filename; /* name of the input file to store in literal data packet */
int64_t filemtime{}; /* file modification time to store in literal data packet */
int64_t sigcreate{}; /* signature creation time */
uint64_t sigexpire{}; /* signature expiration time */
bool clearsign{}; /* cleartext signature */
bool detached{}; /* detached signature */
pgp_hash_alg_t halg{}; /* hash algorithm */
pgp_symm_alg_t ealg{}; /* encryption algorithm */
pgp_hash_alg_t halg; /* hash algorithm */
pgp_symm_alg_t ealg; /* encryption algorithm */
int zalg{}; /* compression algorithm used */
int zlevel{}; /* compression level */
pgp_aead_alg_t aalg{}; /* non-zero to use AEAD */
int abits{}; /* AEAD chunk bits */
pgp_aead_alg_t aalg; /* non-zero to use AEAD */
int abits; /* AEAD chunk bits */
bool overwrite{}; /* allow to overwrite output file if exists */
bool armor{}; /* whether to use ASCII armor on output */
bool no_wrap{}; /* do not wrap source in literal data packet */
Expand All @@ -114,12 +116,20 @@ typedef struct rnp_ctx_t {
#if defined(ENABLE_PQC)
bool pref_pqc_enc_subkey{}; /* prefer to encrypt to PQC subkey */
#endif
std::list<pgp_key_t *> recipients{}; /* recipients of the encrypted message */
std::list<rnp_symmetric_pass_info_t> passwords{}; /* passwords to encrypt message */
std::list<rnp_signer_info_t> signers{}; /* keys to which sign message */
rnp::SecurityContext * ctx{}; /* pointer to rnp::RNG */
std::list<pgp_key_t *> recipients; /* recipients of the encrypted message */
std::list<rnp_symmetric_pass_info_t> passwords; /* passwords to encrypt message */
std::list<rnp_signer_info_t> signers; /* keys to which sign message */
rnp::SecurityContext & sec_ctx; /* security context */
rnp::KeyProvider & key_provider; /* Key provider */
pgp_password_provider_t & pass_provider; /* Password provider */

rnp_ctx_t(rnp::SecurityContext & sctx,
rnp::KeyProvider & kprov,
pgp_password_provider_t &pprov)
: halg(DEFAULT_PGP_HASH_ALG), ealg(DEFAULT_PGP_SYMM_ALG), aalg(PGP_AEAD_NONE),
abits(DEFAULT_AEAD_CHUNK_BITS), sec_ctx(sctx), key_provider(kprov),
pass_provider(pprov){};

rnp_ctx_t() = default;
rnp_ctx_t(const rnp_ctx_t &) = delete;
rnp_ctx_t(rnp_ctx_t &&) = delete;

Expand Down
Loading
Loading