Topics

[PATCH v2 1/1] Disable elliptic curve when not supported by openssl

Fredrik Ternerot <fredrik.ternerot@...>
 

Disable EC support based on OPENSSL_NO_EC to allow compilation for
systems where EC is not supported by openssl.

Signed-off-by: Fredrik Ternerot <fredrikt@...>
---
create_tpm2_key.c | 32 ++++++++++++++++++++++++++++++++
e_tpm2-ecc.c | 6 ++++++
e_tpm2-rsa.c | 2 ++
e_tpm2.c | 4 ++++
e_tpm2.h | 2 ++
tpm2-common.c | 32 +++++++++++++++++++++++++++++++-
tpm2-common.h | 2 ++
7 files changed, 79 insertions(+), 1 deletion(-)

diff --git a/create_tpm2_key.c b/create_tpm2_key.c
index b7c3167..84cca34 100644
--- a/create_tpm2_key.c
+++ b/create_tpm2_key.c
@@ -297,6 +297,8 @@ void tpm2_public_template_rsa(TPMT_PUBLIC *pub)
pub->parameters.rsaDetail.scheme.scheme = TPM_ALG_NULL;
}

+#ifndef OPENSSL_NO_EC
+
void tpm2_public_template_ecc(TPMT_PUBLIC *pub, TPMI_ECC_CURVE curve)
{
pub->type = TPM_ALG_ECC;
@@ -372,6 +374,8 @@ TPM_RC openssl_to_tpm_public_ecc(TPMT_PUBLIC *pub, EVP_PKEY *pkey)
return rc;
}

+#endif /* OPENSSL_NO_EC */
+
TPM_RC openssl_to_tpm_public_rsa(TPMT_PUBLIC *pub, EVP_PKEY *pkey)
{
RSA *rsa = EVP_PKEY_get1_RSA(pkey);
@@ -419,14 +423,18 @@ TPM_RC openssl_to_tpm_public(TPM2B_PUBLIC *pub, EVP_PKEY *pkey)
switch (EVP_PKEY_type(EVP_PKEY_id(pkey))) {
case EVP_PKEY_RSA:
return openssl_to_tpm_public_rsa(tpub, pkey);
+#ifndef OPENSSL_NO_EC
case EVP_PKEY_EC:
return openssl_to_tpm_public_ecc(tpub, pkey);
+#endif
default:
break;
}
return TPM_RC_ASYMMETRIC;
}

+#ifndef OPENSSL_NO_EC
+
TPM_RC openssl_to_tpm_private_ecc(TPMT_SENSITIVE *s, EVP_PKEY *pkey)
{
const BIGNUM *pk;
@@ -458,6 +466,8 @@ TPM_RC openssl_to_tpm_private_ecc(TPMT_SENSITIVE *s, EVP_PKEY *pkey)
return rc;
}

+#endif /* OPENSSL_NO_EC */
+
TPM_RC openssl_to_tpm_private_rsa(TPMT_SENSITIVE *s, EVP_PKEY *pkey)
{
const BIGNUM *q;
@@ -487,8 +497,10 @@ TPM_RC openssl_to_tpm_private(TPMT_SENSITIVE *priv, EVP_PKEY *pkey)
switch (EVP_PKEY_type(EVP_PKEY_id(pkey))) {
case EVP_PKEY_RSA:
return openssl_to_tpm_private_rsa(priv, pkey);
+#ifndef OPENSSL_NO_EC
case EVP_PKEY_EC:
return openssl_to_tpm_private_ecc(priv, pkey);
+#endif
default:
break;
}
@@ -516,6 +528,8 @@ TPM_RC wrap_key(TPMT_SENSITIVE *s, const char *password, EVP_PKEY *pkey)
return TPM_RC_SUCCESS;
}

+#ifndef OPENSSL_NO_EC
+
static void list_curves(void)
{
TSS_CONTEXT *tssContext;
@@ -566,6 +580,8 @@ static void list_curves(void)
exit(1);
}

