Merge GH #1483 Use uncompressed points for ECC by default

11 files changed, 78 insertions, 50 deletions

diff --git a/src/lib/prov/bearssl/bearssl_ec.cpp b/src/lib/prov/bearssl/bearssl_ec.cpp
index 29ff1b5ad..23f566043 100644
--- a/src/lib/prov/bearssl/bearssl_ec.cpp
+++ b/src/lib/prov/bearssl/bearssl_ec.cpp
@@ -92,7 +92,7 @@ class BearSSL_ECDSA_Verification_Operation final : public PK_Ops::Verification
          if (m_hf == nullptr)
             throw Lookup_Error("BearSSL ECDSA does not support hash " + req.arg(0));
 
-         m_q_buf = EC2OSP(ecdsa.public_point(), PointGFp::UNCOMPRESSED);
+         m_q_buf = ecdsa.public_point().encode(PointGFp::UNCOMPRESSED);
 
          m_key.qlen = m_q_buf.size();
          m_key.q = m_q_buf.data();
@@ -123,7 +123,7 @@ class BearSSL_ECDSA_Verification_Operation final : public PK_Ops::Verification
    private:
       br_ec_public_key m_key;
       std::unique_ptr<HashFunction> m_hf;
-      secure_vector<uint8_t> m_q_buf;
+      std::vector<uint8_t> m_q_buf;
       const br_hash_class *m_hash;
       size_t m_order_bits;
    };
diff --git a/src/lib/prov/openssl/openssl_ec.cpp b/src/lib/prov/openssl/openssl_ec.cpp
index 90b5e2023..c61f83d02 100644
--- a/src/lib/prov/openssl/openssl_ec.cpp
+++ b/src/lib/prov/openssl/openssl_ec.cpp
@@ -56,7 +56,7 @@ secure_vector<uint8_t> PKCS8_for_openssl(const EC_PrivateKey& ec)
       .raw_bytes(ec.domain().DER_encode(EC_DOMPAR_ENC_OID))
       .end_cons()
       .start_cons(ASN1_Tag(1), PRIVATE)
-      .encode(EC2OSP(pub_key, PointGFp::UNCOMPRESSED), BIT_STRING)
+      .encode(pub_key.encode(PointGFp::UNCOMPRESSED), BIT_STRING)
       .end_cons()
       .end_cons()
       .get_contents();
@@ -147,7 +147,7 @@ class OpenSSL_ECDSA_Verification_Operation final : public PK_Ops::Verification_w
          if(!::EC_KEY_set_group(m_ossl_ec.get(), grp.get()))
             throw OpenSSL_Error("EC_KEY_set_group");
 
-         const secure_vector<uint8_t> enc = EC2OSP(ecdsa.public_point(), PointGFp::UNCOMPRESSED);
+         const std::vector<uint8_t> enc = ecdsa.public_point().encode(PointGFp::UNCOMPRESSED);
          const uint8_t* enc_ptr = enc.data();
          EC_KEY* key_ptr = m_ossl_ec.get();
          if(!::o2i_ECPublicKey(&key_ptr, &enc_ptr, enc.size()))
diff --git a/src/lib/prov/pkcs11/p11_ecc_key.cpp b/src/lib/prov/pkcs11/p11_ecc_key.cpp
index d5a9a2b76..2bcf41a2c 100644
--- a/src/lib/prov/pkcs11/p11_ecc_key.cpp
+++ b/src/lib/prov/pkcs11/p11_ecc_key.cpp
@@ -111,7 +111,7 @@ size_t PKCS11_EC_PrivateKey::key_length() const
 
 std::vector<uint8_t> PKCS11_EC_PrivateKey::public_key_bits() const
    {
-   return unlock(EC2OSP(public_point(), PointGFp::COMPRESSED));
+   return public_point().encode(PointGFp::COMPRESSED);
    }
 
 size_t PKCS11_EC_PrivateKey::estimated_strength() const
