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

Eddsa #324

Merged
merged 23 commits into from Apr 2, 2018
Merged

Eddsa #324

merged 23 commits into from Apr 2, 2018

Conversation

fxdupont
Copy link
Contributor

@fxdupont fxdupont commented Jun 4, 2017

It is very experimental, for instance there is no Ed448 support (it will come but it is far from to be ready, cf SHA3/SHAKE dependency), and I use internal layout of Ed25519 key pair (OpenSSL provides only ASN.1 encode/decode but I had too trouble with a similar feature for GOST)...
To play with this:

  • get OpenSSL GitHub version (no Botan yet)
  • you have 2 new mechanisms (EDDSA one shot sign/verify and key pair generation) and a new key type.
  • it is Ed448 ready for the PKCS#11 side using curve OIDs (i.e., CKA_EC_PARAMS shared with standard ECC).
  • Bind 9 rt44696 branch supports Ed25519 DNSSEC (cf RFC 8080) for both OpenSSL and PKCS#11 backends.
  • I expect to play with this at the IETF 99 Hackathon in Prague.

@bellgrim
Copy link

bellgrim commented Jun 5, 2017

Thank you for this pull request. Will have a look at it soon.

To fix the Travis CI, you also need to add --disable-eddsa to testing/travis/travis.sh for both Botan and OpenSSL. Travis do not have a crypto backend with EDDSA support yet.

@fxdupont
Copy link
Contributor Author

fxdupont commented Jun 5, 2017

I have a take 2 "eddsa2" branch nearly ready and far better (even it still requires the --disable-eddsa).
For instance I added support for X25519 key derivation.

@fxdupont
Copy link
Contributor Author

fxdupont commented Sep 2, 2017

Ed25519 is now fully supported by (GitHub/dev versions of) OpenSSL and Botan so we can resume work. As far as I know the main issue is to disable EdDSA by default.

@@ -973,6 +973,10 @@ typedef CK_ULONG CK_MECHANISM_TYPE;
#define CKM_RSA_PKCS_TPM_1_1 0x00004001UL
#define CKM_RSA_PKCS_OAEP_TPM_1_1 0x00004002UL

#define CKM_EDDSA_KEY_PAIR_GEN 0x00009040UL
#define CKM_EDDSA 0x00009041UL
#define CKM_EDDSA_DERIVE 0x00009050UL
Copy link
Contributor

Choose a reason for hiding this comment

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

Where are these mechanisms defined? Are they in any published PKCS#11 document?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nowhere/None. I sent a message to the OASIS PKCS#11 list asking if an action was planned but if I got a few "interested" answers nothing from a member of the PKCS#11 committee (so if you know one please push). BTW this falls into the IETF curdle WG scope too (i.e., it is a second way to get progress).

Copy link
Contributor

Choose a reason for hiding this comment

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

Would you be interested in writing the proposal?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes but I have no good way to push it...

Choose a reason for hiding this comment

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

Should we for now use something similar to this?

#define CKK_EDDSA (CKK_VENDOR_DEFINED|0x00008003)

@fxdupont
Copy link
Contributor Author

The situation is a bit different for SHA-3 because there is an official PKCS#11 draft document...

unsigned long OSSLEDDSA::getMinKeySize()
{
// Ed25519 is supported
return 32*8;

Choose a reason for hiding this comment

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

Botan returns number of bytes and this in bits. What should it be?

unsigned long OSSLEDDSA::getMaxKeySize()
{
// Ed448 will be supported
return 57*8;

Choose a reason for hiding this comment

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

Botan returns number of bytes and this in bits. What should it be?

@bellgrim
Copy link

To get Travis up and running:

diff --git a/README.md b/README.md
index 4584027..6e370f7 100644
--- a/README.md
+++ b/README.md
@@ -62,6 +62,7 @@ Options:
                                Disable non-paged memory for secure storage
                                (default enabled)
        --disable-ecc           Disable support for ECC (default enabled)
+       --disable-eddsa         Disable support for EDDSA (default enabled)
        --disable-gost          Disable support for GOST (default enabled)
        --disable-visibility    Disable hidden visibilty link mode [enabled]
        --with-crypto-backend   Select crypto backend (openssl|botan)
diff --git a/testing/travis/travis.sh b/testing/travis/travis.sh
index 6a1ca1f..826fbc4 100644
--- a/testing/travis/travis.sh
+++ b/testing/travis/travis.sh
@@ -6,11 +6,11 @@ CONF_OBJSTORE=""
 case $CRYPTO in
 botan)
        CONF_CRYPTO="$CONF_CRYPTO --with-crypto-backend=botan --with-botan=/usr"
-       CONF_CRYPTO="$CONF_CRYPTO --disable-ecc --disable-gost"
+       CONF_CRYPTO="$CONF_CRYPTO --disable-ecc --disable-eddsa --disable-gost"
        ;;
 openssl)
        CONF_CRYPTO="$CONF_CRYPTO --with-crypto-backend=openssl --with-openssl=/usr"