+#endif /* OPENSSL_NO_EC */
+
static TPM_HANDLE get_parent(const char *pstr)
{
TPM_HANDLE p;
@@ -618,7 +634,9 @@ int main(int argc, char **argv)
TPM2B_PUBLIC *pub;
TPM2B_PRIVATE *priv;
char *key = NULL, *parent_auth = NULL;
+#ifndef OPENSSL_NO_EC
TPMI_ECC_CURVE ecc = TPM_ECC_NONE;
+#endif
int rsa = -1;
uint32_t noda = TPMA_OBJECT_NODA;
TPM_HANDLE authHandle;
@@ -694,15 +712,25 @@ int main(int argc, char **argv)
rsa = 1;
break;
case 'e':
+#ifndef OPENSSL_NO_EC
ecc = tpm2_curve_name_to_TPMI(optarg);
if (ecc == TPM_ECC_NONE) {
printf("Unknown Curve\n");
exit(1);
}
+#else
+ printf("ECC not supported\n");
+ exit(1);
+#endif
break;
case 'l':
+#ifndef OPENSSL_NO_EC
list_curves();
exit(0);
+#else
+ printf("ECC not supported\n");
+ exit(1);
+#endif
case 'd':
noda = 0;
break;
@@ -736,12 +764,14 @@ int main(int argc, char **argv)
key_size = 2048;
}

+#ifndef OPENSSL_NO_EC
if (rsa == 1 && ecc != TPM_ECC_NONE) {
fprintf(stderr, "Cannot specify both --rsa and --ecc\n");
exit(1);
} else if (ecc != TPM_ECC_NONE) {
rsa = 0;
}
+#endif

dir = tpm2_set_unique_tssdir();
rc = tpm2_create(&tssContext, dir);
@@ -899,8 +929,10 @@ int main(int argc, char **argv)
cin.inPublic.publicArea.parameters.rsaDetail.exponent = 0;
cin.inPublic.publicArea.unique.rsa.t.size = 0;

+#ifndef OPENSSL_NO_EC
} else {
tpm2_public_template_ecc(&cin.inPublic.publicArea, ecc);
+#endif
}

if (policyFilename) {
diff --git a/e_tpm2-ecc.c b/e_tpm2-ecc.c
index e35b730..b0af4c7 100644
--- a/e_tpm2-ecc.c
+++ b/e_tpm2-ecc.c
@@ -5,6 +5,10 @@
*
*/

+#include <openssl/opensslconf.h>
+
+#ifndef OPENSSL_NO_EC
+
#include <stdio.h>
#include <string.h>

@@ -358,3 +362,5 @@ int tpm2_setup_ecc_methods(void)

return 1;
}
+
+#endif /* OPENSSL_NO_EC */
diff --git a/e_tpm2-rsa.c b/e_tpm2-rsa.c
index 50c2cac..f736363 100644
--- a/e_tpm2-rsa.c
+++ b/e_tpm2-rsa.c
@@ -9,7 +9,9 @@
#include <string.h>

#include <openssl/crypto.h>
+#ifndef OPENSSL_NO_EC
#include <openssl/ec.h>
+#endif
#include <openssl/engine.h>
#include <openssl/evp.h>
#include <openssl/objects.h>
diff --git a/e_tpm2.c b/e_tpm2.c
index ed2bbc2..4906aa3 100644
--- a/e_tpm2.c
+++ b/e_tpm2.c
@@ -180,9 +180,11 @@ void tpm2_bind_key_to_engine(EVP_PKEY *pkey, void *data)
case EVP_PKEY_RSA:
tpm2_bind_key_to_engine_rsa(pkey, data);
break;
+#ifndef OPENSSL_NO_EC
case EVP_PKEY_EC:
tpm2_bind_key_to_engine_ecc(pkey, data);
break;
+#endif
default:
break;
}
@@ -485,7 +487,9 @@ static int tpm2_bind_helper(ENGINE * e)
!ENGINE_set_load_pubkey_function(e, tpm2_engine_load_key) ||
!ENGINE_set_load_privkey_function(e, tpm2_engine_load_key) ||
!ENGINE_set_cmd_defns(e, tpm2_cmd_defns) ||
+#ifndef OPENSSL_NO_EC
!tpm2_setup_ecc_methods() ||
+#endif
!tpm2_setup_rsa_methods())
return 0;

diff --git a/e_tpm2.h b/e_tpm2.h
index ef9fd38..50ffba1 100644
--- a/e_tpm2.h
+++ b/e_tpm2.h
@@ -2,7 +2,9 @@
#define _E_TPM2_COMMON_H