diff --git a/src/lib/prov/pkcs11/p11_ecdh.h b/src/lib/prov/pkcs11/p11_ecdh.h
index 203e5ea5f..bbef9a3e5 100644
--- a/src/lib/prov/pkcs11/p11_ecdh.h
+++ b/src/lib/prov/pkcs11/p11_ecdh.h
@@ -95,7 +95,7 @@ class BOTAN_PUBLIC_API(2,0) PKCS11_ECDH_PrivateKey final : public virtual PKCS11
 
       inline std::vector<uint8_t> public_value() const override
          {
-         return unlock(EC2OSP(public_point(), PointGFp::UNCOMPRESSED));
+         return public_point().encode(PointGFp::UNCOMPRESSED);
          }
 
       /// @return the exported ECDH private key
diff --git a/src/lib/pubkey/ec_group/ec_group.cpp b/src/lib/pubkey/ec_group/ec_group.cpp
index 5fb79c923..ccf1969d2 100644
--- a/src/lib/pubkey/ec_group/ec_group.cpp
+++ b/src/lib/pubkey/ec_group/ec_group.cpp
@@ -528,7 +528,7 @@ EC_Group::DER_encode(EC_Group_Encoding form) const
                .encode(BigInt::encode_1363(get_b(), p_bytes),
                        OCTET_STRING)
             .end_cons()
-            .encode(EC2OSP(get_base_point(), PointGFp::UNCOMPRESSED), OCTET_STRING)
+              .encode(get_base_point().encode(PointGFp::UNCOMPRESSED), OCTET_STRING)
             .encode(get_order())
             .encode(get_cofactor())
          .end_cons()
diff --git a/src/lib/pubkey/ec_group/point_gfp.cpp b/src/lib/pubkey/ec_group/point_gfp.cpp
index c71a6cffe..47ca0527d 100644
--- a/src/lib/pubkey/ec_group/point_gfp.cpp
+++ b/src/lib/pubkey/ec_group/point_gfp.cpp
@@ -618,50 +618,42 @@ bool PointGFp::operator==(const PointGFp& other) const
    }
 
 // encoding and decoding
-secure_vector<uint8_t> EC2OSP(const PointGFp& point, uint8_t format)
+std::vector<uint8_t> PointGFp::encode(PointGFp::Compression_Type format) const
    {
-   if(point.is_zero())
-      return secure_vector<uint8_t>(1); // single 0 byte
+   if(is_zero())
+      return std::vector<uint8_t>(1); // single 0 byte
 
-   const size_t p_bytes = point.get_curve().get_p().bytes();
+   const size_t p_bytes = m_curve.get_p().bytes();
 
-   BigInt x = point.get_affine_x();
-   BigInt y = point.get_affine_y();
+   const BigInt x = get_affine_x();
+   const BigInt y = get_affine_y();
 
-   secure_vector<uint8_t> bX = BigInt::encode_1363(x, p_bytes);
-   secure_vector<uint8_t> bY = BigInt::encode_1363(y, p_bytes);
+   std::vector<uint8_t> result;
 
    if(format == PointGFp::UNCOMPRESSED)
       {
-      secure_vector<uint8_t> result;
-      result.push_back(0x04);
-
-      result += bX;
-      result += bY;
-
-      return result;
+      result.resize(1 + 2*p_bytes);
+      result[0] = 0x04;
+      BigInt::encode_1363(&result[1], p_bytes, x);
+      BigInt::encode_1363(&result[1+p_bytes], p_bytes, y);
       }
    else if(format == PointGFp::COMPRESSED)
       {
-      secure_vector<uint8_t> result;
-      result.push_back(0x02 | static_cast<uint8_t>(y.get_bit(0)));
-
-      result += bX;
-
-      return result;
+      result.resize(1 + p_bytes);
+      result[0] = 0x02 | static_cast<uint8_t>(y.get_bit(0));
+      BigInt::encode_1363(&result[1], p_bytes, x);
       }
    else if(format == PointGFp::HYBRID)
       {
-      secure_vector<uint8_t> result;
-      result.push_back(0x06 | static_cast<uint8_t>(y.get_bit(0)));
-
-      result += bX;
-      result += bY;
-
-      return result;
+      result.resize(1 + 2*p_bytes);
+      result[0] = 0x06 | static_cast<uint8_t>(y.get_bit(0));
+      BigInt::encode_1363(&result[1], p_bytes, x);
+      BigInt::encode_1363(&result[1+p_bytes], p_bytes, y);
       }
    else
       throw Invalid_Argument("EC2OSP illegal point encoding");
+
+   return result;
    }
 
 namespace {
diff --git a/src/lib/pubkey/ec_group/point_gfp.h b/src/lib/pubkey/ec_group/point_gfp.h
index 6f2e34f27..81e34c634 100644
--- a/src/lib/pubkey/ec_group/point_gfp.h
+++ b/src/lib/pubkey/ec_group/point_gfp.h
@@ -99,6 +99,12 @@ class BOTAN_PUBLIC_API(2,0) PointGFp final
       PointGFp(const CurveGFp& curve, const BigInt& x, const BigInt& y);
 
       /**
+      * EC2OSP - elliptic curve to octet string primitive
+      * @param format which format to encode using
+      */
+      std::vector<uint8_t> encode(PointGFp::Compression_Type format) const;
+
+      /**
       * += Operator
       * @param rhs the PointGFp to add to the local value
       * @result resulting PointGFp
@@ -131,12 +137,6 @@ class BOTAN_PUBLIC_API(2,0) PointGFp final
          }
 
       /**
-      * Return base curve of this point
-      * @result the curve over GF(p) of this point
-      */
-      const CurveGFp& get_curve() const { return m_curve; }
-
-      /**
       * get affine x coordinate
       * @result affine x coordinate
       */
@@ -199,7 +199,7 @@ class BOTAN_PUBLIC_API(2,0) PointGFp final
 
       /**
       * Point addition - mixed J+A
-      * @param other affine point to add
+      * @param other affine point to add - assumed to be affine!
       * @param workspace temp space, at least WORKSPACE_SIZE elements
       */
       void add_affine(const PointGFp& other, std::vector<BigInt>& workspace);
@@ -226,6 +226,14 @@ class BOTAN_PUBLIC_API(2,0) PointGFp final
       */
       PointGFp zero() const { return PointGFp(m_curve); }
 
+      /**
+      * Return base curve of this point
+      * @result the curve over GF(p) of this point
+      *
+      * You should not need to use this
+      */
+      const CurveGFp& get_curve() const { return m_curve; }
+
    private:
       CurveGFp m_curve;
       BigInt m_coord_x, m_coord_y, m_coord_z;
@@ -281,7 +289,12 @@ inline PointGFp operator*(const PointGFp& point, const BigInt& scalar)
    }
 
 // encoding and decoding
-secure_vector<uint8_t> BOTAN_PUBLIC_API(2,0) EC2OSP(const PointGFp& point, uint8_t format);
+inline secure_vector<uint8_t> BOTAN_DEPRECATED("Use PointGFp::encode")
+   EC2OSP(const PointGFp& point, uint8_t format)
+   {
+   std::vector<uint8_t> enc = point.encode(static_cast<PointGFp::Compression_Type>(format));
+   return secure_vector<uint8_t>(enc.begin(), enc.end());
+   }
 
 PointGFp BOTAN_PUBLIC_API(2,0) OS2ECP(const uint8_t data[], size_t data_len,
                                       const CurveGFp& curve);
diff --git a/src/lib/pubkey/ecc_key/ecc_key.cpp b/src/lib/pubkey/ecc_key/ecc_key.cpp
index 4b591ff56..7b00d3209 100644
--- a/src/lib/pubkey/ecc_key/ecc_key.cpp
+++ b/src/lib/pubkey/ecc_key/ecc_key.cpp
@@ -68,7 +68,17 @@ AlgorithmIdentifier EC_PublicKey::algorithm_identifier() const
 
 std::vector<uint8_t> EC_PublicKey::public_key_bits() const
    {
-   return unlock(EC2OSP(public_point(), PointGFp::COMPRESSED));
+   return public_point().encode(point_encoding());
+   }
+
+void EC_PublicKey::set_point_encoding(PointGFp::Compression_Type enc)
+   {
+   if(enc != PointGFp::COMPRESSED &&
+      enc != PointGFp::UNCOMPRESSED &&
+      enc != PointGFp::HYBRID)
+      throw Invalid_Argument("Invalid point encoding for EC_PublicKey");
+
+   m_point_encoding = enc;
    }
 
 void EC_PublicKey::set_parameter_encoding(EC_Group_Encoding form)
diff --git a/src/lib/pubkey/ecc_key/ecc_key.h b/src/lib/pubkey/ecc_key/ecc_key.h
index 427be56ef..ec2b5f9be 100644
--- a/src/lib/pubkey/ecc_key/ecc_key.h
+++ b/src/lib/pubkey/ecc_key/ecc_key.h
@@ -78,6 +78,12 @@ class BOTAN_PUBLIC_API(2,0) EC_PublicKey : public virtual Public_Key
       void set_parameter_encoding(EC_Group_Encoding enc);
 
       /**
+      * Set the point encoding method to be used when encoding this key.
+      * @param enc the encoding to use
+      */
+      void set_point_encoding(PointGFp::Compression_Type enc);
+
+      /**
       * Return the DER encoding of this keys domain in whatever format
       * is preset for this particular key
       */
@@ -91,6 +97,13 @@ class BOTAN_PUBLIC_API(2,0) EC_PublicKey : public virtual Public_Key
       EC_Group_Encoding domain_format() const
          { return m_domain_encoding; }
 
+      /**
+      * Get the point encoding method to be used when encoding this key.
+      * @result the encoding to use
+      */
+      PointGFp::Compression_Type point_encoding() const
+         { return m_point_encoding; }
+
       size_t key_length() const override;
       size_t estimated_strength() const override;
 
@@ -101,6 +114,7 @@ class BOTAN_PUBLIC_API(2,0) EC_PublicKey : public virtual Public_Key
       EC_Group m_domain_params;
       PointGFp m_public_key;
       EC_Group_Encoding m_domain_encoding;
+      PointGFp::Compression_Type m_point_encoding = PointGFp::UNCOMPRESSED;
    };
 
 /**
diff --git a/src/lib/pubkey/ecdh/ecdh.h b/src/lib/pubkey/ecdh/ecdh.h
index d967c749f..f88955ac4 100644
--- a/src/lib/pubkey/ecdh/ecdh.h
+++ b/src/lib/pubkey/ecdh/ecdh.h
@@ -48,13 +48,13 @@ class BOTAN_PUBLIC_API(2,0) ECDH_PublicKey : public virtual EC_PublicKey
       * @return public point value
       */
       std::vector<uint8_t> public_value() const
-         { return unlock(EC2OSP(public_point(), PointGFp::UNCOMPRESSED)); }
+         { return public_point().encode(PointGFp::UNCOMPRESSED); }
 
       /**
       * @return public point value
       */
-      std::vector<uint8_t> public_value(PointGFp::Compression_Type type) const
-         { return unlock(EC2OSP(public_point(), static_cast<uint8_t>(type))); }
+      std::vector<uint8_t> public_value(PointGFp::Compression_Type format) const
+         { return public_point().encode(format); }
 
    protected:
       ECDH_PublicKey() = default;
diff --git a/src/lib/pubkey/ecies/ecies.cpp b/src/lib/pubkey/ecies/ecies.cpp
index 06d5cfeee..8bc4e2600 100644
--- a/src/lib/pubkey/ecies/ecies.cpp
+++ b/src/lib/pubkey/ecies/ecies.cpp
@@ -168,7 +168,7 @@ SymmetricKey ECIES_KA_Operation::derive_secret(const std::vector<uint8_t>& eph_p
       }
 
    // ISO 18033: encryption step f / decryption step h
-   secure_vector<uint8_t> other_public_key_bin = EC2OSP(other_point, static_cast<uint8_t>(m_params.compression_type()));
+   std::vector<uint8_t> other_public_key_bin = other_point.encode(m_params.compression_type());
     // Note: the argument `m_params.secret_length()` passed for `key_len` will only be used by providers because
    // "Raw" is passed to the `PK_Key_Agreement` if the implementation of botan is used.
    const SymmetricKey peh = m_ka.derive_key(m_params.domain().get_order().bytes(), other_public_key_bin.data(), other_public_key_bin.size());
@@ -247,8 +247,7 @@ ECIES_Encryptor::ECIES_Encryptor(const PK_Key_Agreement_Key& private_key,
       {
       // ISO 18033: step d 
       // convert only if necessary; m_eph_public_key_bin has been initialized with the uncompressed format
-      m_eph_public_key_bin = unlock(EC2OSP(m_params.domain().OS2ECP(m_eph_public_key_bin),
-                                           static_cast<uint8_t>(ecies_params.compression_type())));
+      m_eph_public_key_bin = m_params.domain().OS2ECP(m_eph_public_key_bin).encode(ecies_params.compression_type());
       }
    }