noloader / cryptopp-pem

PEM parsing of keys and parameters for Crypto++ project
42 stars 31 forks source link

References to ElGamal::Private/PublicKey should be updated to ElGamalKeys::Private/PublicKey #7

Closed rbenet-lbo closed 5 years ago

rbenet-lbo commented 5 years ago

pem.h, pem_read.h and pem_write.h have references to ElGamal::Private/PublicKey type. But, in the current release of Crypto++, 8.2.0, those types are actually under ElGamal**Keys**::Private/PublicKey.

noloader commented 5 years ago

Thanks @rbenet-lbo.

I'm not sure the best way to approach it. I think there are two options. First, users can delete Load and Save for ElGamal keys. I don't think anyone uses them so it is not a great loss. Second, users can apply Commit 3d96234038b5, which will be part of Crypto++ 8.3.

What do you suggest?

rbenet-lbo commented 5 years ago

This is my local patch for PEM read (there is the equivalent for pem.h and pem_write.cpp). It compiles fine, but haven't test it functionally (we are not using ElGamal).

@@ -976,7 +976,7 @@ void PEM_Load(BufferedTransformation& bt
     PEM_LoadPrivateKey(t3, dsa);
 }

-void PEM_Load(BufferedTransformation& bt, ElGamal::PublicKey& key)
+void PEM_Load(BufferedTransformation& bt, ElGamalKeys::PublicKey& key)
 {
     ByteQueue t1, t2, t3;
     if (PEM_NextObject(bt, t1) == false)
@@ -994,12 +994,12 @@ void PEM_Load(BufferedTransformation& bt
     PEM_LoadPublicKey(t3, key, type == PEM_PUBLIC_KEY);
 }

-void PEM_Load(BufferedTransformation& bt, ElGamal::PrivateKey& key)
+void PEM_Load(BufferedTransformation& bt, ElGamalKeys::PrivateKey& key)
 {
     return PEM_Load(bt, key, NULL, 0);
 }

-void PEM_Load(BufferedTransformation& bt, ElGamal::PrivateKey& key, const char* password, size_t length)
+void PEM_Load(BufferedTransformation& bt, ElGamalKeys::PrivateKey& key, const char* password, size_t length)
 {
noloader commented 5 years ago

I think the underlying problem is, we are not doing releases. cryptopp-pem Master follows cryptopp Master. Someone using Crypto++ 8.2 should be able to download a copy of cryptopp-pem that works out of the box.

I think the way forward is, merge your patch, release and tag CRYPTOPP_8_2_0, then make the ElGamal change. That is what we do with Cmake and Autotools.

noloader commented 5 years ago

Cleared at Commit d88f17f72d69. We'll release CRYPTOPP_8_2_0 so we can make the change for Master.