Skip to content

[lib] refactor crypt.cpp, set minimal required OpenSSL version to 3.0 - #7209

Merged
davidpanderson merged 34 commits into
masterfrom
vko_refactor_crypt_cpp
Aug 21, 2026
Merged

[lib] refactor crypt.cpp, set minimal required OpenSSL version to 3.0#7209
davidpanderson merged 34 commits into
masterfrom
vko_refactor_crypt_cpp

Conversation

@AenBleidd

@AenBleidd AenBleidd commented Aug 9, 2026

Copy link
Copy Markdown
Member

Summary by cubic

Refactors crypto to OpenSSL ≥ 3.0 and unifies key/signature APIs across client, scheduler, and tools. Old behavior used OpenSSL 1.x with mixed return semantics and inconsistent cert checks; new behavior uses EVP/OSSL_PARAM/openssl/encoder, returns explicit status + validity, and fixes certificate verification.

  • Changes

    • Standardize returns:
      • check_file_signature/check_string_signaturestd::pair<int,bool>.
      • read_key_filestd::pair<int,R_RSA_*_KEY>.
      • generate_signature → hex std::string.
      • sign_executablestd::pair<int,std::string>.
    • Remove check_*_signature2; remove check_validity_of_cert from public API; cert_verify_file now returns int status (0 = verified).
    • Enforce OpenSSL 3.0 in m4/check_ssl.m4, CMake (find_package(OpenSSL 3.0)), and CI; add python3-openssl, pyopenssl, cryptography.
    • Large refactor of lib/crypt.cpp and lib/crypt_prog.cpp; add tests/unit-tests/lib/test_crypt.cpp, tests/sign_executable_tests.py, and expand tests/crypt_prog_tests.py.
  • Required migration

    • Replace check_*_signature2 with check_*_signature and handle std::pair<int,bool> (check both status and validity).
    • Update callers of read_key_file and sign_executable to destructure std::pair<int,...>.
    • Treat cert_verify_file return 0 as success; update any boolean checks.
    • Consume generate_signature as a hex std::string.

Written for commit 024b9a2. Summary will update on new commits.

Review in cubic

@AenBleidd AenBleidd added this to the Client/Manager 8.2.16 milestone Aug 9, 2026
Copilot AI lite review requested due to automatic review settings August 9, 2026 10:03
@github-project-automation github-project-automation Bot moved this to In progress in Client/Manager Aug 9, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Refactors BOINC’s crypto/key/signature utilities toward modern OpenSSL (EVP/OSSL_PARAM) usage, updates call sites to new return types, and expands unit/integration testing coverage (including CI dependency updates).

Changes:

  • Modernized lib/crypt.* APIs (vectors/strings/RAII) and updated client/scheduler/tools callers accordingly.
  • Added extensive crypto unit tests and new integration tests for crypt_prog + sign_executable.
  • Updated CMake and GitHub Actions workflows to link/install OpenSSL and Python crypto dependencies for tests.

Reviewed changes

Copilot reviewed 22 out of 23 changed files in this pull request and generated 9 comments.

Show a summary per file
File Description
win_build/unittests.vcxproj.filters Adds test_crypt.cpp to the VS filter structure.
win_build/unittests.vcxproj Builds test_crypt.cpp in Windows unit tests project.
tools/sign_executable.cpp Refactors signing tool to C++/new crypto APIs and returns richer result.
tools/process_result_template.cpp Updates signature generation call sites to new generate_signature() API.
tests/unit-tests/lib/test_crypt.cpp Adds large unit test suite for crypto/key/signature/cert verification helpers.
tests/unit-tests/lib/CMakeLists.txt Links unit tests against OpenSSL targets.
tests/unit-tests/CMakeLists.txt Finds OpenSSL for unit-test builds.
tests/sign_executable_tests.py Adds integration test runner for sign_executable.
tests/crypt_prog_tests.py Expands crypt_prog integration tests and adds cert-store verification coverage.
sched/transitioner.cpp Updates key reading call site to new read_key_file() return type.
sched/sched_assign.cpp Updates key reading call site to new read_key_file() return type.
sched/make_work.cpp Updates key reading call site to new read_key_file() return type.
sched/get_file.cpp Updates key reading call site to new read_key_file() return type.
sched/file_upload_handler.cpp Updates signature/key scanning to new APIs and improves error logging.
lib/crypt.h Introduces new OpenSSL RAII helpers + modernized crypto function signatures.
lib/crypt.cpp Implements modernized crypto/key IO + signature verification using EVP APIs.
lib/crypt_prog.cpp Updates CLI tool implementation to new crypto APIs; adds encoder-based key export.
client/cs_scheduler.cpp Updates code-sign key signature verification call site to new API.
client/cs_files.cpp Updates file signature verification call site to new API.
client/async_file.cpp Updates async signature verification call site to new API.
client/acct_mgr.cpp Updates account manager URL signature verification call site to new API.
.github/workflows/windows.yml Installs Python crypto dependencies for Windows test runs.
.github/workflows/linux.yml Installs Python OpenSSL dependency and runs new integration test.
Suppressed comments (1)

tests/crypt_prog_tests.py:197

  • os.symlink() can fail on Windows without Developer Mode/admin privileges. Since this test is executed in the Windows workflow, add a fallback (e.g., hard-link) when symlink creation fails.
            os.symlink(leaf_cert_path, os.path.join(ca_dir, f"{leaf_hash}.0"))

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread lib/crypt.cpp Outdated
Comment thread lib/crypt.cpp Outdated
Comment thread lib/crypt.cpp Outdated
Comment thread lib/crypt_prog.cpp
Comment thread tools/sign_executable.cpp
Comment thread tests/unit-tests/lib/test_crypt.cpp
Comment thread tests/unit-tests/lib/test_crypt.cpp
Comment thread tests/crypt_prog_tests.py
Comment thread tests/sign_executable_tests.py
Copilot AI review requested due to automatic review settings August 9, 2026 15:18

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 22 out of 23 changed files in this pull request and generated 3 comments.

Suppressed comments (8)

lib/crypt_prog.cpp:122

  • genkey() calls EVP_PKEY_CTX_set1_rsa_keygen_pubexp(ctx.get(), BN_new()) with an uninitialized exponent (and leaks that BIGNUM). It also ignores the retval from openssl_to_keys(), so key conversion failures would still write garbage keys to disk.
    unique_PKEY_CTX ctx = unique_PKEY_CTX(EVP_PKEY_CTX_new_from_name(
        nullptr, "RSA", nullptr));
    EVP_PKEY_keygen_init(ctx.get());
    EVP_PKEY_CTX_set_rsa_keygen_bits(ctx.get(), n);
    EVP_PKEY_CTX_set1_rsa_keygen_pubexp(ctx.get(), BN_new());
    EVP_PKEY* rp = nullptr;
    if (EVP_PKEY_keygen(ctx.get(), &rp) <= 0) {
        print_error("EVP_PKEY_keygen");
        return 2;
    }

    unique_EVP_PKEY rsa_key(rp);

    R_RSA_PUBLIC_KEY public_key;
    R_RSA_PRIVATE_KEY private_key;
    int retval = 0;
    std::tie(retval, private_key, public_key) = openssl_to_keys(rsa_key);
    FILE *fpriv = open_file(private_keyfile, "w");

tools/sign_executable.cpp:21

  • This file calls fprintf()/printf() but doesn't include <cstdio> (and no other include here guarantees those declarations). This can fail to compile depending on toolchain/headers.
#include "config.h"
#include "crypt.h"

tests/crypt_prog_tests.py:160

  • os.symlink() can fail on Windows runners (privilege/developer-mode dependent). Since this test runs in the Windows workflow, a missing symlink privilege will fail CI even though the cert contents are correct. Consider falling back to copying when symlinks aren't available.
            ca_hash = f"{ca_x509_cert.subject_name_hash():08x}"
            os.symlink(ca_cert_path, os.path.join(ca_dir, f"{ca_hash}.0"))

