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

FFI: botan_pubkey_load_ecdsa() call started to fail on master. #4615

Closed
ni4 opened this issue Jan 30, 2025 · 10 comments
Closed

FFI: botan_pubkey_load_ecdsa() call started to fail on master. #4615

ni4 opened this issue Jan 30, 2025 · 10 comments

Comments

@ni4
Copy link

ni4 commented Jan 30, 2025

Previously working for years botan_pubkey_load_ecdsa() call started to fail within the latest week or two (we have CI runs based on Botan's latest master). Is it something we should take care of? Actually, it is preferred to keep FFI working in the way it was before as may cause problems to existing users.

Here is the code snippet we use:

    const size_t curve_order = curve->bytes();
    rnp::bn      px(keydata.p.mpi + 1, curve_order);
    rnp::bn      py(keydata.p.mpi + 1 + curve_order, curve_order);

    if (!px || !py) {
        return false;
    }

    bool res = !botan_pubkey_load_ecdsa(&pubkey.get(), px.get(), py.get(), curve->botan_name);
    if (!res) {
        RNP_LOG("failed to load ecdsa public key");
    }
@randombit
Copy link
Owner

Does this fail for all curves or only some? If only some which ones?

Are you compiling with --disable-deprecated-features? That does (as of recently) in fact disable some (deprecated) elliptic curves. I wouldn't think any of these curves would be used in PGP though...

@randombit
Copy link
Owner

What error code is returned?

Is there any additional information if you set env BOTAN_FFI_PRINT_EXCEPTIONS=1

@randombit
Copy link
Owner

Use of --minimized flag to configure.py could also cause this, we minimize quite a bit more than we used to.

@ni4
Copy link
Author

ni4 commented Jan 30, 2025

@randombit Thanks for the prompt reply! Here is configuration step from CI runner, we do use --minimized-build. Waiting for CI to finish to check other things.

2025-01-30T13:12:12.3150162Z Running tools.sh with args: build_and_install_botan head, DIR_TOOLS: /opt/tools
2025-01-30T13:12:12.3151018Z Running build_and_install_botan version master (installing to /opt/botan/head)
2025-01-30T13:12:12.3165423Z Cloning into '/tmp/local_builds/botan'...
2025-01-30T13:12:14.1141050Z /tmp/local_builds/botan /__w/rnp/rnp
2025-01-30T13:12:14.1176129Z Building botan with modules: aead,aes,auto_rng,bigint,blowfish,camellia,cast128,cbc,cfb,crc24,x25519,des,dl_algo,dl_group,dsa,eax,ecc_key,ecdh,ecdsa,ed25519,elgamal,eme_pkcs1,emsa_pkcs1,emsa_raw,ffi,hash,raw_hash,hmac,hmac_drbg,idea,kdf,md5,ocb,pgp_s2k,rfc3394,rmd160,rsa,sha1,sha2_32,sha2_64,sha3,sm2,sm3,sm4,sp800_56a,twofish,
2025-01-30T13:12:14.2688696Z    INFO: ./configure.py invoked with options "--prefix=/opt/botan/head --with-debug-info --extra-cxxflags=-fno-omit-frame-pointer -fPIC --os=linux --cpu=x86_64 --disable-cc-tests --without-documentation --build-targets=shared,cli --minimized-build --enable-modules=aead,aes,auto_rng,bigint,blowfish,camellia,cast128,cbc,cfb,crc24,x25519,des,dl_algo,dl_group,dsa,eax,ecc_key,ecdh,ecdsa,ed25519,elgamal,eme_pkcs1,emsa_pkcs1,emsa_raw,ffi,hash,raw_hash,hmac,hmac_drbg,idea,kdf,md5,ocb,pgp_s2k,rfc3394,rmd160,rsa,sha1,sha2_32,sha2_64,sha3,sm2,sm3,sm4,sp800_56a,twofish,"
2025-01-30T13:12:14.2704278Z    INFO: Configuring to build Botan 3.7.0 (revision git:63746e757fe1892311e802c370790b889c5059c3)
2025-01-30T13:12:14.2705293Z    INFO: Python version: "3.12.3 (main, Apr 17 2024, 00:00:00) [GCC 14.0.1 20240411 (Red Hat 14.0.1-0)]"
2025-01-30T13:12:14.2706101Z    INFO: Implicit --cc-bin=g++ due to environment variable CXX
2025-01-30T13:12:14.2717675Z    INFO: Autodetected platform information: OS="Linux" machine="x86_64" proc=""
2025-01-30T13:12:14.2718776Z    INFO: Using /etc/ssl/certs/ca-certificates.crt as system certificate store
2025-01-30T13:12:14.2720431Z    INFO: Target is gcc:0.0-linux-x86_64
2025-01-30T13:12:14.2720912Z    INFO: Assuming target x86_64 is little endian
2025-01-30T13:12:14.2747296Z    INFO: Skipping (incompatible CPU): aes_armv8 aes_power8 sha1_armv8 sha2_32_armv8 sha2_64_armv8 shacal2_armv8 sm4_armv8
2025-01-30T13:12:14.2748828Z    INFO: Skipping (incompatible OS): certstor_system_macos certstor_system_windows commoncrypto win32_stats
2025-01-30T13:12:14.2763110Z    INFO: Skipping (not requested): adler32 aes_crystals_xof aes_ni aes_vaes aes_vperm argon2 argon2_avx2 argon2_ssse3 argon2fmt aria asio base32 base58 bcrypt bcrypt_pbkdf bitvector blake2 blake2mac blake2s cascade ccm certstor_flatfile certstor_sql certstor_sqlite3 certstor_system chacha chacha20poly1305 chacha_avx2 chacha_avx512 chacha_rng chacha_simd32 classic_mceliece comb4p compression crc32 cryptobox curve448 dh dilithium dilithium_aes dilithium_common dilithium_round3 dilithium_shake dlies dyn_load ecgdsa ecies eckcdsa ed448 eme_oaep eme_raw emsa_x931 entropy fd_unix filters fpe_fe1 frodokem frodokem_aes frodokem_common gcm getentropy ghash ghash_cpu ghash_vperm gmac gost_28147 gost_3410 gost_3411 hkdf hotp hss_lms http_util idea_sse2 iso9796 jitter_rng kdf1 kdf1_iso18033 keccak keccak_perm_bmi2 kuznyechik kyber kyber_90s kyber_common kyber_round3 legacy_ec_point lion locking_allocator mce md4 mem_pool ml_dsa ml_kem noekeon noekeon_simd ofb par_hash passhash9 pbes2 pbkdf2 pcurves_brainpool256r1 pcurves_brainpool384r1 pcurves_brainpool512r1 pcurves_frp256v1 pcurves_impl pcurves_numsp512d1 pcurves_secp192r1 pcurves_secp224r1 pcurves_secp256k1 pcurves_secp256r1 pcurves_secp384r1 pcurves_secp521r1 pcurves_sm2p256v1 pkcs11 poly1305 pqcrystals prf_tls prf_x942 processor_rng psk_db rc4 rdseed rfc6979 roughtime salsa20 scrypt seed serpent serpent_avx2 serpent_avx512 serpent_simd sessions_sql sessions_sqlite3 sha1_sse2 sha1_x86 sha2_32_bmi2 sha2_32_x86 sha2_64_bmi2 shacal2 shacal2_avx2 shacal2_simd shacal2_x86 shake shake_cipher shake_xof simd simd_avx2 simd_avx512 siphash siv skein slh_dsa_sha2 slh_dsa_shake sm4_gfni socket sodium sp800_108 sp800_56c sphincsplus_common sphincsplus_sha2 sphincsplus_sha2_base sphincsplus_shake sphincsplus_shake_base srp6 streebog thread_utils threefish_512 tls tls12 tls13 tls13_pqc tls_cbc tpm2_crypto_backend tpm2_ecc tpm2_rsa tree_hash trunc_hash tss uuid whirlpool x448 x509 x919_mac xmd xmss xts zfec zfec_sse2 zfec_vperm
2025-01-30T13:12:14.2774979Z    INFO: Skipping (requires external dependency): boost bzip2 esdm_rng lzma sqlite3 tpm tpm2 zlib
2025-01-30T13:12:14.2776229Z WARNING: These modules are deprecated and will be removed in a future release (consider disabling with --disable-deprecated-features): md5
2025-01-30T13:12:14.2780072Z    INFO: Loading modules: aead aes asn1 auto_rng base base64 bigint blinding block blowfish camellia cast128 cbc cfb checksum cmac cpuid crc24 cshake_xof ctr des dl_algo dl_group dsa eax ec_group ecc_key ecdh ecdsa ed25519 elgamal eme_pkcs1 emsa_pkcs1 emsa_pssr emsa_raw ffi hash hash_id hex hmac hmac_drbg idea kdf kdf2 keccak_perm keypair kmac mac md5 mdx_hash mgf1 mode_pad modes mp nist_keywrap numbertheory ocb os_utils pbkdf pcurves pem pgp_s2k pk_pad poly_dbl pubkey raw_hash rfc3394 rmd160 rng rsa sha1 sha2_32 sha2_64 sha3 sm2 sm3 sm4 sp800_56a stateful_rng stream system_rng twofish utils x25519 xof
2025-01-30T13:12:14.7948317Z    INFO: Using symlink to link files into build dir (use --link-method to change)
2025-01-30T13:12:15.0412588Z    INFO: Botan 3.7.0 (revision git:63746e757fe1892311e802c370790b889c5059c3) (unreleased undated) build setup is complete

@randombit
Copy link
Owner

OK that's it

In 3.7.0 --minimized --enable-modules=ecdsa will result in zero elliptic curves being supported.

You can then add back specific curves by enabling a module that implements just that curve:

--minimized --enable-modules=ecdsa,pcurves_secp256r1,pcurves_secp384r1,pcurves_secp521r1,pcurves_brainpool256r1,...

In 3.8.0 there will also be a pcurves_generic (#4554) which can handle all (well, most) curves with a single implementation, albeit much slower than the per-curve logic currently in place.

Lastly you can enable legacy_ec_point which is the old BigInt-based implementation which I am trying very hard to kill off (#4027), which provides all curves with relatively poor performance.

@ni4
Copy link
Author

ni4 commented Jan 30, 2025

@randombit Thanks for the explanation, we'll adjust our CI scripts accordingly. Do I correctly understand that curve module names are newly added ones, which were not available before the 3.7.0?

1 similar comment
@ni4
Copy link
Author

ni4 commented Jan 30, 2025

@randombit Thanks for the explanation, we'll adjust our CI scripts accordingly. Do I correctly understand that curve module names are newly added ones, which were not available before the 3.7.0?

@randombit
Copy link
Owner

Do I correctly understand that curve module names are newly added ones, which were not available before the 3.7.0?

Most were added in 3.5.0, with the exceptions of pcurves_secp192r1, pcurves_secp224r1, and pcurves_numsp512d1 which were added in 3.6.0

@randombit
Copy link
Owner

OK I think you're set here so closing. I added a lengthy release note in #4616 to hopefully reduce the number of people who hit this

@ni4
Copy link
Author

ni4 commented Jan 31, 2025

Thanks for your help!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants