Move EC_Point to a deprecated submodule

Split EC_Point_Format to its own header; ideally this enum would go
into ec_group.h but this would cause a circular dependency between
ec_group.h and ec_point.h

Avoid almost all uses of EC_Group within the library. Add new functions where
required to pass points as EC_AffinePoint instead of EC_Point.

Move EC_Point and associated code into a new submodule `legacy_ec_point`.
Guard all uses of EC_Point with a feature macro check.

While the library builds with `legacy_ec_point` disabled, many tests fail since
disabling this also implicitly disables all curves not supported by pcurves,
such as secp224k1, or any application specific/test specific curves.

GH #4027

src/cli/perf_pk_misc.cpp

@@ -77,8 +77,9 @@ class PerfTest_ECDSAKeyRec final : public PerfTest {
                const uint8_t v = key.recovery_param(message, r, s);
 
                recovery_timer->run([&]() {
-                  Botan::ECDSA_PublicKey pubkey(group, message, r, s, v);
-                  BOTAN_ASSERT(pubkey.public_point() == key.public_point(), "Recovered public key");
+                  Botan::ECDSA_PublicKey recovered_key(group, message, r, s, v);
+                  BOTAN_ASSERT(recovered_key.public_key_bits() == key.public_key_bits(),
+                               "Recovered public key correctly");
                });
             }
 

src/examples/ecc_raw_public_key.cpp

@@ -30,7 +30,8 @@ int main() {
 
    // This loads the public point into an ECDSA_PublicKey. Creating an
    // ECDH_PublicKey would work the same way.
-   const auto public_key = Botan::ECDSA_PublicKey(domain, domain.OS2ECP(encoded_public_point));
+
+   const auto public_key = Botan::ECDSA_PublicKey(domain, Botan::EC_AffinePoint(domain, encoded_public_point));
 
    std::cout << "Public Key (PEM):\n\n" << Botan::X509::PEM_encode(public_key) << '\n';
 }

src/examples/pkcs11_ecdh.cpp

@@ -53,8 +53,7 @@ int main() {
 
    // set import properties
    std::vector<uint8_t> ec_point;
-   Botan::DER_Encoder(ec_point).encode(priv_key_sw.public_point().encode(Botan::EC_Point_Format::Uncompressed),
-                                       Botan::ASN1_Type::OctetString);
+   Botan::DER_Encoder(ec_point).encode(priv_key_sw.raw_public_key_bits(), Botan::ASN1_Type::OctetString);
    Botan::PKCS11::EC_PublicKeyImportProperties pub_import_props(priv_key_sw.DER_domain(), ec_point);
 
    pub_import_props.set_token(true);
@@ -103,11 +102,9 @@ int main() {
    Botan::PK_Key_Agreement ka(key_pair.second, rng, "Raw", "pkcs11");
    Botan::PK_Key_Agreement kb(key_pair_other.second, rng, "Raw", "pkcs11");
 
-   Botan::SymmetricKey alice_key =
-      ka.derive_key(32, key_pair_other.first.public_point().encode(Botan::EC_Point_Format::Uncompressed));
+   Botan::SymmetricKey alice_key = ka.derive_key(32, key_pair_other.first.raw_public_key_bits());
 
-   Botan::SymmetricKey bob_key =
-      kb.derive_key(32, key_pair.first.public_point().encode(Botan::EC_Point_Format::Uncompressed));
+   Botan::SymmetricKey bob_key = kb.derive_key(32, key_pair.first.raw_public_key_bits());
 
    bool eq = alice_key == bob_key;
 

src/examples/pkcs11_ecdsa.cpp

@@ -52,8 +52,7 @@ int main() {
 
    // import to card
    std::vector<uint8_t> ec_point;
-   Botan::DER_Encoder(ec_point).encode(priv_key_sw.public_point().encode(Botan::EC_Point_Format::Uncompressed),
-                                       Botan::ASN1_Type::OctetString);
+   Botan::DER_Encoder(ec_point).encode(priv_key_sw.raw_public_key_bits(), Botan::ASN1_Type::OctetString);
    Botan::PKCS11::EC_PublicKeyImportProperties pub_import_props(priv_key_sw.DER_domain(), ec_point);
 
    pub_import_props.set_token(true);

src/examples/tls_custom_curves_client.cpp

@@ -49,7 +49,7 @@ class Callbacks : public Botan::TLS::Callbacks {
             std::get<Botan::TLS::Group_Params>(group) == Botan::TLS::Group_Params(0xFE00)) {
             // load the peer's public key of my custom curve
             const auto ec_group = Botan::EC_Group::from_name("numsp256d1");
-            return std::make_unique<Botan::ECDH_PublicKey>(ec_group, ec_group.OS2ECP(public_value));
+            return std::make_unique<Botan::ECDH_PublicKey>(ec_group, Botan::EC_AffinePoint(ec_group, public_value));
          } else {
             // no custom curve used: up-call the default implementation
             return Botan::TLS::Callbacks::tls_deserialize_peer_public_key(group, public_value);

src/lib/ffi/ffi_pkey_algs.cpp

@@ -125,7 +125,7 @@ int pubkey_load_ec(std::unique_ptr<ECPublicKey_t>& key,
    const auto group = Botan::EC_Group::from_name(curve_name);
 
    if(auto pt = Botan::EC_AffinePoint::from_bigint_xy(group, public_x, public_y)) {
-      key.reset(new ECPublicKey_t(group, pt->to_legacy_point()));
+      key.reset(new ECPublicKey_t(group, pt.value()));
       return BOTAN_FFI_SUCCESS;
    } else {
       return BOTAN_FFI_ERROR_BAD_PARAMETER;
@@ -139,9 +139,9 @@ Botan::BigInt pubkey_get_field(const Botan::Public_Key& key, std::string_view fi
    // Not currently handled by get_int_field
    if(const Botan::EC_PublicKey* ecc = dynamic_cast<const Botan::EC_PublicKey*>(&key)) {
       if(field == "public_x") {
-         return ecc->public_point().get_affine_x();
+         return Botan::BigInt::from_bytes(ecc->_public_ec_point().x_bytes());
       } else if(field == "public_y") {
-         return ecc->public_point().get_affine_y();
+         return Botan::BigInt::from_bytes(ecc->_public_ec_point().y_bytes());
       }
    }
 #endif
@@ -158,9 +158,9 @@ Botan::BigInt privkey_get_field(const Botan::Private_Key& key, std::string_view
    // Not currently handled by get_int_field
    if(const Botan::EC_PublicKey* ecc = dynamic_cast<const Botan::EC_PublicKey*>(&key)) {
       if(field == "public_x") {
-         return ecc->public_point().get_affine_x();
+         return Botan::BigInt::from_bytes(ecc->_public_ec_point().x_bytes());
       } else if(field == "public_y") {
-         return ecc->public_point().get_affine_y();
+         return Botan::BigInt::from_bytes(ecc->_public_ec_point().y_bytes());
       }
    }
 #endif
@@ -620,7 +620,8 @@ int botan_pubkey_sm2_compute_za(
       const std::string ident_str(ident);
       std::unique_ptr<Botan::HashFunction> hash = Botan::HashFunction::create_or_throw(hash_algo);
 
-      const std::vector<uint8_t> za = Botan::sm2_compute_za(*hash, ident_str, ec_key->domain(), ec_key->public_point());
+      const std::vector<uint8_t> za =
+         Botan::sm2_compute_za(*hash, ident_str, ec_key->domain(), ec_key->_public_ec_point());
 
       return write_vec_output(out, out_len, za);
    });
@@ -1306,7 +1307,7 @@ int botan_pubkey_view_ec_public_point(const botan_pubkey_t key, botan_view_ctx c
 #if defined(BOTAN_HAS_ECC_PUBLIC_KEY_CRYPTO)
    return BOTAN_FFI_VISIT(key, [=](const auto& k) -> int {
       if(auto ecc = dynamic_cast<const Botan::EC_PublicKey*>(&k)) {
-         auto pt = ecc->public_point().encode(Botan::EC_Point_Format::Uncompressed);
+         auto pt = ecc->_public_ec_point().serialize_uncompressed();
          return invoke_view_callback(view, ctx, pt);
       } else {
          return BOTAN_FFI_ERROR_BAD_PARAMETER;

src/lib/prov/pkcs11/p11_ecc_key.cpp

@@ -102,27 +102,27 @@ PKCS11_EC_PrivateKey::PKCS11_EC_PrivateKey(Session& session,
    Object public_key(session, pub_key_handle);
 
    auto pt_bytes = public_key.get_attribute_value(AttributeType::EcPoint);
-   m_public_key = decode_public_point(m_domain_params, pt_bytes).to_legacy_point();
+   m_public_key = decode_public_point(m_domain_params, pt_bytes);
 }
 
 size_t PKCS11_EC_PrivateKey::key_length() const {
    return m_domain_params.get_order_bits();
 }
 
 std::vector<uint8_t> PKCS11_EC_PrivateKey::raw_public_key_bits() const {
-   return public_point().encode(EC_Point_Format::Compressed);
+   return public_ec_point().serialize_compressed();
 }
 
 std::vector<uint8_t> PKCS11_EC_PrivateKey::public_key_bits() const {
-   return public_point().encode(EC_Point_Format::Compressed);
+   return raw_public_key_bits();
 }
 
 size_t PKCS11_EC_PrivateKey::estimated_strength() const {
    return ecp_work_factor(key_length());
 }
 
 bool PKCS11_EC_PrivateKey::check_key(RandomNumberGenerator& /*rng*/, bool /*strong*/) const {
-   return m_public_key.on_the_curve();
+   return true;
 }
 
 AlgorithmIdentifier PKCS11_EC_PrivateKey::algorithm_identifier() const {

src/lib/prov/pkcs11/p11_ecc_key.h

@@ -138,12 +138,24 @@ class BOTAN_PUBLIC_API(2, 0) PKCS11_EC_PrivateKey : public virtual Private_Key,
       /// @returns the domain of the EC private key
       inline const EC_Group& domain() const { return m_domain_params; }
 
+   #if defined(BOTAN_HAS_LEGACY_EC_POINT)
       /**
       * Sets the associated public point of this private key
       * @param point the public point
       * @param point_encoding encoding of the point (default DER-encoded)
       */
       void set_public_point(const EC_Point& point, PublicPointEncoding point_encoding = PublicPointEncoding::Der) {
+         this->set_public_point(EC_AffinePoint(domain(), point), point_encoding);
+      }
+   #endif
+
+      /**
+      * Sets the associated public point of this private key
+      * @param point the public point
+      * @param point_encoding encoding of the point (default DER-encoded)
+      */
+      void set_public_point(const EC_AffinePoint& point,
+                            PublicPointEncoding point_encoding = PublicPointEncoding::Der) {
          m_public_key = point;
          m_point_encoding = point_encoding;
       }
@@ -155,19 +167,33 @@ class BOTAN_PUBLIC_API(2, 0) PKCS11_EC_PrivateKey : public virtual Private_Key,
        */
       void set_point_encoding(PublicPointEncoding point_encoding) { m_point_encoding = point_encoding; }
 
+   #if defined(BOTAN_HAS_LEGACY_EC_POINT)
       /**
       * Gets the public_point
       * @note the public key must be set using `set_public_point`
       *       because it is not possible to infer the public key from a PKCS#11 EC private key
       * @return the public point of the private key
       * @throws Exception if the public point was not set using set_public_point()
       */
-      const EC_Point& public_point() const {
-         if(m_public_key.is_zero()) {
+      EC_Point public_point() const { return this->public_ec_point().to_legacy_point(); }
+   #endif
+
+      /**
+      * Gets the elliptic curve point associated with the public key
+      *
+      * @note the public key must be set using `set_public_point` because it is
+      *       not possible to infer the public key from a PKCS#11 EC private key
+      *
+      * @return the public point of the private key
+      * @throws Exception if the public point was not set using set_public_point()
+      */
+      EC_AffinePoint public_ec_point() const {
+         if(m_public_key) {
+            return m_public_key.value();
+         } else {
             throw Invalid_State(
                "Public point not set. Inferring the public key from a PKCS#11 ec private key is not possible.");
          }
-         return m_public_key;
       }
 
       /// @return the encoding format for the public point when it is passed to cryptoki functions as an argument
@@ -189,7 +215,7 @@ class BOTAN_PUBLIC_API(2, 0) PKCS11_EC_PrivateKey : public virtual Private_Key,
 
    private:
       EC_Group m_domain_params;
-      EC_Point m_public_key;
+      std::optional<EC_AffinePoint> m_public_key;
       PublicPointEncoding m_point_encoding = PublicPointEncoding::Der;
 };
 }  // namespace Botan::PKCS11

src/lib/prov/pkcs11/p11_ecdh.cpp

@@ -18,7 +18,7 @@
 namespace Botan::PKCS11 {
 
 ECDH_PublicKey PKCS11_ECDH_PublicKey::export_key() const {
-   return ECDH_PublicKey(domain(), public_point());
+   return ECDH_PublicKey(domain(), _public_ec_point());
 }
 
 ECDH_PrivateKey PKCS11_ECDH_PrivateKey::export_key() const {
@@ -29,7 +29,7 @@ ECDH_PrivateKey PKCS11_ECDH_PrivateKey::export_key() const {
 }
 
 std::unique_ptr<Public_Key> PKCS11_ECDH_PrivateKey::public_key() const {
-   return std::make_unique<ECDH_PublicKey>(domain(), public_point());
+   return std::make_unique<ECDH_PublicKey>(domain(), public_ec_point());
 }
 
 secure_vector<uint8_t> PKCS11_ECDH_PrivateKey::private_key_bits() const {

src/lib/prov/pkcs11/p11_ecdh.h

@@ -95,9 +95,7 @@ class BOTAN_PUBLIC_API(2, 0) PKCS11_ECDH_PrivateKey final : public virtual PKCS1
 
       std::unique_ptr<Public_Key> public_key() const override;
 
-      inline std::vector<uint8_t> public_value() const override {
-         return public_point().encode(EC_Point_Format::Uncompressed);
-      }
+      inline std::vector<uint8_t> public_value() const override { return public_ec_point().serialize_uncompressed(); }
 
       /// @return the exported ECDH private key
       ECDH_PrivateKey export_key() const;

src/lib/prov/pkcs11/p11_ecdsa.cpp

@@ -18,19 +18,15 @@
 namespace Botan::PKCS11 {
 
 ECDSA_PublicKey PKCS11_ECDSA_PublicKey::export_key() const {
-   return ECDSA_PublicKey(domain(), public_point());
+   return ECDSA_PublicKey(domain(), _public_ec_point());
 }
 
 bool PKCS11_ECDSA_PrivateKey::check_key(RandomNumberGenerator& rng, bool strong) const {
-   if(!public_point().on_the_curve()) {
-      return false;
-   }
-
    if(!strong) {
       return true;
    }
 
-   ECDSA_PublicKey pubkey(domain(), public_point());
+   ECDSA_PublicKey pubkey(domain(), public_ec_point());
    return KeyPair::signature_consistency_check(rng, *this, pubkey, "SHA-256");
 }
 
@@ -46,7 +42,7 @@ secure_vector<uint8_t> PKCS11_ECDSA_PrivateKey::private_key_bits() const {
 }
 
 std::unique_ptr<Public_Key> PKCS11_ECDSA_PrivateKey::public_key() const {
-   return std::make_unique<ECDSA_PublicKey>(domain(), public_point());
+   return std::make_unique<ECDSA_PublicKey>(domain(), public_ec_point());
 }
 
 namespace {

src/lib/prov/tpm2/tpm2_crypto_backend/tpm2_crypto_backend_impl.cpp

@@ -655,9 +655,9 @@ TSS2_RC get_ecdh_point(TPM2B_PUBLIC* key,
 
       // Serialize public key coordinates into TPM2B_ECC_PARAMETER with Big Endian encoding.
       // This ensures bn_{x,y}.bytes() <= curve_order_byte_size.
-      const Botan::PointGFp& eph_pub_point = eph_key.public_point();
-      eph_pub_point.get_affine_x().serialize_to(Botan::TPM2::as_span(Q->x, curve_order_byte_size));
-      eph_pub_point.get_affine_y().serialize_to(Botan::TPM2::as_span(Q->y, curve_order_byte_size));
+      const auto& eph_pub_point = eph_key._public_ec_point();
+      eph_pub_point.serialize_x_to(Botan::TPM2::as_span(Q->x, curve_order_byte_size));
+      eph_pub_point.serialize_y_to(Botan::TPM2::as_span(Q->y, curve_order_byte_size));
 
       // 3: ECDH Key Agreement
       Botan::PK_Key_Agreement ecdh(eph_key, rng, "Raw" /*No KDF used here*/);

src/lib/pubkey/ec_group/ec_apoint.cpp

@@ -39,9 +39,17 @@ EC_AffinePoint::EC_AffinePoint(const EC_Group& group, std::span<const uint8_t> b
    }
 }
 
+#if defined(BOTAN_HAS_LEGACY_EC_POINT)
+
+EC_Point EC_AffinePoint::to_legacy_point() const {
+   return m_point->to_legacy_point();
+}
+
 EC_AffinePoint::EC_AffinePoint(const EC_Group& group, const EC_Point& pt) :
       EC_AffinePoint(group, pt.encode(EC_Point_Format::Uncompressed)) {}
 
+#endif
+
 bool EC_AffinePoint::operator==(const EC_AffinePoint& other) const {
    if(this == &other) {
       return true;
@@ -66,7 +74,8 @@ EC_AffinePoint EC_AffinePoint::identity(const EC_Group& group) {
 }
 
 EC_AffinePoint EC_AffinePoint::generator(const EC_Group& group) {
-   return EC_AffinePoint(group, group.get_base_point());
+   // TODO it would be nice to improve this
+   return EC_AffinePoint::from_bigint_xy(group, group.get_g_x(), group.get_g_y()).value();
 }
 
 std::optional<EC_AffinePoint> EC_AffinePoint::from_bigint_xy(const EC_Group& group, const BigInt& x, const BigInt& y) {
@@ -158,6 +167,21 @@ EC_AffinePoint EC_AffinePoint::negate() const {
    return EC_AffinePoint(std::move(pt));
 }
 
+std::vector<uint8_t> EC_AffinePoint::serialize(EC_Point_Format format) const {
+   if(format == EC_Point_Format::Compressed) {
+      return this->serialize_compressed();
+   } else if(format == EC_Point_Format::Uncompressed) {
+      return this->serialize_uncompressed();
+   } else {
+      // The deprecated "hybrid" point encoding
+      // TODO(Botan4) Remove this
+      auto enc = this->serialize_uncompressed();
+      const bool y_is_odd = (enc[enc.size() - 1] & 0x01) == 0x01;
+      enc.front() = y_is_odd ? 0x07 : 0x06;
+      return enc;
+   }
+}
+
 void EC_AffinePoint::serialize_x_to(std::span<uint8_t> bytes) const {
    BOTAN_STATE_CHECK(!this->is_identity());
    m_point->serialize_x_to(bytes);
@@ -183,10 +207,6 @@ void EC_AffinePoint::serialize_uncompressed_to(std::span<uint8_t> bytes) const {
    m_point->serialize_uncompressed_to(bytes);
 }
 
-EC_Point EC_AffinePoint::to_legacy_point() const {
-   return m_point->to_legacy_point();
-}
-
 EC_AffinePoint EC_AffinePoint::_from_inner(std::unique_ptr<EC_AffinePoint_Data> inner) {
    return EC_AffinePoint(std::move(inner));
 }

src/lib/pubkey/ec_group/ec_apoint.h

@@ -8,6 +8,7 @@
 #define BOTAN_EC_APOINT_H_
 
 #include <botan/concepts.h>
+#include <botan/ec_point_format.h>
 #include <botan/secmem.h>
 #include <botan/types.h>
 #include <optional>
@@ -21,7 +22,10 @@ class BigInt;
 class RandomNumberGenerator;
 class EC_Group;
 class EC_Scalar;
+
+#if defined(BOTAN_HAS_LEGACY_EC_POINT)
 class EC_Point;
+#endif
 
 class EC_Group_Data;
 class EC_AffinePoint_Data;
@@ -207,12 +211,16 @@ class BOTAN_UNSTABLE_API EC_AffinePoint final {
 
       bool operator!=(const EC_AffinePoint& other) const { return !(*this == other); }
 
+      /// Return an encoding depending on the requested format
+      std::vector<uint8_t> serialize(EC_Point_Format format) const;
+
       EC_AffinePoint(const EC_AffinePoint& other);
       EC_AffinePoint(EC_AffinePoint&& other) noexcept;
 
       EC_AffinePoint& operator=(const EC_AffinePoint& other);
       EC_AffinePoint& operator=(EC_AffinePoint&& other) noexcept;
 
+#if defined(BOTAN_HAS_LEGACY_EC_POINT)
       /**
       * Deprecated conversion
       */
@@ -222,6 +230,7 @@ class BOTAN_UNSTABLE_API EC_AffinePoint final {
       * Deprecated conversion
       */
       EC_Point to_legacy_point() const;
+#endif
 
       ~EC_AffinePoint();