tests/crypt_prog_tests.py:198

  • Same as above: os.symlink() may be unavailable on Windows. Add a fallback so the cert-store test doesn't fail due to platform symlink restrictions.
            leaf_hash = f"{leaf_x509_cert.subject_name_hash():08x}"
            os.symlink(leaf_cert_path, os.path.join(ca_dir, f"{leaf_hash}.0"))

lib/crypt.cpp:207

  • sscan_key_hex() writes num_bits into a std::vector<uint8_t> via reinterpret_cast<short int*>, which can violate alignment/strict-aliasing rules. Use memcpy into the byte buffer instead.
    result.resize(sizeof(num_bits));
    *reinterpret_cast<short int*>(result.data()) = num_bits;

lib/crypt.cpp:668

  • openssl_to_public() allocates BIGNUMs via EVP_PKEY_get_bn_param() but never frees them, leaking memory on every call. Wrap the returned BIGNUM* in RAII (or BN_free() them before returning).
    BIGNUM *n = nullptr, *e = nullptr;
    if (!EVP_PKEY_get_bn_param(pkey.get(), OSSL_PKEY_PARAM_RSA_N, &n) ||
        !EVP_PKEY_get_bn_param(pkey.get(), OSSL_PKEY_PARAM_RSA_E, &e)) {
        return std::make_pair(ERR_CRYPTO, pub);
    }

    if (!bn2bin(n, pub.modulus, sizeof(pub.modulus)) ||
        !bn2bin(e, pub.exponent, sizeof(pub.exponent))) {
        return std::make_pair(ERR_CRYPTO, pub);
    }

lib/crypt_prog.cpp:161

  • sign() leaks the opened key file handle on both success and failure paths (e.g. returning after scan_private_key_hex() fails). Similar early-return patterns exist in other helpers; using RAII for FILE* avoids these leaks.
int sign(const std::string& file, const std::string& private_keyfile) {
    FILE* fpriv = open_file(private_keyfile, "r");
    if (!fpriv) {
        print_error("fopen");
        return 2;
    }
    bool result = false;
    R_RSA_PRIVATE_KEY private_key;
    std::tie(result, private_key) = scan_private_key_hex(fpriv);
    if (!result) {
        print_error("scan_private_key_hex");
        return 2;
    }
    std::vector<uint8_t> signature;
    std::tie(result, signature) = sign_file(file, private_key);
    if (!result || signature.empty()) {
        print_error("sign_file");
        return 2;
    }
    print_hex_data(stdout, signature);
    return 0;
}

tools/process_result_template.cpp:74

  • If generate_signature() fails, the current code returns ERR_XML_PARSE, which is misleading (the XML may be fine; signing failed). Return a crypto-related error code instead so callers/logs reflect the real failure mode.
        bool result = false;
        std::string signature_hex;
        std::tie(result, signature_hex) = generate_signature(signed_xml, key);
        if (!result || signature_hex.empty()) {
            return ERR_XML_PARSE;
        }

Comment thread lib/crypt.h
Comment thread lib/crypt.cpp
Comment thread lib/crypt.cpp
Copilot AI review requested due to automatic review settings August 9, 2026 18:44

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 22 out of 23 changed files in this pull request and generated no new comments.

Suppressed comments (4)

tools/sign_executable.cpp:22

  • tools/sign_executable.cpp uses fprintf/printf and std::tie/std::pair but doesn't include the standard headers that declare them. With the current includes (only config.h/crypt.h), this will fail to compile on conforming C++ compilers (missing , /).
#include "config.h"
#include "crypt.h"

lib/crypt_prog.cpp:109

  • genkey() sets the RSA public exponent via EVP_PKEY_CTX_set1_rsa_keygen_pubexp(ctx, BN_new()) without initializing the BIGNUM (e.g., to RSA_F4/65537) and without freeing it. This is likely to generate an invalid key or fail keygen, and it leaks the BIGNUM on success paths.
    unique_PKEY_CTX ctx = unique_PKEY_CTX(EVP_PKEY_CTX_new_from_name(
        nullptr, "RSA", nullptr));
    EVP_PKEY_keygen_init(ctx.get());
    EVP_PKEY_CTX_set_rsa_keygen_bits(ctx.get(), n);
    EVP_PKEY_CTX_set1_rsa_keygen_pubexp(ctx.get(), BN_new());

lib/crypt.cpp:682

  • openssl_to_private() allocates multiple BIGNUMs via EVP_PKEY_get_bn_param() (n/e and optionally d/p/q/...) but never frees them. These BIGNUMs are heap-allocated by OpenSSL, so this leaks memory for each call.
    BIGNUM *n = nullptr, *e = nullptr, *d = nullptr;
    BIGNUM *p = nullptr, *q = nullptr;
    BIGNUM *dmp1 = nullptr, *dmq1 = nullptr, *iqmp = nullptr;

    if (!EVP_PKEY_get_bn_param(pkey.get(), OSSL_PKEY_PARAM_RSA_N, &n) ||

lib/crypt.cpp:669

  • openssl_to_public() allocates BIGNUMs via EVP_PKEY_get_bn_param() but never frees them. EVP_PKEY_get_bn_param() returns newly allocated BIGNUMs that must be BN_free()'d, so this leaks on every call (including error paths after the allocation).

This issue also appears on line 678 of the same file.

    BIGNUM *n = nullptr, *e = nullptr;
    if (!EVP_PKEY_get_bn_param(pkey.get(), OSSL_PKEY_PARAM_RSA_N, &n) ||
        !EVP_PKEY_get_bn_param(pkey.get(), OSSL_PKEY_PARAM_RSA_E, &e)) {
        return std::make_pair(ERR_CRYPTO, pub);
    }

Copilot AI review requested due to automatic review settings August 11, 2026 13:50
@AenBleidd AenBleidd changed the title [lib] refactor crypt.cpp [lib] refactor crypt.cpp, set minimal required OpenSSL version to 3.0 Aug 11, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 23 out of 24 changed files in this pull request and generated 1 comment.

Suppressed comments (3)

lib/crypt_prog.cpp:110

  • EVP_PKEY_CTX_set1_rsa_keygen_pubexp(ctx.get(), BN_new()) passes an uninitialized exponent (and leaks the BIGNUM). This can make key generation fail or generate keys with an invalid public exponent.
    unique_PKEY_CTX ctx = unique_PKEY_CTX(EVP_PKEY_CTX_new_from_name(
        nullptr, "RSA", nullptr));
    EVP_PKEY_keygen_init(ctx.get());
    EVP_PKEY_CTX_set_rsa_keygen_bits(ctx.get(), n);
    EVP_PKEY_CTX_set1_rsa_keygen_pubexp(ctx.get(), BN_new());
    EVP_PKEY* rp = nullptr;

tools/sign_executable.cpp:22

  • This file calls fprintf()/printf() but doesn't include <cstdio> (or <stdio.h>). Relying on transitive includes is fragile and can break builds depending on how config.h is generated.
// syntax: sign_executable data_file private_key_file

#include "config.h"
#include "crypt.h"

lib/crypt.cpp:210

  • sscan_key_hex() writes num_bits into a std::vector<uint8_t> using reinterpret_cast<short int*>, which can be an unaligned write and is undefined behavior on some architectures. Use memcpy() instead.
    result.resize(sizeof(num_bits));
    *reinterpret_cast<short int*>(result.data()) = num_bits;
    result.reserve(result.size() + data.size());
    result.insert(result.end(), data.begin(), data.end());

Comment thread lib/crypt_prog.cpp
Copilot AI review requested due to automatic review settings August 11, 2026 14:23

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 25 out of 26 changed files in this pull request and generated 1 comment.

Suppressed comments (8)

lib/crypt_prog.cpp:112

  • genkey(): EVP_PKEY_CTX_set1_rsa_keygen_pubexp(ctx.get(), BN_new()) creates a BIGNUM with value 0 (and leaks it), which can cause RSA key generation to fail or generate an invalid key. The public exponent should be explicitly set (typically 65537/RSA_F4) and error-checked.
    unique_PKEY_CTX ctx = unique_PKEY_CTX(EVP_PKEY_CTX_new_from_name(
        nullptr, "RSA", nullptr));
    EVP_PKEY_keygen_init(ctx.get());
    EVP_PKEY_CTX_set_rsa_keygen_bits(ctx.get(), n);
    EVP_PKEY_CTX_set1_rsa_keygen_pubexp(ctx.get(), BN_new());
    EVP_PKEY* rp = nullptr;
    if (EVP_PKEY_keygen(ctx.get(), &rp) <= 0) {
        print_error("EVP_PKEY_keygen");

tools/sign_executable.cpp:22

  • sign_executable.cpp calls fprintf/printf but no longer includes /<stdio.h>, and crypt.h no longer includes it either. This can cause compilation failures depending on include order.
// syntax: sign_executable data_file private_key_file

#include "config.h"
#include "crypt.h"

lib/crypt.h:27

  • crypt.h declares APIs using std::pair/std::tuple and FILE*, but it doesn't include the standard headers that define them (, , ). Depending on transitive includes, this can break compilation for consumers that include crypt.h first.
#include <vector>
#include <string>
#include <memory>

#include <openssl/rsa.h>
#include <openssl/evp.h>

lib/crypt.cpp:691

  • openssl_to_private() also leaks BIGNUMs returned by EVP_PKEY_get_bn_param() (n/e and the optional CRT params). These should be freed with BN_free() on all paths (RAII is simplest).
    BIGNUM *n = nullptr, *e = nullptr, *d = nullptr;
    BIGNUM *p = nullptr, *q = nullptr;
    BIGNUM *dmp1 = nullptr, *dmq1 = nullptr, *iqmp = nullptr;

    if (!EVP_PKEY_get_bn_param(pkey.get(), OSSL_PKEY_PARAM_RSA_N, &n) ||
        !EVP_PKEY_get_bn_param(pkey.get(), OSSL_PKEY_PARAM_RSA_E, &e)) {
        return std::make_pair(ERR_CRYPTO, priv);
    }
    EVP_PKEY_get_bn_param(pkey.get(), OSSL_PKEY_PARAM_RSA_D, &d);
    EVP_PKEY_get_bn_param(pkey.get(), OSSL_PKEY_PARAM_RSA_FACTOR1, &p);
    EVP_PKEY_get_bn_param(pkey.get(), OSSL_PKEY_PARAM_RSA_FACTOR2, &q);
    EVP_PKEY_get_bn_param(pkey.get(), OSSL_PKEY_PARAM_RSA_EXPONENT1, &dmp1);
    EVP_PKEY_get_bn_param(pkey.get(), OSSL_PKEY_PARAM_RSA_EXPONENT2, &dmq1);
    EVP_PKEY_get_bn_param(pkey.get(), OSSL_PKEY_PARAM_RSA_COEFFICIENT1, &iqmp);

lib/crypt_prog.cpp:53

  • This file uses FILE/fopen/fclose in open_file() and elsewhere, but no longer includes /<stdio.h>. Relying on transitive includes is fragile and can break builds on some platforms/compilers.
#include <iostream>

#include <openssl/pem.h>
#include <openssl/err.h>
#include <openssl/encoder.h>

#include "crypt.h"
#include "md5_file.h"

lib/crypt.cpp:666

  • openssl_to_public()/openssl_to_private() allocate BIGNUMs via EVP_PKEY_get_bn_param(), but they are never freed. EVP_PKEY_get_bn_param() expects the caller to release these with BN_free(), so this leaks memory each time these conversion helpers are used.

This issue also appears on line 678 of the same file.

    BIGNUM *n = nullptr, *e = nullptr;
    if (!EVP_PKEY_get_bn_param(pkey.get(), OSSL_PKEY_PARAM_RSA_N, &n) ||
        !EVP_PKEY_get_bn_param(pkey.get(), OSSL_PKEY_PARAM_RSA_E, &e)) {
        return std::make_pair(ERR_CRYPTO, pub);
    }

    if (!bn2bin(n, pub.modulus, sizeof(pub.modulus)) ||
        !bn2bin(e, pub.exponent, sizeof(pub.exponent))) {
        return std::make_pair(ERR_CRYPTO, pub);
    }

lib/crypt.cpp:210

  • sscan_key_hex() writes num_bits into a std::vector<uint8_t> via reinterpret_cast<short int*>(result.data()), which can be misaligned and is undefined behavior on some platforms. Use memcpy instead.
    result.resize(sizeof(num_bits));
    *reinterpret_cast<short int*>(result.data()) = num_bits;
    result.reserve(result.size() + data.size());
    result.insert(result.end(), data.begin(), data.end());

tests/sign_executable_tests.py:44

  • _run_app() builds a single string and uses .split() to create argv. This breaks when paths/args contain spaces (common on Windows) and can change semantics of quoted arguments. Use shlex.split() and include shlex.
    def _run_app(self, app, args):
        proc = subprocess.Popen((app + " " + args).split(), stdout=subprocess.PIPE, stderr=subprocess.PIPE)
        result, err = proc.communicate()
        exit_code = proc.wait()
        return result, err, exit_code

Comment thread 3rdParty/vcpkg_ports/configs/libs/android/vcpkg.json
Copilot AI review requested due to automatic review settings August 11, 2026 19:58

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 25 out of 26 changed files in this pull request and generated no new comments.

Suppressed comments (6)

lib/crypt_prog.cpp:111

  • genkey() sets the RSA public exponent with BN_new() but never assigns a value (so the exponent is 0), which will generate invalid keys. Also, EVP_PKEY_CTX_new_from_name() and the keygen init calls aren’t checked for failure before dereferencing ctx.
    unique_PKEY_CTX ctx = unique_PKEY_CTX(EVP_PKEY_CTX_new_from_name(
        nullptr, "RSA", nullptr));
    EVP_PKEY_keygen_init(ctx.get());
    EVP_PKEY_CTX_set_rsa_keygen_bits(ctx.get(), n);
    EVP_PKEY_CTX_set1_rsa_keygen_pubexp(ctx.get(), BN_new());
    EVP_PKEY* rp = nullptr;
    if (EVP_PKEY_keygen(ctx.get(), &rp) <= 0) {

lib/crypt.h:92

  • This comment has a typo: “vaue” → “value”.
// first 'bool' vaue indicates if the data was converted successfully

tools/sign_executable.cpp:22

  • This file uses fprintf()/printf() but no longer includes <cstdio>/<stdio.h>. Since config.h is generated and may not include standard headers, this can break compilation depending on build configuration.
#include "config.h"
#include "crypt.h"

lib/crypt.cpp:209

  • sscan_key_hex() writes num_bits into a std::vector<uint8_t> using reinterpret_cast<short int*>, which can be undefined behavior due to alignment/strict-aliasing. Prefer memcpy into the byte buffer.
    result.resize(sizeof(num_bits));
    *reinterpret_cast<short int*>(result.data()) = num_bits;
    result.reserve(result.size() + data.size());

lib/crypt.cpp:830

  • check_validity_of_cert() calls memcmp(recovered.data(), md5_md.data(), recovered_len) without verifying md5_md.size() is at least recovered_len. If md5_md is unexpectedly shorter, this becomes an out-of-bounds read.
    return (
        recovered_len == (MD5_DIGEST_LENGTH * 2) &&
        !memcmp(recovered.data(), md5_md.data(), recovered_len)
    );

lib/crypt.h:85

  • This comment has a typo: “vaue” → “value”.

This issue also appears on line 92 of the same file.

// first 'bool' vaue indicates if the data was converted successfully

Copilot AI review requested due to automatic review settings August 11, 2026 21:05

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 26 out of 27 changed files in this pull request and generated no new comments.

Suppressed comments (6)

lib/crypt.h:23

  • crypt.h now uses uint8_t, std::pair, and std::tuple, and also declares APIs taking FILE*, but the header no longer includes the standard headers that define these types. This makes the header non-self-contained and can break builds depending on include order.
#include <vector>
#include <string>
#include <memory>

lib/crypt_prog.cpp:112

  • genkey() sets the RSA public exponent with BN_new() but never initializes it (e.g., to 65537). This can cause key generation to fail (or produce invalid keys), and it also leaks the created BIGNUM.
    unique_PKEY_CTX ctx = unique_PKEY_CTX(EVP_PKEY_CTX_new_from_name(
        nullptr, "RSA", nullptr));
    EVP_PKEY_keygen_init(ctx.get());
    EVP_PKEY_CTX_set_rsa_keygen_bits(ctx.get(), n);
    EVP_PKEY_CTX_set1_rsa_keygen_pubexp(ctx.get(), BN_new());
    EVP_PKEY* rp = nullptr;
    if (EVP_PKEY_keygen(ctx.get(), &rp) <= 0) {
        print_error("EVP_PKEY_keygen");

lib/crypt.cpp:686

  • openssl_to_private() allocates several BIGNUMs via EVP_PKEY_get_bn_param() and currently never frees them. This will leak on every call; wrap the returned BIGNUM pointers in unique_BN guards (or BN_free() them on all exit paths).
    BIGNUM *n = nullptr, *e = nullptr, *d = nullptr;
    BIGNUM *p = nullptr, *q = nullptr;
    BIGNUM *dmp1 = nullptr, *dmq1 = nullptr, *iqmp = nullptr;

    if (!EVP_PKEY_get_bn_param(pkey.get(), OSSL_PKEY_PARAM_RSA_N, &n) ||
        !EVP_PKEY_get_bn_param(pkey.get(), OSSL_PKEY_PARAM_RSA_E, &e)) {
        return std::make_pair(ERR_CRYPTO, priv);
    }
    EVP_PKEY_get_bn_param(pkey.get(), OSSL_PKEY_PARAM_RSA_D, &d);

lib/crypt_prog.cpp:317

  • test_crypt() prints the decrypted bytes by reinterpreting out.data() as a C-string, but the decrypted buffer is not guaranteed to be NUL-terminated. This can read past the end of the vector (UB) and print garbage.
    std::cout << "out: " <<
        reinterpret_cast<const char*>(out.data()) <<
        std::endl;

lib/crypt.cpp:670

  • openssl_to_public() allocates BIGNUMs via EVP_PKEY_get_bn_param(), but never frees them. These allocations need to be released (or wrapped in RAII) to avoid leaking per call.

This issue also appears on line 678 of the same file.

    BIGNUM *n = nullptr, *e = nullptr;
    if (!EVP_PKEY_get_bn_param(pkey.get(), OSSL_PKEY_PARAM_RSA_N, &n) ||
        !EVP_PKEY_get_bn_param(pkey.get(), OSSL_PKEY_PARAM_RSA_E, &e)) {
        return std::make_pair(ERR_CRYPTO, pub);
    }

    if (!bn2bin(n, pub.modulus, sizeof(pub.modulus)) ||
        !bn2bin(e, pub.exponent, sizeof(pub.exponent))) {
        return std::make_pair(ERR_CRYPTO, pub);
    }

lib/crypt.cpp:210

  • sscan_key_hex() writes num_bits into the result buffer via reinterpret_cast<short int*>, which can violate alignment/aliasing rules. Use memcpy to write the value portably.
    result.resize(sizeof(num_bits));
    *reinterpret_cast<short int*>(result.data()) = num_bits;
    result.reserve(result.size() + data.size());
    result.insert(result.end(), data.begin(), data.end());

@Vulpine05 Vulpine05 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Minor grammar in comments, please review.

Comment thread lib/crypt.h Outdated
Comment thread lib/crypt.h Outdated
Comment thread lib/crypt.h Outdated
Comment thread lib/crypt.h Outdated
Comment thread lib/crypt.h Outdated
Comment thread lib/crypt.h Outdated
Comment thread lib/crypt.h Outdated
Comment thread lib/crypt.h Outdated
@AenBleidd
AenBleidd force-pushed the vko_refactor_crypt_cpp branch from f423850 to f2649e8 Compare August 16, 2026 01:52
Signed-off-by: Vitalii Koshura <lestat.de.lionkur@gmail.com>
Signed-off-by: Vitalii Koshura <lestat.de.lionkur@gmail.com>
Signed-off-by: Vitalii Koshura <lestat.de.lionkur@gmail.com>
Signed-off-by: Vitalii Koshura <lestat.de.lionkur@gmail.com>
…tions

Signed-off-by: Vitalii Koshura <lestat.de.lionkur@gmail.com>
Signed-off-by: Vitalii Koshura <lestat.de.lionkur@gmail.com>
Signed-off-by: Vitalii Koshura <lestat.de.lionkur@gmail.com>
…t tests

Signed-off-by: Vitalii Koshura <lestat.de.lionkur@gmail.com>
Signed-off-by: Vitalii Koshura <lestat.de.lionkur@gmail.com>
Signed-off-by: Vitalii Koshura <lestat.de.lionkur@gmail.com>
Signed-off-by: Vitalii Koshura <lestat.de.lionkur@gmail.com>
Signed-off-by: Vitalii Koshura <lestat.de.lionkur@gmail.com>
Signed-off-by: Vitalii Koshura <lestat.de.lionkur@gmail.com>
Signed-off-by: Vitalii Koshura <lestat.de.lionkur@gmail.com>
Signed-off-by: Vitalii Koshura <lestat.de.lionkur@gmail.com>
Signed-off-by: Vitalii Koshura <lestat.de.lionkur@gmail.com>
Signed-off-by: Vitalii Koshura <lestat.de.lionkur@gmail.com>
Signed-off-by: Vitalii Koshura <lestat.de.lionkur@gmail.com>
Signed-off-by: Vitalii Koshura <lestat.de.lionkur@gmail.com>
Signed-off-by: Vitalii Koshura <lestat.de.lionkur@gmail.com>
Signed-off-by: Vitalii Koshura <lestat.de.lionkur@gmail.com>
Signed-off-by: Vitalii Koshura <lestat.de.lionkur@gmail.com>
Signed-off-by: Vitalii Koshura <lestat.de.lionkur@gmail.com>
Signed-off-by: Vitalii Koshura <lestat.de.lionkur@gmail.com>
Signed-off-by: Vitalii Koshura <lestat.de.lionkur@gmail.com>
@AenBleidd
AenBleidd force-pushed the vko_refactor_crypt_cpp branch from b377ad7 to 024b9a2 Compare August 21, 2026 19:15

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

All reported issues were addressed across 24 files

Not reviewed (too large): tests/unit-tests/lib/test_crypt.cpp (~2,830 lines) - if these are generated or fixture files, add them to ignored paths to exclude them from future reviews.

Tip: cubic can generate docs of your entire codebase and keep them up to date. Try it here.

Re-trigger cubic

Comment thread lib/crypt.cpp
Comment thread lib/crypt_prog.cpp
Comment thread lib/crypt.cpp
@davidpanderson
davidpanderson merged commit c5c6cb7 into master Aug 21, 2026
425 of 427 checks passed
@davidpanderson
davidpanderson deleted the vko_refactor_crypt_cpp branch August 21, 2026 21:39
@github-project-automation github-project-automation Bot moved this from In progress to Merged in Client/Manager Aug 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Merged

Development

Successfully merging this pull request may close these issues.

boinc fails to build with OpenSSL 4.0

4 participants