-       CONF_CRYPTO="$CONF_CRYPTO --disable-gost"
+       CONF_CRYPTO="$CONF_CRYPTO --disable-eddsa --disable-gost"
        openssl version -a
        ;;
 *)

@bellgrim
Copy link

bellgrim commented Sep 14, 2017

Cannot build using Botan. Fix it by removing inclusion of missing header.

diff --git a/src/lib/crypto/BotanCryptoFactory.cpp b/src/lib/crypto/BotanCryptoFactory.cpp
index 9257e96..0f0e4a0 100644
--- a/src/lib/crypto/BotanCryptoFactory.cpp
+++ b/src/lib/crypto/BotanCryptoFactory.cpp
@@ -54,7 +54,6 @@
 #endif
 #include "BotanHMAC.h"
 #ifdef WITH_EDDSA
-#include "BotanEDDH.h"
 #include "BotanEDDSA.h"
 #endif

@bellgrim
Copy link

Also need to apply changes for the Windows build and AppVeyor.

diff --git a/.appveyor.yml b/.appveyor.yml
index 121d805..4a46ea5 100644
--- a/.appveyor.yml
+++ b/.appveyor.yml
@@ -46,13 +46,15 @@ environment:
   - CRYPTO_BACKEND: openssl
     PACKAGE_VERSION_NAME: OpenSSL-1.0.2j
     CRYPTO_PACKAGE_NAME: openssl-1.0.2j
+    ADDITIONAL_CONFIGURE_OPTIONS: disable-eddsa
   - CRYPTO_BACKEND: botan
     PACKAGE_VERSION_NAME: Botan-1.10.13
     CRYPTO_PACKAGE_NAME: botan-1.10.13
+    ADDITIONAL_CONFIGURE_OPTIONS: disable-eddsa
   - CRYPTO_BACKEND: openssl
     PACKAGE_VERSION_NAME: OpenSSL-1.1.0b
     CRYPTO_PACKAGE_NAME: openssl-1.1.0b
-    ADDITIONAL_CONFIGURE_OPTIONS: disable-gost
+    ADDITIONAL_CONFIGURE_OPTIONS: disable-eddsa disable-gost
 install:
 - cmd: powershell -File testing/appveyor/appveyor_download_requirements.ps1
 build_script:
diff --git a/win32/Configure.py b/win32/Configure.py
index 0f7637e..ab0da6e 100644
--- a/win32/Configure.py
+++ b/win32/Configure.py
@@ -35,6 +35,7 @@ filelist = ["config.h",
 # test files
 testlist = ["botan",
             "ecc",
+            "eddsa",
             "gnump",
             "gost",
             "ossl",
@@ -66,6 +67,7 @@ condvals = {}
 
 condnames = ["BOTAN",
              "ECC",
+             "EDDSA",
              "GOST",
              "NONPAGE",
              "OPENSSL",
@@ -78,6 +80,7 @@ condnames = ["BOTAN",
 enablelist = ["64bit",
               "debug",
               "ecc",
+              "eddsa",
               "gost",
               "keep",
               "non-paged-memory",
@@ -117,6 +120,7 @@ usage + [\
 "  enable-64bit             enable 64-bit compiling [default=no]",
 "  enable-debug             enable build of Debug config [default=yes]",
 "  enable-ecc               enable support for ECC [default=yes]",
+"  enable-eddsa             enable support for EDDSA [default=yes]",
 "  enable-gost              enable support for GOST [default=yes]",
 "  enable-non-paged-memory  enable non-paged memory [default=yes]",
 "\nOptional Packages:",
@@ -140,6 +144,7 @@ unknown_value = None
 enable_keep = False
 enable_debug = True
 enable_ecc = True
+enable_eddsa = True
 enable_gost = True
 enable_non_paged = True
 platform = 32
@@ -255,6 +260,7 @@ def myenable(key, val):
     global platform
     global enable_debug
     global enable_ecc
+    global enable_eddsa
     global enable_gost
     global enable_non_paged
     global enable_keep
@@ -273,6 +279,10 @@ def myenable(key, val):
         if not val:
             enable_ecc = False
         return
+    if key.lower() == "eddsa":
+        if not val:
+            enable_eddsa = False
+        return
     if key.lower() == "gost":
         if not val:
             enable_gost = False
@@ -430,9 +440,11 @@ def doconfig():
         varvals["PLATFORM"] = "x64"
         varvals["PLATFORMDIR"] = "x64\\"
 
-    # configure ECC and GOST
+    # configure ECC, EDDSA, and GOST
     if enable_ecc:
         condvals["ECC"] = True
+    if enable_eddsa:
+        condvals["EDDSA"] = True
     if enable_gost:
         condvals["GOST"] = True
 
@@ -583,6 +595,36 @@ int main() {\n\
                 print("can't find P256: upgrade to Botan >= 1.10.6", file=sys.stderr)
                 sys.exit(1)
 
+        # Botan EDDSA support
+        if enable_eddsa:
+            if verbose:
+                print("checking Botan EDDSA support")
+            testfile = open("testeddsa.cpp", "w")
+            print('\
+#include <botan/init.h>\n\
+#include <botan/ed25519.h>\n\
+#include <botan/version.h>\n\
+int main() {\n\
+ Botan::secure_vector<uint8_t> k(32);\n\
+ try {\n\
+  Botan::Ed25519_PrivateKey* key =\n\
+   new Botan::Ed25519_PrivateKey(k);\n\
+ } catch(...) {\n\
+  return 1;\n\
+ }\n\
+ return 0;\n\
+}', file=testfile)
+            testfile.close()
+            command = ["cl", "/nologo", "/MD", "/I", inc, "testeddsa.cpp", lib]
+            command.extend(system_libs)
+            subprocess.check_output(command, stderr=subprocess.STDOUT)
+            if not os.path.exists(".\\testeddsa.exe"):
+                print("can't create .\\testeddsa.exe", file=sys.stderr)
+                sys.exit(1)
+            if subprocess.call(".\\testeddsa.exe") != 0:
+                print("can't find EDDSA: upgrade to Botan >= 2.2.0", file=sys.stderr)
+                sys.exit(1)
+
         # Botan GOST support
         if enable_gost:
             if verbose:
@@ -820,6 +862,33 @@ int main() {\n\
                 print("can't find P256, P384, or P521: no ECC support", file=sys.stderr)
                 sys.exit(1)
 
+        # OpenSSL EDDSA support
+        if enable_eddsa:
+            if verbose:
+                print("checking OpenSSL EDDSA support")
+            testfile = open("testeddsa.c", "w")
+            print('\
+#include <openssl/evp.h>\n\
+#include <openssl/objects.h>\n\
+int main()\n\
+{\n\
+ EVP_PKEY_CTX *ctx;\n\
+ ctx = EVP_PKEY_CTX_new_id(NID_ED25519, NULL);\n\
+ if (ctx == NULL)\n\
+  return 1;\n\
+ return 0;\n\
+}', file=testfile)
+            testfile.close()
+            command = ["cl", "/nologo", "/MD", "/I", inc, "testeddsa.c", lib]
+            command.extend(system_libs)
+            subprocess.check_output(command, stderr=subprocess.STDOUT)
+            if not os.path.exists(".\\testeddsa.exe"):
+                print("can't create .\\testeddsa.exe", file=sys.stderr)
+                sys.exit(1)
+            if subprocess.call(".\\testeddsa.exe") != 0:
+                print("can't find EDDSA", file=sys.stderr)
+                sys.exit(1)
+
         # OpenSSL GOST support
         if enable_gost:
             if verbose:
@@ -1052,6 +1121,10 @@ def main(args):
             print("ecc: enabled")
         else:
             print("ecc: disabled")
+        if enable_eddsa:
+            print("eddsa: enabled")
+        else:
+            print("eddsa: disabled")
         if enable_gost:
             print("gost: enabled")
         else:
@@ -1106,6 +1179,7 @@ main(sys.argv)
 # Notes: Unix configure.ac options
 #  --enable-64bit supported
 #  --enable-ecc supported
+#  --enable-eddsa supported
 #  --enable-gost supported
 #  --enable-non-paged-memory supported
 #  --enable-visibility (enforced by DLLs)
diff --git a/win32/config.h.in b/win32/config.h.in
index 1a16172..41e0927 100644
--- a/win32/config.h.in
+++ b/win32/config.h.in
@@ -110,6 +110,13 @@
 #undef WITH_ECC
 @END ECC
 
+/* Compile with EDDSA support */
+@IF EDDSA
+#define WITH_EDDSA 1
+@ELSE EDDSA
+#undef WITH_EDDSA
+@END EDDSA
+
 /* Compile with GOST support */
 @IF GOST
 #define WITH_GOST 1

@bellgrim
Copy link

Thank you @fxdupont for the great work on EDDSA. Once we have resolved the issues above, I can merge it.

@nmav
Copy link
Contributor

nmav commented Sep 20, 2017

Would it make sense waiting some time until there is a proposal by PKCS#11 wg to avoid including something that is softhsm-specific?

@jariq
Copy link
Contributor

jariq commented Sep 20, 2017

FYI PKCS#11 TC/WG is currently maintaining headers in https://github.com/czimman/pkcs11 repository. They are also reserving identifiers for new algorithms there but I think you first need to file a proposal as a member of TC/WG.

@bellgrim
Copy link

I have fixed all the open issues that I mentioned above and resolved the conflicts with the develop branch.

The current blocking issue is the PKCS#11 perspective on EDDSA.

@nmav
Copy link
Contributor

nmav commented Sep 22, 2017

There is some activity in the WG on eddsa. I'll post once there is something concrete.

@bellgrim
Copy link

There are still changes made to Eddsa in PKCS#11. The lastest was done in draft4: https://lists.oasis-open.org/archives/pkcs11/201712/msg00008.html

Will wait until it gets more stable.

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

Successfully merging this pull request may close these issues.

None yet

4 participants