#include "e_tpm2-rsa.h"
+#ifndef OPENSSL_NO_EC
#include "e_tpm2-ecc.h"
+#endif

#define TPM2_ENGINE_EX_DATA_UNINIT -1

diff --git a/tpm2-common.c b/tpm2-common.c
index ee3eb4c..90a8534 100644
--- a/tpm2-common.c
+++ b/tpm2-common.c
@@ -27,6 +27,9 @@ struct myTPM2B {
UINT16 s;
BYTE *const b;
};
+
+#ifndef OPENSSL_NO_EC
+
struct tpm2_ECC_Curves {
const char *name;
int nid;
@@ -220,6 +223,8 @@ struct tpm2_ECC_Curves tpm2_supported_curves[] = {
{ .name = NULL, }
};

+#endif /* OPENSSL_NO_EC */
+
void tpm2_error(TPM_RC rc, const char *reason)
{
const char *msg, *submsg, *num;
@@ -253,8 +258,12 @@ TPM_RC tpm2_load_srk(TSS_CONTEXT *tssContext, TPM_HANDLE *h, const char *auth,TP
/* no PCR state */
in.creationPCR.count = 0;

- /* public parameters for an RSA2048 key */
+ /* public parameters for an ECC/RSA key */
+#ifdef OPENSSL_NO_EC
+ in.inPublic.publicArea.type = TPM_ALG_RSA;
+#else
in.inPublic.publicArea.type = TPM_ALG_ECC;
+#endif
in.inPublic.publicArea.nameAlg = TPM_ALG_SHA256;
in.inPublic.publicArea.objectAttributes.val =
TPMA_OBJECT_NODA |
@@ -262,6 +271,16 @@ TPM_RC tpm2_load_srk(TSS_CONTEXT *tssContext, TPM_HANDLE *h, const char *auth,TP
TPMA_OBJECT_USERWITHAUTH |
TPMA_OBJECT_DECRYPT |
TPMA_OBJECT_RESTRICTED;
+#ifdef OPENSSL_NO_EC
+ in.inPublic.publicArea.parameters.rsaDetail.symmetric.algorithm = TPM_ALG_AES;
+ in.inPublic.publicArea.parameters.rsaDetail.symmetric.keyBits.aes = 128;
+ in.inPublic.publicArea.parameters.rsaDetail.symmetric.mode.aes = TPM_ALG_CFB;
+ in.inPublic.publicArea.parameters.rsaDetail.scheme.scheme = TPM_ALG_NULL;
+ in.inPublic.publicArea.parameters.rsaDetail.keyBits = 2048;
+ in.inPublic.publicArea.parameters.rsaDetail.exponent = 0;
+
+ in.inPublic.publicArea.unique.rsa.t.size = 0;
+#else
in.inPublic.publicArea.parameters.eccDetail.symmetric.algorithm = TPM_ALG_AES;
in.inPublic.publicArea.parameters.eccDetail.symmetric.keyBits.aes = 128;
in.inPublic.publicArea.parameters.eccDetail.symmetric.mode.aes = TPM_ALG_CFB;
@@ -271,6 +290,7 @@ TPM_RC tpm2_load_srk(TSS_CONTEXT *tssContext, TPM_HANDLE *h, const char *auth,TP

in.inPublic.publicArea.unique.ecc.x.t.size = 0;
in.inPublic.publicArea.unique.ecc.y.t.size = 0;
+#endif
in.inPublic.publicArea.authPolicy.t.size = 0;

/* use a bound session here because we have no known key objects
@@ -322,6 +342,8 @@ void tpm2_flush_handle(TSS_CONTEXT *tssContext, TPM_HANDLE h)
TPM_RH_NULL, NULL, 0);
}

+#ifndef OPENSSL_NO_EC
+
int tpm2_get_ecc_group(EC_KEY *eck, TPMI_ECC_CURVE curveID)
{
const int nid = tpm2_curve_name_to_nid(curveID);
@@ -444,6 +466,8 @@ static EVP_PKEY *tpm2_to_openssl_public_ecc(TPMT_PUBLIC *pub)
return NULL;
}

+#endif /* OPENSSL_NO_EC */
+
static EVP_PKEY *tpm2_to_openssl_public_rsa(TPMT_PUBLIC *pub)
{
RSA *rsa = RSA_new();
@@ -498,8 +522,10 @@ EVP_PKEY *tpm2_to_openssl_public(TPMT_PUBLIC *pub)
switch (pub->type) {
case TPM_ALG_RSA:
return tpm2_to_openssl_public_rsa(pub);
+#ifndef OPENSSL_NO_EC
case TPM_ALG_ECC:
return tpm2_to_openssl_public_ecc(pub);
+#endif
default:
break;
}
@@ -863,6 +889,8 @@ TPM_RC tpm2_ObjectPublic_GetName(TPM2B_NAME *name,
return rc;
}

+#ifndef OPENSSL_NO_EC
+
TPMI_ECC_CURVE tpm2_curve_name_to_TPMI(const char *name)
{
int i;
@@ -955,6 +983,8 @@ const char *tpm2_curve_name_to_text(TPMI_ECC_CURVE curve)
return NULL;
}

+#endif /* OPENSSL_NO_EC */
+
const char *tpm2_set_unique_tssdir(void)
{
char *prefix = getenv("XDG_RUNTIME_DIR"), *template,
diff --git a/tpm2-common.h b/tpm2-common.h
index dd46f0a..7c3133a 100644
--- a/tpm2-common.h
+++ b/tpm2-common.h
@@ -30,11 +30,13 @@ TPM_RC tpm2_SensitiveToDuplicate(TPMT_SENSITIVE *s,
TPM2B_PRIVATE *p);
TPM_RC tpm2_ObjectPublic_GetName(TPM2B_NAME *name,
TPMT_PUBLIC *tpmtPublic);
+#ifndef OPENSSL_NO_EC
TPMI_ECC_CURVE tpm2_curve_name_to_TPMI(const char *name);
int tpm2_curve_name_to_nid(TPMI_ECC_CURVE curve);
TPMI_ECC_CURVE tpm2_nid_to_curve_name(int nid);
TPMI_ECC_CURVE tpm2_get_curve_name(const EC_GROUP *g);
const char *tpm2_curve_name_to_text(TPMI_ECC_CURVE curve);
+#endif
const char *tpm2_set_unique_tssdir(void);
TPM_RC tpm2_create(TSS_CONTEXT **tsscp, const char *dir);
TPM_RC tpm2_readpublic(TSS_CONTEXT *tssContext, TPM_HANDLE handle,
--
2.11.0

James Bottomley
 

On Mon, 2018-11-05 at 09:57 +0100, Fredrik Ternerot wrote:
Disable EC support based on OPENSSL_NO_EC to allow compilation for
systems where EC is not supported by openssl.
As a general comment, I think you could lose a lot of the ifdefs. Just
because you don't support ec doesn't mean everything ec related has to
go (for instance, just keep the curve set to TPM_ECC_NONE in
create_tpm_key ... all the condition checks then do the right thing,
you don't need to eliminate them).

The other big problem is this:

[...]
@@ -253,8 +258,12 @@ TPM_RC tpm2_load_srk(TSS_CONTEXT *tssContext,
TPM_HANDLE *h, const char *auth,TP
/* no PCR state */
in.creationPCR.count = 0;

- /* public parameters for an RSA2048 key */
+ /* public parameters for an ECC/RSA key */
+#ifdef OPENSSL_NO_EC
+ in.inPublic.publicArea.type = TPM_ALG_RSA;
+#else
in.inPublic.publicArea.type = TPM_ALG_ECC;
+#endif
in.inPublic.publicArea.nameAlg = TPM_ALG_SHA256;
in.inPublic.publicArea.objectAttributes.val =
TPMA_OBJECT_NODA |
@@ -262,6 +271,16 @@ TPM_RC tpm2_load_srk(TSS_CONTEXT *tssContext,
TPM_HANDLE *h, const char *auth,TP
TPMA_OBJECT_USERWITHAUTH |
TPMA_OBJECT_DECRYPT |
TPMA_OBJECT_RESTRICTED;
+#ifdef OPENSSL_NO_EC
+ in.inPublic.publicArea.parameters.rsaDetail.symmetric.algori
thm = TPM_ALG_AES;
+ in.inPublic.publicArea.parameters.rsaDetail.symmetric.keyBit
s.aes = 128;
+ in.inPublic.publicArea.parameters.rsaDetail.symmetric.mode.a
es = TPM_ALG_CFB;
+ in.inPublic.publicArea.parameters.rsaDetail.scheme.scheme =
TPM_ALG_NULL;
+ in.inPublic.publicArea.parameters.rsaDetail.keyBits = 2048;
+ in.inPublic.publicArea.parameters.rsaDetail.exponent = 0;
+
+ in.inPublic.publicArea.unique.rsa.t.size = 0;
+#else
Primary RSA templates are really nasty: on my current laptop it takes
50 seconds simply to generate one, which is a huge amount of time to
wait for *every* key operation, so I really think if you want no-EC to
work you can't support any key type with MSO 40 parent (meaning we
create the primary on the fly).

The other reason this causes problems is that parent templates are used
for the key derivation function of the decryption key of the private
blob: use the wrong template the key won't load, so a MSO 40 parent
created with RSA only can't be loaded into a build that supports EC
(and vice versa) which will cause all sorts of support issues.

James

Fredrik Ternerot <fredrik.ternerot@...>
 

On Tue, Nov 06, 2018 at 12:32:44 -0800, James Bottomley wrote:
On Mon, 2018-11-05 at 09:57 +0100, Fredrik Ternerot wrote:
Disable EC support based on OPENSSL_NO_EC to allow compilation for
systems where EC is not supported by openssl.
As a general comment, I think you could lose a lot of the ifdefs. Just
because you don't support ec doesn't mean everything ec related has to
go (for instance, just keep the curve set to TPM_ECC_NONE in
create_tpm_key ... all the condition checks then do the right thing,
you don't need to eliminate them).
Right, this could be improved.


The other big problem is this:

[...]
@@ -253,8 +258,12 @@ TPM_RC tpm2_load_srk(TSS_CONTEXT *tssContext,
TPM_HANDLE *h, const char *auth,TP
/* no PCR state */
in.creationPCR.count = 0;

- /* public parameters for an RSA2048 key */
+ /* public parameters for an ECC/RSA key */
+#ifdef OPENSSL_NO_EC
+ in.inPublic.publicArea.type = TPM_ALG_RSA;
+#else
in.inPublic.publicArea.type = TPM_ALG_ECC;
+#endif
in.inPublic.publicArea.nameAlg = TPM_ALG_SHA256;
in.inPublic.publicArea.objectAttributes.val =
TPMA_OBJECT_NODA |
@@ -262,6 +271,16 @@ TPM_RC tpm2_load_srk(TSS_CONTEXT *tssContext,
TPM_HANDLE *h, const char *auth,TP
TPMA_OBJECT_USERWITHAUTH |
TPMA_OBJECT_DECRYPT |
TPMA_OBJECT_RESTRICTED;
+#ifdef OPENSSL_NO_EC
+ in.inPublic.publicArea.parameters.rsaDetail.symmetric.algori
thm = TPM_ALG_AES;
+ in.inPublic.publicArea.parameters.rsaDetail.symmetric.keyBit
s.aes = 128;
+ in.inPublic.publicArea.parameters.rsaDetail.symmetric.mode.a
es = TPM_ALG_CFB;
+ in.inPublic.publicArea.parameters.rsaDetail.scheme.scheme =
TPM_ALG_NULL;
+ in.inPublic.publicArea.parameters.rsaDetail.keyBits = 2048;
+ in.inPublic.publicArea.parameters.rsaDetail.exponent = 0;
+
+ in.inPublic.publicArea.unique.rsa.t.size = 0;
+#else
Primary RSA templates are really nasty: on my current laptop it takes
50 seconds simply to generate one, which is a huge amount of time to
wait for *every* key operation, so I really think if you want no-EC to
work you can't support any key type with MSO 40 parent (meaning we
create the primary on the fly).
I see. I always use MSO 81 parent so haven't really thought about this.


The other reason this causes problems is that parent templates are used
for the key derivation function of the decryption key of the private
blob: use the wrong template the key won't load, so a MSO 40 parent
created with RSA only can't be loaded into a build that supports EC
(and vice versa) which will cause all sorts of support issues.
I see. Seems a bit problematic to do this change then. Maybe I need to
handle it as a local patch for the parts relevant to me.

--
Fredrik Ternerot