Merge lp:~hipl-core/hipl/ecdsa-redhat into lp:hipl

[Miika Komu]

Compilation has been broken long time for Fedora and other RPM-based systems because they decided to drop elliptic curve support from OpenSSL. To put more heat on this bug #838116 and to make detailed code review easier, I decided to propose for early merging.

I would suggest to comment details here and design-level issues in the actual bug item:

https://bugs.launchpad.net/hipl/+bug/838116

NEW: I believe I have resolved the comments from Diego.

[Diego Biurrun (diego-biurrun)]

 review needs-fixing

On Sun, Oct 30, 2011 at 07:49:28AM +0000, Miika Komu wrote:
> Miika Komu has proposed merging lp:~hipl-core/hipl/ecdsa-redhat into lp:hipl.
>
> --- firewall/rule_management.c 2011-08-15 14:11:56 +0000
> +++ firewall/rule_management.c 2011-10-30 07:48:24 +0000
> @@ -81,8 +81,9 @@
> /* filename needs to contain one of these to be valid HI file */
> #define RSA_FILE "_rsa_"
> #define DSA_FILE "_dsa_"
> +#ifdef HAVE_EC_CRYPTO
> #define ECDSA_FILE "_ecdsa_"
> -
> +#endif /* HAVE_EC_CRYPTO */
> #define MAX_LINE_LENGTH 512

unnecessary

> @@ -444,6 +445,7 @@
> return err;
> }
>
> +#ifdef HAVE_EC_CRYPTO
> /**
> * Load an ECDSA public key from a file and convert it into a hip_host_id.
> *
> @@ -479,6 +481,8 @@
> return err;
> }
>
> +#endif /* HAVE_EC_CRYPTO */
> +
> /**
> * load a public key from a file and convert it to a hip_host_id structure
> *

Drop the empty line before the #endif, same below

> --- lib/core/builder.h 2011-08-15 14:11:56 +0000
> +++ lib/core/builder.h 2011-10-30 07:48:24 +0000
> @@ -26,18 +26,21 @@
> #ifndef HIP_LIB_CORE_BUILDER_H
> #define HIP_LIB_CORE_BUILDER_H
>
> +#include "config.h"
> +
> #include <stdint.h>
> #include <netinet/in.h>
> #include <openssl/rsa.h>
> #include <openssl/dsa.h>
> -#include <openssl/ec.h>
>
> -#include "config.h"
> #include "certtools.h"
> #include "debug.h"
> #include "icomm.h"
> #include "state.h"
>
> +#ifdef HAVE_EC_CRYPTO
> +#include <openssl/ec.h>
> +#endif /* HAVE_EC_CRYPTO */

We have system headers before local headers for a reason.

> --- lib/core/crypto.h 2011-07-18 13:10:26 +0000
> +++ lib/core/crypto.h 2011-10-30 07:48:24 +0000
> @@ -26,12 +26,16 @@
> #ifndef HIP_LIB_CORE_CRYPTO_H
> #define HIP_LIB_CORE_CRYPTO_H
>
> +#include "config.h"
> +
> #include <stdint.h>
> #include <netinet/in.h>
> #include <sys/types.h>
> #include <openssl/dsa.h>
> #include <openssl/rsa.h>
> +#ifdef HAVE_EC_CRYPTO
> #include <openssl/ec.h>
> +#endif /* HAVE_EC_CRYPTO */
> #include <openssl/dh.h>
> #include <openssl/pem.h>

.. like you did here ..

> --- lib/core/hostid.c 2011-10-25 21:14:16 +0000
> +++ lib/core/hostid.c 2011-10-30 07:48:24 +0000
> @@ -28,6 +28,8 @@
> * @brief Host identifier manipulation functions
> */
>
> +#include "config.h"
> +
> #include <errno.h>
> #include <stdint.h>
> #include <stdlib.h>
> @@ -40,7 +42,6 @@
> #include <openssl/pem.h>
> #include <openssl/rsa.h>
>
> -#include "config.h"
> #include "lib/tool/pk.h"
> #include "builder.h"
> #include "crypto.h"

unnecessary / unrelated

> @@ -689,6 +715,12 @@
> struct endpoint_hip *endpoint_ecdsa_hip = NULL;
> struct endpoint_hip *endpoint_ecdsa_pub_hip = NULL;
>
> + if (ecdsa_nid < 0) {
> + err = -1;
> + HIP_ERROR("NID for ECDSA is strange %d\n", ecdsa_nid);
> + goto out_err;
> + }

?

> @@ -1059,41 +1101,58 @@
> if (rsa_filenamebase_pub != NULL) {
> change_key_file_perms(rsa_filenamebase_pub);
> }
> - if (ecdsa_filenamebase_pub != NULL) {
> - change_key_file_perms(ecdsa_filenamebase_pub);
> - }
> - if (ecdsa_filenamebase_pub != NULL) {
> - change_key_file...

review: Needs Fixing

[Miika Komu (miika-iki)]

> review needs-fixing
>
> On Sun, Oct 30, 2011 at 07:49:28AM +0000, Miika Komu wrote:
> > Miika Komu has proposed merging lp:~hipl-core/hipl/ecdsa-redhat into
> lp:hipl.
> >
> > --- firewall/rule_management.c 2011-08-15 14:11:56 +0000
> > +++ firewall/rule_management.c 2011-10-30 07:48:24 +0000
> > @@ -81,8 +81,9 @@
> > /* filename needs to contain one of these to be valid HI file */
> > #define RSA_FILE "_rsa_"
> > #define DSA_FILE "_dsa_"
> > +#ifdef HAVE_EC_CRYPTO
> > #define ECDSA_FILE "_ecdsa_"
> > -
> > +#endif /* HAVE_EC_CRYPTO */
> > #define MAX_LINE_LENGTH 512
>
> unnecessary

Fixed.

> > @@ -444,6 +445,7 @@
> > return err;
> > }
> >
> > +#ifdef HAVE_EC_CRYPTO
> > /**
> > * Load an ECDSA public key from a file and convert it into a hip_host_id.
> > *
> > @@ -479,6 +481,8 @@
> > return err;
> > }
> >
> > +#endif /* HAVE_EC_CRYPTO */
> > +
> > /**
> > * load a public key from a file and convert it to a hip_host_id structure
> > *
>
> Drop the empty line before the #endif, same below

Done.

> > --- lib/core/builder.h 2011-08-15 14:11:56 +0000
> > +++ lib/core/builder.h 2011-10-30 07:48:24 +0000
> > @@ -26,18 +26,21 @@
> > #ifndef HIP_LIB_CORE_BUILDER_H
> > #define HIP_LIB_CORE_BUILDER_H
> >
> > +#include "config.h"
> > +
> > #include <stdint.h>
> > #include <netinet/in.h>
> > #include <openssl/rsa.h>
> > #include <openssl/dsa.h>
> > -#include <openssl/ec.h>
> >
> > -#include "config.h"
> > #include "certtools.h"
> > #include "debug.h"
> > #include "icomm.h"
> > #include "state.h"
> >
> > +#ifdef HAVE_EC_CRYPTO
> > +#include <openssl/ec.h>
> > +#endif /* HAVE_EC_CRYPTO */
>
> We have system headers before local headers for a reason.

Fixed.

> > --- lib/core/crypto.h 2011-07-18 13:10:26 +0000
> > +++ lib/core/crypto.h 2011-10-30 07:48:24 +0000
> > @@ -26,12 +26,16 @@
> > #ifndef HIP_LIB_CORE_CRYPTO_H
> > #define HIP_LIB_CORE_CRYPTO_H
> >
> > +#include "config.h"
> > +
> > #include <stdint.h>
> > #include <netinet/in.h>
> > #include <sys/types.h>
> > #include <openssl/dsa.h>
> > #include <openssl/rsa.h>
> > +#ifdef HAVE_EC_CRYPTO
> > #include <openssl/ec.h>
> > +#endif /* HAVE_EC_CRYPTO */
> > #include <openssl/dh.h>
> > #include <openssl/pem.h>
>
> .. like you did here ..
>
> > --- lib/core/hostid.c 2011-10-25 21:14:16 +0000
> > +++ lib/core/hostid.c 2011-10-30 07:48:24 +0000
> > @@ -28,6 +28,8 @@
> > * @brief Host identifier manipulation functions
> > */
> >
> > +#include "config.h"
> > +
> > #include <errno.h>
> > #include <stdint.h>
> > #include <stdlib.h>
> > @@ -40,7 +42,6 @@
> > #include <openssl/pem.h>
> > #include <openssl/rsa.h>
> >
> > -#include "config.h"
> > #include "lib/tool/pk.h"
> > #include "builder.h"
> > #include "crypto.h"
>
> unnecessary / unrelated

Removed.

> > @@ -689,6 +715,12 @@
> > struct endpoint_hip *endpoint_ecdsa_hip = NULL;
> > struct endpoint_hip *endpoint_ecdsa_pub_hip = NULL;
> >
> > + if (ecdsa_nid < 0) {
> > + err = -1;
> > + HIP_ERROR("NID for ECDSA is strange %d\n", ecdsa_nid);
> > + goto out_err;
> > + }
>
> ?

Does not compile otherwise when ECDSA is mi...

[Diego Biurrun (diego-biurrun)]

On Thu, Nov 03, 2011 at 03:00:30PM +0000, Miika Komu wrote:
> > On Sun, Oct 30, 2011 at 07:49:28AM +0000, Miika Komu wrote:
> > > Miika Komu has proposed merging lp:~hipl-core/hipl/ecdsa-redhat into
> > lp:hipl.
> > >
> > > --- test/lib/tool/pk.c 2011-07-18 13:10:10 +0000
> > > +++ test/lib/tool/pk.c 2011-10-30 07:48:24 +0000
> > > @@ -27,7 +27,9 @@
> > > #include <stdlib.h>
> > > #include <string.h>
> > > #include <stdio.h>
> > > +#ifdef HAVE_EC_CRYPTO
> > > #include <openssl/ec.h>
> > > +#endif /* HAVE_EC_CRYPTO */
> > > #include <openssl/pem.h>
> >
> > see above
>
> Did not get this.

missing config.h

Diego

[Miika Komu (miika-iki)]

Hi,

On 11/03/2011 05:27 PM, Diego Biurrun wrote:
> On Thu, Nov 03, 2011 at 03:00:30PM +0000, Miika Komu wrote:
>>> On Sun, Oct 30, 2011 at 07:49:28AM +0000, Miika Komu wrote:
>>>> Miika Komu has proposed merging lp:~hipl-core/hipl/ecdsa-redhat into
>>> lp:hipl.
>>>>
>>>> --- test/lib/tool/pk.c 2011-07-18 13:10:10 +0000
>>>> +++ test/lib/tool/pk.c 2011-10-30 07:48:24 +0000
>>>> @@ -27,7 +27,9 @@
>>>> #include<stdlib.h>
>>>> #include<string.h>
>>>> #include<stdio.h>
>>>> +#ifdef HAVE_EC_CRYPTO
>>>> #include<openssl/ec.h>
>>>> +#endif /* HAVE_EC_CRYPTO */
>>>> #include<openssl/pem.h>
>>>
>>> see above
>>
>> Did not get this.
>
> missing config.h

thanks for the correction, committed.

[lp:~hipl-core/hipl/ecdsa-redhat]

6105. By Miika Komu on 2011-11-03

Added a missing include

File test/lib/tool/pk.c was missing a include for "config.h". It's
needed due to the conditional compilation of elliptic curves in
OpenSSL.

[Diego Biurrun (diego-biurrun)]

On Thu, Nov 03, 2011 at 03:00:30PM +0000, Miika Komu wrote:
> > On Sun, Oct 30, 2011 at 07:49:28AM +0000, Miika Komu wrote:
> > > Miika Komu has proposed merging lp:~hipl-core/hipl/ecdsa-redhat into lp:hipl.
> > >
> > > --- lib/core/hostid.c 2011-10-25 21:14:16 +0000
> > > +++ lib/core/hostid.c 2011-10-30 07:48:24 +0000
> > > @@ -689,6 +715,12 @@
> > > struct endpoint_hip *endpoint_ecdsa_hip = NULL;
> > > struct endpoint_hip *endpoint_ecdsa_pub_hip = NULL;
> > >
> > > + if (ecdsa_nid < 0) {
> > > + err = -1;
> > > + HIP_ERROR("NID for ECDSA is strange %d\n", ecdsa_nid);
> > > + goto out_err;
> > > + }
> >
> > ?
>
> Does not compile otherwise when ECDSA is missing (gcc complains about
> missing variable). If you insist, I'll commit this separately to trunk
> or suggest a better fix.

You mean that gcc complains about unused parameter? This is badly designed.
The function should not need one extra parameter for each crypto algorithm
that is added.

Diego

[Diego Biurrun (diego-biurrun)]

 review needs-fixing

On Thu, Nov 03, 2011 at 03:01:41PM +0000, Miika Komu wrote:
> Miika Komu has proposed merging lp:~hipl-core/hipl/ecdsa-redhat into lp:hipl.
>
> NEW: I believe I have resolved the comments from Diego.

sort of :)

> --- firewall/rule_management.c 2011-08-15 14:11:56 +0000
> +++ firewall/rule_management.c 2011-11-03 15:00:34 +0000
> @@ -82,7 +82,6 @@
> #define RSA_FILE "_rsa_"
> #define DSA_FILE "_dsa_"
> #define ECDSA_FILE "_ecdsa_"
> -
> #define MAX_LINE_LENGTH 512

stray unrelated change

> @@ -444,6 +443,7 @@
> return err;
> }
>
> +#ifdef HAVE_EC_CRYPTO
> /**
> * Load an ECDSA public key from a file and convert it into a hip_host_id.
> *
> @@ -479,6 +479,8 @@
> return err;
> }
>
> +#endif /* HAVE_EC_CRYPTO */
> +
> /**
> * load a public key from a file and convert it to a hip_host_id structure
> *

Please drop the empty line befor the #ifdef.

same elsewhere

> --- lib/core/builder.h 2011-08-15 14:11:56 +0000
> +++ lib/core/builder.h 2011-11-03 15:00:34 +0000
> @@ -26,19 +26,22 @@
> #ifndef HIP_LIB_CORE_BUILDER_H
> #define HIP_LIB_CORE_BUILDER_H
>
> +#include "config.h"
> +
> #include <stdint.h>
> #include <netinet/in.h>
> #include <openssl/rsa.h>
> #include <openssl/dsa.h>
> +#ifdef HAVE_EC_CRYPTO
> #include <openssl/ec.h>
> -
> -#include "config.h"
> +#endif /* HAVE_EC_CRYPTO */
> +
> +
> #include "certtools.h"
> #include "debug.h"
> #include "icomm.h"
> #include "state.h"
>
> -
> /* Removed in 2.6.11 - why ? */
> extern struct hip_cert_spki_info hip_cert_spki_info;

one stray empty line inserted, one removed

> --- lib/core/crypto.h 2011-10-30 11:54:44 +0000
> +++ lib/core/crypto.h 2011-11-03 15:00:34 +0000
> @@ -92,25 +96,33 @@
> uint16_t hip_get_dh_size(uint8_t hip_dh_group_type);
> DSA *create_dsa_key(const int bits);
> RSA *create_rsa_key(const int bits);
> +#ifdef HAVE_EC_CRYPTO
> EC_KEY *create_ecdsa_key(const int nid);
> +#endif /* HAVE_EC_CRYPTO */
> int save_dsa_private_key(const char *const filenamebase, DSA *const dsa);
> int save_rsa_private_key(const char *const filenamebase, RSA *const rsa);
> +#ifdef HAVE_EC_CRYPTO
> int save_ecdsa_private_key(const char *const filenamebase, EC_KEY *const ecdsa);
> +#endif /* HAVE_EC_CRYPTO */
> int load_dsa_private_key(const char *const filenamebase, DSA **const dsa);
> int load_rsa_private_key(const char *const filename, RSA **const rsa);
> +#ifdef HAVE_EC_CRYPTO
> int load_ecdsa_private_key(const char *const filename, EC_KEY **const ec);
> +#endif /* HAVE_EC_CRYPTO */
> int impl_dsa_sign(const unsigned char *const digest,
> DSA *const dsa,
> unsigned char *const signature);
> int impl_dsa_verify(const unsigned char *const digest,
> DSA *const dsa,
> const unsigned char *const signature);
> +#ifdef HAVE_EC_CRYPTO
> int impl_ecdsa_sign(const unsigned char *const digest,
> EC_KEY *const ecdsa,
> unsigned char *const signature);
> int impl_ecdsa_verify(const unsigned char *const digest,
> EC_KEY *const ecdsa,
> const unsigned char *const signa...

review: Needs Fixing

[lp:~hipl-core/hipl/ecdsa-redhat]

6106. By Miika Komu on 2011-11-03

Cleaning up the ECDSA changes

As suggested by Diego:

* Removed unrelated changes and stray empty lines
* Reverted incorrectly deleted empty lines
* Regrouped ifdeffery
* Fixed one occurrence of config.h

[Miika Komu (miika-iki)]

On 11/03/2011 07:38 PM, Diego Biurrun wrote:
> review needs-fixing
>
> On Thu, Nov 03, 2011 at 03:01:41PM +0000, Miika Komu wrote:
>> Miika Komu has proposed merging lp:~hipl-core/hipl/ecdsa-redhat into lp:hipl.
>>
>> NEW: I believe I have resolved the comments from Diego.
>
> sort of :)
>
>> --- firewall/rule_management.c 2011-08-15 14:11:56 +0000
>> +++ firewall/rule_management.c 2011-11-03 15:00:34 +0000
>> @@ -82,7 +82,6 @@
>> #define RSA_FILE "_rsa_"
>> #define DSA_FILE "_dsa_"
>> #define ECDSA_FILE "_ecdsa_"
>> -
>> #define MAX_LINE_LENGTH 512
>
> stray unrelated change

Fixed.

>> @@ -444,6 +443,7 @@
>> return err;
>> }
>>
>> +#ifdef HAVE_EC_CRYPTO
>> /**
>> * Load an ECDSA public key from a file and convert it into a hip_host_id.
>> *
>> @@ -479,6 +479,8 @@
>> return err;
>> }
>>
>> +#endif /* HAVE_EC_CRYPTO */
>> +
>> /**
>> * load a public key from a file and convert it to a hip_host_id structure
>> *
>
> Please drop the empty line befor the #ifdef.
>
> same elsewhere

I thought this was the policy, observe bzr commit:

The following differences were found between the code to commit and the
rules in '.uncrustify-0.57.cfg':

---
/home/mkomu/projects/hipl-bzr/ecdsa-redhat/firewall/rule_management.c
2011-11-03 23:08:00.909848709 +0200
+++ - 2011-11-03 23:29:16.724968313 +0200
@@ -479,6 +479,7 @@
      free(ecdsa_key_rr);
      return err;
  }
+
  #endif /* HAVE_EC_CRYPTO */

  /**
--- /home/mkomu/projects/hipl-bzr/ecdsa-redhat/hipd/hidb.c 2011-11-03
23:11:13.994806140 +0200
+++ - 2011-11-03 23:29:16.747797056 +0200
@@ -103,6 +103,7 @@

      return 0;
  }
+
  #endif /* HAVE_EC_CRYPTO */

  /**
--- /home/mkomu/projects/hipl-bzr/ecdsa-redhat/lib/core/builder.c
2011-11-03 23:14:08.179669916 +0200
+++ - 2011-11-03 23:29:16.766928680 +0200
@@ -3505,6 +3505,7 @@
      *endpoint = NULL;
      return err;
  }
+
  #endif /* HAVE_EC_CRYPTO */

  /**
--- /home/mkomu/projects/hipl-bzr/ecdsa-redhat/lib/core/crypto.c
2011-11-03 23:16:09.272270365 +0200
+++ - 2011-11-03 23:29:16.831275760 +0200
@@ -495,6 +495,7 @@
      ECDSA_SIG_free(ecdsa_sig);
      return err;
  }
+
  #endif /* HAVE_EC_CRYPTO */

  /**
@@ -573,6 +574,7 @@
      ECDSA_SIG_free(ecdsa_sig);
      return err;
  }
+
  #endif /* HAVE_EC_CRYPTO */

  /**
@@ -823,6 +825,7 @@
      EC_GROUP_free(group);
      return err;
  }
+
  #endif /* HAVE_EC_CRYPTO */

  /**
@@ -1176,6 +1179,7 @@

      return 0;
  }
+
  #endif /* HAVE_EC_CRYPTO */

  /**
--- /home/mkomu/projects/hipl-bzr/ecdsa-redhat/lib/core/hostid.c
2011-11-03 23:21:15.901790851 +0200
+++ - 2011-11-03 23:29:16.870163381 +0200
@@ -291,6 +291,7 @@

      return 0;
  }
+
  #endif /* HAVE_EC_CRYPTO */

  /**
@@ -427,6 +428,7 @@

      return 0;
  }
+
  #endif /* HAVE_EC_CRYPTO */

  /**
@@ -638,6 +640,7 @@

      return ret;
  }
+
  #endif /* HAVE_EC_CRYPTO */

  /**
@@ -1202,6 +1205,7 @@
      free(buffer);
      return err;
  }
+
  #endif /* HAVE_EC_CRYPTO */

  /**
--- /home/mkomu/projects/hipl-bzr/ecdsa-redhat/lib/tool/pk.c 2011-11-03
23:22:35.954187819 +0200
+++ - 2011-11-03 23:29:16.912158459 +0200
@@ -132,6 +132,7 @@

      return 0;
  }
+
  #endif /* HAVE_EC_CRYPTO */

  /**
@@...

[Diego Biurrun (diego-biurrun)]

On Thu, Nov 03, 2011 at 11:37:40PM +0200, Miika Komu wrote:
> On 11/03/2011 07:38 PM, Diego Biurrun wrote:
> >On Thu, Nov 03, 2011 at 03:01:41PM +0000, Miika Komu wrote:
> >>--- firewall/rule_management.c 2011-08-15 14:11:56 +0000
> >>+++ firewall/rule_management.c 2011-11-03 15:00:34 +0000
> >>@@ -444,6 +443,7 @@
> >> return err;
> >> }
> >>
> >>+#ifdef HAVE_EC_CRYPTO
> >> /**
> >> * Load an ECDSA public key from a file and convert it into a hip_host_id.
> >> *
> >>@@ -479,6 +479,8 @@
> >> return err;
> >> }
> >>
> >>+#endif /* HAVE_EC_CRYPTO */
> >>+
> >> /**
> >> * load a public key from a file and convert it to a hip_host_id structure
> >> *
> >
> >Please drop the empty line befor the #ifdef.
> >
> >same elsewhere
>
> I thought this was the policy, observe bzr commit:
>
> The following differences were found between the code to commit and
> the rules in '.uncrustify-0.57.cfg':
>
> ---
> /home/mkomu/projects/hipl-bzr/ecdsa-redhat/firewall/rule_management.c
> 2011-11-03 23:08:00.909848709 +0200
> +++ - 2011-11-03 23:29:16.724968313 +0200
> @@ -479,6 +479,7 @@
> free(ecdsa_key_rr);
> return err;
> }
> +
> #endif /* HAVE_EC_CRYPTO */
>
> /**
>
> The above changes are also available in the file /tmp/stylecheck-diff-L4x2dJ
>
> What to do about this... any help?

It seems our uncrustify configuration is in need of fixing.
I'll look into it next week - harass me if I don't.

Diego

[René Hummen (rene-hummen)]

Apart from Diego's comments, this looks good to me. Did you check "make doxygen", "make check", and "make alltests"? Especially, "make alltests" may through errors as you ifdef the ecdsa.h include but node code in test/lib/tool/pk.c.

review: Needs Information

[René Hummen (rene-hummen)]

Sorry, "make check" might be the trouble-maker.

[Miika Komu (miika-iki)]

Good catch. All tests work on Ubuntu but check and alltests fail on Fedora 15:

[mkomu@hipserver hipl-1.0.6]$ make check
make check-am
make[1]: Entering directory `/home/mkomu/temp/hipl-1.0.6'
make test/check_firewall test/check_lib_core test/check_lib_tool test/check_modules_midauth
make[2]: Entering directory `/home/mkomu/temp/hipl-1.0.6'
  CC test/check_firewall.o
  CC test/mocks.o
  CC test/firewall/conntrack.o
  CC test/firewall/file_buffer.o
  CC test/firewall/helpers.o
  CC test/firewall/line_parser.o
  CC test/firewall/port_bindings.o
  CCLD test/check_firewall
  CC test/check_lib_core.o
  CC test/lib/core/crypto.o
test/lib/core/crypto.c: In function ‘test_create_ecdsa_key_invalid_id’:
test/lib/core/crypto.c:40:5: error: implicit declaration of function ‘create_ecdsa_key’ [-Werror=implicit-function-declaration]
test/lib/core/crypto.c:40:5: error: comparison between pointer and integer [-Werror]
test/lib/core/crypto.c:41:5: error: comparison between pointer and integer [-Werror]
test/lib/core/crypto.c:42:5: error: comparison between pointer and integer [-Werror]
test/lib/core/crypto.c:43:5: error: comparison between pointer and integer [-Werror]
test/lib/core/crypto.c: In function ‘test_create_ecdsa_key’:
test/lib/core/crypto.c:53:5: error: unknown type name ‘EC_KEY’
test/lib/core/crypto.c:59:9: error: assignment makes pointer from integer without a cast [-Werror]
test/lib/core/crypto.c:64:9: error: implicit declaration of function ‘EC_KEY_check_key’ [-Werror=implicit-function-declaration]
test/lib/core/crypto.c:65:9: error: implicit declaration of function ‘EC_KEY_free’ [-Werror=implicit-function-declaration]
test/lib/core/crypto.c: In function ‘test_create_different_ecdsa_keys’:
test/lib/core/crypto.c:76:5: error: unknown type name ‘EC_KEY’
test/lib/core/crypto.c:83:20: error: assignment makes pointer from integer without a cast [-Werror]
test/lib/core/crypto.c:85:9: error: implicit declaration of function ‘EVP_PKEY_assign_EC_KEY’ [-Werror=implicit-function-declaration]
test/lib/core/crypto.c: In function ‘test_save_invalid_ecdsa_key’:
test/lib/core/crypto.c:98:5: error: unknown type name ‘EC_KEY’
test/lib/core/crypto.c:101:5: error: implicit declaration of function ‘save_ecdsa_private_key’ [-Werror=implicit-function-declaration]
test/lib/core/crypto.c:103:11: error: assignment makes pointer from integer without a cast [-Werror]
test/lib/core/crypto.c:107:5: error: implicit declaration of function ‘EC_KEY_new’ [-Werror=implicit-function-declaration]
test/lib/core/crypto.c:107:11: error: assignment makes pointer from integer without a cast [-Werror]
test/lib/core/crypto.c: In function ‘test_load_save_ecdsa_key’:
test/lib/core/crypto.c:123:5: error: unknown type name ‘EC_KEY’
test/lib/core/crypto.c:124:5: error: unknown type name ‘EC_KEY’
test/lib/core/crypto.c:132:24: error: assignment makes pointer from integer without a cast [-Werror]
test/lib/core/crypto.c:137:5: error: implicit declaration of function ‘EVP_PKEY_get1_EC_KEY’ [-Werror=implicit-function-declaration]
test/lib/core/crypto.c:140:5: error: implicit declaration of function ‘load_ecdsa_private_key’ [-Werror=implicit-function-declarat...

[Diego Biurrun (diego-biurrun)]

On Thu, Nov 03, 2011 at 10:07:31PM +0000, Diego Biurrun wrote:
> On Thu, Nov 03, 2011 at 11:37:40PM +0200, Miika Komu wrote:
> > On 11/03/2011 07:38 PM, Diego Biurrun wrote:
> > >On Thu, Nov 03, 2011 at 03:01:41PM +0000, Miika Komu wrote:
> > >>--- firewall/rule_management.c 2011-08-15 14:11:56 +0000
> > >>+++ firewall/rule_management.c 2011-11-03 15:00:34 +0000
> > >>@@ -444,6 +443,7 @@
> > >>
> > >>+#ifdef HAVE_EC_CRYPTO
> > >> /**
> > >> * Load an ECDSA public key from a file and convert it into a hip_host_id.
> > >> *
> > >>@@ -479,6 +479,8 @@
> > >> return err;
> > >> }
> > >>
> > >>+#endif /* HAVE_EC_CRYPTO */
> > >>+
> > >> /**
> > >> * load a public key from a file and convert it to a hip_host_id structure
> > >> *
> > >
> > >Please drop the empty line befor the #ifdef.
> > >
> > >same elsewhere
> >
> > I thought this was the policy, observe bzr commit:
> >
> > The following differences were found between the code to commit and
> > the rules in '.uncrustify-0.57.cfg':
> >
> > ---
> > /home/mkomu/projects/hipl-bzr/ecdsa-redhat/firewall/rule_management.c
> > 2011-11-03 23:08:00.909848709 +0200
> > +++ - 2011-11-03 23:29:16.724968313 +0200
> > @@ -479,6 +479,7 @@
> > free(ecdsa_key_rr);
> > return err;
> > }
> > +
> > #endif /* HAVE_EC_CRYPTO */
> >
> > /**
> >
> > The above changes are also available in the file /tmp/stylecheck-diff-L4x2dJ
> >
> > What to do about this... any help?
>
> It seems our uncrustify configuration is in need of fixing.
> I'll look into it next week - harass me if I don't.

It's the following setting:

  # The number of newlines after '}' of a multi-line function body
  nl_after_func_body = 2 # number

This will force an empty line between functions. It does not take #endifs
into account, however. I don't much care how we handle this. We can
either live with the newline before the #endif or drop the setting.
Make your choice.

Diego

[lp:~hipl-core/hipl/ecdsa-redhat]

6107. By Miika Komu on 2011-11-09

Syncronized with trunk revision 6119

6108. By Miika Komu on 2011-11-09

Deleted some empty lines between function bodies and #endif statements

According to the new crustify policy, there is no need need to have an
empty line between the end of a function body (closing curly bracket)
and following #endif (if any present). Adjusted ECDSA-related code
according to the new policy.

[Miika Komu (miika-iki)]

Hi,

On 08/11/11 18:16, Diego Biurrun wrote:
> On Thu, Nov 03, 2011 at 10:07:31PM +0000, Diego Biurrun wrote:
>> On Thu, Nov 03, 2011 at 11:37:40PM +0200, Miika Komu wrote:
>>> On 11/03/2011 07:38 PM, Diego Biurrun wrote:
>>>> On Thu, Nov 03, 2011 at 03:01:41PM +0000, Miika Komu wrote:
>>>>> --- firewall/rule_management.c 2011-08-15 14:11:56 +0000
>>>>> +++ firewall/rule_management.c 2011-11-03 15:00:34 +0000
>>>>> @@ -444,6 +443,7 @@
>>>>>
>>>>> +#ifdef HAVE_EC_CRYPTO
>>>>> /**
>>>>> * Load an ECDSA public key from a file and convert it into a hip_host_id.
>>>>> *
>>>>> @@ -479,6 +479,8 @@
>>>>> return err;
>>>>> }
>>>>>
>>>>> +#endif /* HAVE_EC_CRYPTO */
>>>>> +
>>>>> /**
>>>>> * load a public key from a file and convert it to a hip_host_id structure
>>>>> *
>>>>
>>>> Please drop the empty line befor the #ifdef.
>>>>
>>>> same elsewhere
>>>
>>> I thought this was the policy, observe bzr commit:
>>>
>>> The following differences were found between the code to commit and
>>> the rules in '.uncrustify-0.57.cfg':
>>>
>>> ---
>>> /home/mkomu/projects/hipl-bzr/ecdsa-redhat/firewall/rule_management.c
>>> 2011-11-03 23:08:00.909848709 +0200
>>> +++ - 2011-11-03 23:29:16.724968313 +0200
>>> @@ -479,6 +479,7 @@
>>> free(ecdsa_key_rr);
>>> return err;
>>> }
>>> +
>>> #endif /* HAVE_EC_CRYPTO */
>>>
>>> /**
>>>
>>> The above changes are also available in the file /tmp/stylecheck-diff-L4x2dJ
>>>
>>> What to do about this... any help?
>>
>> It seems our uncrustify configuration is in need of fixing.
>> I'll look into it next week - harass me if I don't.
>
> It's the following setting:
>
> # The number of newlines after '}' of a multi-line function body
> nl_after_func_body = 2 # number
>
> This will force an empty line between functions. It does not take #endifs
> into account, however. I don't much care how we handle this. We can
> either live with the newline before the #endif or drop the setting.
> Make your choice.

done, changed this to "1" and adjusted trunk code to match this new policy.

=== modified file 'firewall/conntrack.c'
--- firewall/conntrack.c	2011-11-08 18:22:33 +0000
+++ firewall/conntrack.c	2011-11-09 06:56:25 +0000
@@ -695,9 +695,11 @@
                 case HIP_HI_RSA:
                     RSA_free(hip_tuple->data->src_pub_key);
                     break;
+#ifdef HAVE_EC_CRYPTO
                 case HIP_HI_ECDSA:
                     EC_KEY_free(hip_tuple->data->src_pub_key);
                     break;
+#endif /* HAVE_EC_CRYPTO */
                 case HIP_HI_DSA:
                     DSA_free(hip_tuple->data->src_pub_key);
                     break;
@@ -1070,10 +1072,12 @@
         tuple->hip_tuple->data->src_pub_key = hip_key_rr_to_rsa((const struct hip_host_id_priv *) host_id, 0);
         tuple->hip_tuple->data->verify      = hip_rsa_verify;
         break;
+#ifdef HAVE_EC_CRYPTO
     case HIP_HI_ECDSA:
         tuple->hip_tuple->data->src_pub_key = hip_key_rr_to_ecdsa((const struct hip_host_id_priv *) host_id, 0);
         tuple->hip_tuple->data->verify      = hip_ecdsa_verify;
         break;
+#endif /* HAVE_EC_CRYPTO */
     case HIP_HI_DSA:
         tuple->hip_tuple->data->src_pub_key = hip_key_rr_to_dsa((const struct hip_host_id_priv *) host_id, 0);
         tuple->hip_tuple->data->verify      = hip_dsa_verify;

=== modified file 'firewall/rule_management.c'
--- firewall/rule_management.c	2011-08-15 14:11:56 +0000
+++ firewall/rule_management.c	2011-11-09 06:56:25 +0000
@@ -444,6 +444,7 @@
     return err;
 }
 
+#ifdef HAVE_EC_CRYPTO
 /**
  * Load an ECDSA public key from a file and convert it into a hip_host_id.
  *
@@ -478,6 +479,7 @@
     free(ecdsa_key_rr);
     return err;
 }
+#endif /* HAVE_EC_CRYPTO */
 
 /**
  * load a public key from a file and convert it to a hip_host_id structure
@@ -506,8 +508,10 @@
         algo = HIP_HI_RSA;
     } else if (strstr(token, DSA_FILE)) {
         algo = HIP_HI_DSA;
+#ifdef HAVE_EC_CRYPTO
     } else if (strstr(token, ECDSA_FILE)) {
         algo = HIP_HI_ECDSA;
+#endif /* HAVE_EC_CRYPTO */
     } else {
         HIP_DEBUG("Invalid filename for HI: missing _rsa_ or _dsa_ \n");
         return NULL;
@@ -519,9 +523,11 @@
     case HIP_HI_RSA:
         HIP_IFEL(load_rsa_file(fp, hi), -1, "Failed to load RSA key\n");
         break;
+#ifdef HAVE_EC_CRYPTO
     case HIP_HI_ECDSA:
         HIP_IFEL(load_ecdsa_file(fp, hi), -1, "Failed to load ECDSA key\n")
         break;
+#endif /* HAVE_EC_CRYPTO */
     case HIP_HI_DSA:
         HIP_IFEL(load_dsa_file(fp, hi), -1, "Failed to load DSA key\n")
         break;

=== modified file 'hipd/cookie.c'
--- hipd/cookie.c	2011-10-25 21:14:16 +0000
+++ hipd/cookie.c	2011-11-09 06:56:25 +0000
@@ -349,9 +349,11 @@
     case HIP_HI_DSA:
         signature_func = hip_dsa_sign;
         break;
+#ifdef HAVE_EC_CRYPTO
     case HIP_HI_ECDSA:
         signature_func = hip_ecdsa_sign;
         break;
+#endif /* HAVE_EC_CRYPTO */
     default:
         HIP_ERROR("Unkown algorithm");
         return -1;

=== modified file 'hipd/hadb.c'
--- hipd/hadb.c	2011-11-09 06:32:02 +0000
+++ hipd/hadb.c	2011-11-09 06:56:25 +0000
@@ -818,9 +818,11 @@
         case HIP_HI_RSA:
             RSA_free(ha->peer_pub_key);
             break;
+#ifdef HAVE_EC_CRYPTO
         case HIP_HI_ECDSA:
             EC_KEY_free(ha->peer_pub_key);
             break;
+#endif /* HAVE_EC_CRYPTO */
         case HIP_HI_DSA:
             DSA_free(ha->peer_pub_key);
             break;
@@ -909,10 +911,12 @@
         entry->verify       = hip_dsa_verify;
         entry->peer_pub_key = hip_key_rr_to_dsa((struct hip_host_id_priv *) entry->peer_pub, 0);
         break;
+#ifdef HAVE_EC_CRYPTO
     case HIP_HI_ECDSA:
         entry->verify       = hip_ecdsa_verify;
         entry->peer_pub_key = hip_key_rr_to_ecdsa((struct hip_host_id_priv *) entry->peer_pub, 0);
         break;
+#endif /* HAVE_EC_CRYPTO */
     default:
         HIP_OUT_ERR(-1, "Unkown algorithm");
     }
@@ -978,9 +982,11 @@
     case HIP_HI_RSA:
         entry->sign = hip_rsa_sign;
         break;
+#ifdef HAVE_EC_CRYPTO
     case HIP_HI_ECDSA:
         entry->sign = hip_ecdsa_sign;
         break;
+#endif /* HAVE_EC_CRYPTO */
     default:
         err = -1;
     }

=== modified file 'hipd/hidb.c'
--- hipd/hidb.c	2011-10-25 21:14:16 +0000
+++ hipd/hidb.c	2011-11-09 06:56:25 +0000
@@ -63,6 +63,7 @@
 
 static const char *lsi_addresses[] = { "1.0.0.1", "1.0.0.2", "1.0.0.3", "1.0.0.4" };
 
+#ifdef HAVE_EC_CRYPTO
 /**
  * Strips the private key component from an ECDSA-based host id.
  *
@@ -102,6 +103,7 @@
 
     return 0;
 }
+#endif /* HAVE_EC_CRYPTO */
 
 /**
  * Strips a DSA public key out of a host id with private key component
@@ -192,8 +194,10 @@
         return get_rsa_public_key(hid, ret);
     case HIP_HI_DSA:
         return get_dsa_public_key(hid, ret);
+#ifdef HAVE_EC_CRYPTO
     case HIP_HI_ECDSA:
         return get_ecdsa_public_key(hid, ret);
+#endif /* HAVE_EC_CRYPTO */
     default:
         HIP_ERROR("Unsupported HI algorithm\n");
         return -1;
@@ -284,9 +288,11 @@
     case HIP_HI_RSA:
         RSA_free(id->private_key);
         break;
+#ifdef HAVE_EC_CRYPTO
     case HIP_HI_ECDSA:
         EC_KEY_free(id->private_key);
         break;
+#endif /* HAVE_EC_CRYPTO */
     case HIP_HI_DSA:
         DSA_free(id->private_key);
         break;
@@ -510,9 +516,11 @@
     case HIP_HI_RSA:
         id_entry->private_key = hip_key_rr_to_rsa(host_id, 1);
         break;
+#ifdef HAVE_EC_CRYPTO
     case HIP_HI_ECDSA:
         id_entry->private_key = hip_key_rr_to_ecdsa(host_id, 1);
         break;
+#endif /* HAVE_EC_CRYPTO */
     case HIP_HI_DSA:
         id_entry->private_key = hip_key_rr_to_dsa(host_id, 1);
         break;
@@ -530,9 +538,11 @@
     case HIP_HI_DSA:
         signature_func = hip_dsa_sign;
         break;
+#ifdef HAVE_EC_CRYPTO
     case HIP_HI_ECDSA:
         signature_func = hip_ecdsa_sign;
         break;
+#endif /* HAVE_EC_CRYPTO */
     default:
         HIP_ERROR("Unsupported algorithms\n");
         err = -1;
@@ -558,9 +568,11 @@
         case HIP_HI_RSA:
             RSA_free(id_entry->private_key);
             break;
+#ifdef HAVE_EC_CRYPTO
         case HIP_HI_ECDSA:
             EC_KEY_free(id_entry->private_key);
             break;
+#endif /* HAVE_EC_CRYPTO */
         case HIP_HI_DSA:
             DSA_free(id_entry->private_key);
             break;

=== modified file 'lib/core/builder.c'
--- lib/core/builder.c	2011-11-09 06:32:02 +0000
+++ lib/core/builder.c	2011-11-09 06:56:25 +0000
@@ -3459,6 +3459,7 @@
     return hip_build_param(msg, &name_info);
 }
 
+#ifdef HAVE_EC_CRYPTO
 /**
  * Convert an EC structure from OpenSSL into an endpoint_hip structure
  * used internally by the implementation.
@@ -3503,6 +3504,7 @@
     *endpoint = NULL;
     return err;
 }
+#endif /* HAVE_EC_CRYPTO */
 
 /**
  * Convert a DSA structure from OpenSSL into an endpoint_hip structure
@@ -3605,7 +3607,9 @@
     struct hip_host_id      *host_id_pub = NULL;
     const RSA *const         rsa_key     = any_key;
     const DSA *const         dsa_key     = any_key;
-    const EC_KEY *const      ecdsa_key   = any_key;
+#ifdef HAVE_EC_CRYPTO
+    const EC_KEY *const ecdsa_key = any_key;
+#endif /* HAVE_EC_CRYPTO */
 
     HIP_IFEL(gethostname(hostname, sizeof(hostname)), -1,
              "gethostname failed\n");
@@ -3615,10 +3619,12 @@
         HIP_IFEL((key_rr_len = dsa_to_dns_key_rr(dsa_key, &key_rr)) <= 0, -1,
                  "key_rr_len\n");
         break;
+#ifdef HAVE_EC_CRYPTO
     case HIP_HI_ECDSA:
         HIP_IFEL(((key_rr_len = ecdsa_to_key_rr(ecdsa_key, &key_rr)) <= 0), -1,
                  "key_rr_len\n");
         break;
+#endif /* HAVE_EC_CRYPTO */
     case HIP_HI_RSA:
         HIP_IFEL((key_rr_len = rsa_to_dns_key_rr(rsa_key, &key_rr)) <= 0, -1,
                  "key_rr_len\n");

=== modified file 'lib/core/builder.h'
--- lib/core/builder.h	2011-08-15 14:11:56 +0000
+++ lib/core/builder.h	2011-11-09 06:56:25 +0000
@@ -26,13 +26,16 @@
 #ifndef HIP_LIB_CORE_BUILDER_H
 #define HIP_LIB_CORE_BUILDER_H
 
+#include "config.h"
+
 #include <stdint.h>
 #include <netinet/in.h>
 #include <openssl/rsa.h>
 #include <openssl/dsa.h>
+#ifdef HAVE_EC_CRYPTO
 #include <openssl/ec.h>
+#endif /* HAVE_EC_CRYPTO */
 
-#include "config.h"
 #include "certtools.h"
 #include "debug.h"
 #include "icomm.h"
@@ -219,10 +222,12 @@
                         struct endpoint_hip **endpoint,
                         se_hip_flags endpoint_flags,
                         const char *const hostname);
+#ifdef HAVE_EC_CRYPTO
 int ecdsa_to_hip_endpoint(const EC_KEY *const ecdsa,
                           struct endpoint_hip **const endpoint,
                           const se_hip_flags endpoint_flags,
                           const char *const hostname);
+#endif /* HAVE_EC_CRYPTO */
 int hip_any_key_to_hit(const void *const any_key,
                        hip_hit_t *const hit,
                        const int is_public,

=== modified file 'lib/core/crypto.c'
--- lib/core/crypto.c	2011-09-21 14:22:17 +0000
+++ lib/core/crypto.c	2011-11-09 06:56:25 +0000
@@ -459,6 +459,7 @@
     return err;
 }
 
+#ifdef HAVE_EC_CRYPTO
 /**
  * Sign using ECDSA
  *
@@ -494,6 +495,7 @@
     ECDSA_SIG_free(ecdsa_sig);
     return err;
 }
+#endif /* HAVE_EC_CRYPTO */
 
 /**
  * Sign using DSA
@@ -536,6 +538,7 @@
     return err;
 }
 
+#ifdef HAVE_EC_CRYPTO
 /**
  * Verify an ECDSA signature
  *
@@ -570,6 +573,7 @@
     ECDSA_SIG_free(ecdsa_sig);
     return err;
 }
+#endif /* HAVE_EC_CRYPTO */
 
 /**
  * Verify a DSA signature
@@ -780,6 +784,7 @@
     return NULL;
 }
 
+#ifdef HAVE_EC_CRYPTO
 /**
  * Generate ECDSA parameters and a new key pair.
  *
@@ -818,6 +823,7 @@
     EC_GROUP_free(group);
     return err;
 }
+#endif /* HAVE_EC_CRYPTO */
 
 /**
  * Save host DSA keys to disk.
@@ -1015,6 +1021,7 @@
     return err;
 }
 
+#ifdef HAVE_EC_CRYPTO
 /**
  * Save the host's ECDSA keys to disk.
  *
@@ -1169,6 +1176,7 @@
 
     return 0;
 }
+#endif /* HAVE_EC_CRYPTO */
 
 /**
  * Load host DSA private keys from disk.

=== modified file 'lib/core/crypto.h'
--- lib/core/crypto.h	2011-10-30 11:54:44 +0000
+++ lib/core/crypto.h	2011-11-09 06:56:25 +0000
@@ -26,12 +26,16 @@
 #ifndef HIP_LIB_CORE_CRYPTO_H
 #define HIP_LIB_CORE_CRYPTO_H
 
+#include "config.h"
+
 #include <stdint.h>
 #include <netinet/in.h>
 #include <sys/types.h>
 #include <openssl/dsa.h>
 #include <openssl/rsa.h>
+#ifdef HAVE_EC_CRYPTO
 #include <openssl/ec.h>
+#endif /* HAVE_EC_CRYPTO */
 #include <openssl/dh.h>
 #include <openssl/pem.h>
 
@@ -92,28 +96,31 @@
 uint16_t hip_get_dh_size(uint8_t hip_dh_group_type);
 DSA *create_dsa_key(const int bits);
 RSA *create_rsa_key(const int bits);
-EC_KEY *create_ecdsa_key(const int nid);
 int save_dsa_private_key(const char *const filenamebase, DSA *const dsa);
 int save_rsa_private_key(const char *const filenamebase, RSA *const rsa);
-int save_ecdsa_private_key(const char *const filenamebase, EC_KEY *const ecdsa);
 int load_dsa_private_key(const char *const filenamebase, DSA **const dsa);
 int load_rsa_private_key(const char *const filename, RSA **const rsa);
-int load_ecdsa_private_key(const char *const filename, EC_KEY **const ec);
 int impl_dsa_sign(const unsigned char *const digest,
                   DSA *const dsa,
                   unsigned char *const signature);
 int impl_dsa_verify(const unsigned char *const digest,
                     DSA *const dsa,
                     const unsigned char *const signature);
+int hip_write_hmac(int type, const void *key, void *in, int in_len, void *out);
+int hip_crypto_encrypted(void *data, const void *iv, int enc_alg, int enc_len,
+                         uint8_t *enc_key, int direction);
+void get_random_bytes(void *buf, int n);
+
+#ifdef HAVE_EC_CRYPTO
+EC_KEY *create_ecdsa_key(const int nid);
+int save_ecdsa_private_key(const char *const filenamebase, EC_KEY *const ecdsa);
+int load_ecdsa_private_key(const char *const filename, EC_KEY **const ec);
 int impl_ecdsa_sign(const unsigned char *const digest,
                     EC_KEY *const ecdsa,
                     unsigned char *const signature);
 int impl_ecdsa_verify(const unsigned char *const digest,
                       EC_KEY *const ecdsa,
                       const unsigned char *const signature);
-int hip_write_hmac(int type, const void *key, void *in, int in_len, void *out);
-int hip_crypto_encrypted(void *data, const void *iv, int enc_alg, int enc_len,
-                         uint8_t *enc_key, int direction);
-void get_random_bytes(void *buf, int n);
+#endif /* HAVE_EC_CRYPTO */
 
 #endif /* HIP_LIB_CORE_CRYPTO_H */

=== modified file 'lib/core/hostid.c'
--- lib/core/hostid.c	2011-10-25 21:14:16 +0000
+++ lib/core/hostid.c	2011-11-09 06:56:25 +0000
@@ -250,6 +250,7 @@
     return err;
 }
 
+#ifdef HAVE_EC_CRYPTO
 /**
  * Convert ECDSA-based private host id to a HIT.
  *
@@ -290,6 +291,7 @@
 
     return 0;
 }
+#endif /* HAVE_EC_CRYPTO */
 
 /**
  * Convert RSA, DSA, or ECDSA-based private host id to a HIT
@@ -310,13 +312,16 @@
         return private_dsa_host_id_to_hit(host_id, hit, hit_type);
     case HIP_HI_RSA:
         return private_rsa_host_id_to_hit(host_id, hit, hit_type);
+#ifdef HAVE_EC_CRYPTO
     case HIP_HI_ECDSA:
         return private_ecdsa_host_id_to_hit(host_id, hit, hit_type);
+#endif /* HAVE_EC_CRYPTO */
     default:
         return -ENOSYS;
     }
 }
 
+#ifdef HAVE_EC_CRYPTO
 /*
  * Translate the openssl specific curve id into the coressponding HIP id.
  *
@@ -422,6 +427,7 @@
 
     return 0;
 }
+#endif /* HAVE_EC_CRYPTO */
 
 /**
  * dig out RSA key length from an host id
@@ -553,6 +559,7 @@
     return dsa;
 }
 
+#ifdef HAVE_EC_CRYPTO
 /**
  * convert a ECDSA-based host id into an OpenSSL structure
  *
@@ -631,6 +638,7 @@
 
     return ret;
 }
+#endif /* HAVE_EC_CRYPTO */
 
 /**
  * (Re)create new host identities or load existing ones, and append the
@@ -664,30 +672,44 @@
                                  const int dsa_key_bits,
                                  const int ecdsa_nid)
 {
-    int                      err                = 0, dsa_key_rr_len = 0, rsa_key_rr_len = 0, ecdsa_key_rr_len = 0;
-    int                      dsa_pub_key_rr_len = 0, rsa_pub_key_rr_len = 0, ecdsa_pub_key_rr_len = 0;
+    int                      err                = 0, dsa_key_rr_len = 0, rsa_key_rr_len = 0;
+    int                      dsa_pub_key_rr_len = 0, rsa_pub_key_rr_len = 0;
     hip_hdr                  numeric_action     = 0;
     char                     hostname[HIP_HOST_ID_HOSTNAME_LEN_MAX];
-    const char              *rsa_filenamebase       = DEFAULT_HOST_RSA_KEY_FILE_BASE DEFAULT_ANON_HI_FILE_NAME_SUFFIX;
-    const char              *dsa_filenamebase       = DEFAULT_HOST_DSA_KEY_FILE_BASE DEFAULT_ANON_HI_FILE_NAME_SUFFIX;
-    const char              *ecdsa_filenamebase     = DEFAULT_HOST_ECDSA_KEY_FILE_BASE DEFAULT_ANON_HI_FILE_NAME_SUFFIX;
-    const char              *rsa_filenamebase_pub   = DEFAULT_HOST_RSA_KEY_FILE_BASE DEFAULT_PUB_HI_FILE_NAME_SUFFIX;
-    const char              *dsa_filenamebase_pub   = DEFAULT_HOST_DSA_KEY_FILE_BASE DEFAULT_PUB_HI_FILE_NAME_SUFFIX;
-    const char              *ecdsa_filenamebase_pub = DEFAULT_HOST_ECDSA_KEY_FILE_BASE DEFAULT_PUB_HI_FILE_NAME_SUFFIX;
-    unsigned char           *dsa_key_rr             = NULL, *rsa_key_rr = NULL, *ecdsa_key_rr = NULL;
-    unsigned char           *dsa_pub_key_rr         = NULL, *rsa_pub_key_rr = NULL, *ecdsa_pub_key_rr = NULL;
-    DSA                     *dsa_key                = NULL, *dsa_pub_key = NULL;
-    RSA                     *rsa_key                = NULL, *rsa_pub_key = NULL;
-    EC_KEY                  *ecdsa_key              = NULL, *ecdsa_pub_key = NULL;
-    struct hip_host_id_local rsa_lhi, dsa_lhi, ecdsa_lhi, rsa_pub_lhi, dsa_pub_lhi, ecdsa_pub_lhi;
-    struct hip_host_id      *dsa_host_id            = NULL, *rsa_host_id = NULL, *ecdsa_host_id = NULL;
-    struct hip_host_id      *dsa_pub_host_id        = NULL, *rsa_pub_host_id = NULL, *ecdsa_pub_host_id = NULL;
+    const char              *rsa_filenamebase     = DEFAULT_HOST_RSA_KEY_FILE_BASE DEFAULT_ANON_HI_FILE_NAME_SUFFIX;
+    const char              *dsa_filenamebase     = DEFAULT_HOST_DSA_KEY_FILE_BASE DEFAULT_ANON_HI_FILE_NAME_SUFFIX;
+    const char              *rsa_filenamebase_pub = DEFAULT_HOST_RSA_KEY_FILE_BASE DEFAULT_PUB_HI_FILE_NAME_SUFFIX;
+    const char              *dsa_filenamebase_pub = DEFAULT_HOST_DSA_KEY_FILE_BASE DEFAULT_PUB_HI_FILE_NAME_SUFFIX;
+    unsigned char           *dsa_key_rr           = NULL, *rsa_key_rr = NULL;
+    unsigned char           *dsa_pub_key_rr       = NULL, *rsa_pub_key_rr = NULL;
+    DSA                     *dsa_key              = NULL, *dsa_pub_key = NULL;
+    RSA                     *rsa_key              = NULL, *rsa_pub_key = NULL;
+    struct hip_host_id_local rsa_lhi, dsa_lhi, rsa_pub_lhi, dsa_pub_lhi;
+    struct hip_host_id      *dsa_host_id            = NULL, *rsa_host_id = NULL;
+    struct hip_host_id      *dsa_pub_host_id        = NULL, *rsa_pub_host_id = NULL;
     struct endpoint_hip     *endpoint_dsa_hip       = NULL;
     struct endpoint_hip     *endpoint_dsa_pub_hip   = NULL;
     struct endpoint_hip     *endpoint_rsa_hip       = NULL;
     struct endpoint_hip     *endpoint_rsa_pub_hip   = NULL;
     struct endpoint_hip     *endpoint_ecdsa_hip     = NULL;
     struct endpoint_hip     *endpoint_ecdsa_pub_hip = NULL;
+#ifdef HAVE_EC_CRYPTO
+    int                      ecdsa_key_rr_len       = 0, ecdsa_pub_key_rr_len = 0;
+    const char              *ecdsa_filenamebase     = DEFAULT_HOST_ECDSA_KEY_FILE_BASE DEFAULT_ANON_HI_FILE_NAME_SUFFIX;
+    const char              *ecdsa_filenamebase_pub = DEFAULT_HOST_ECDSA_KEY_FILE_BASE DEFAULT_PUB_HI_FILE_NAME_SUFFIX;
+    unsigned char           *ecdsa_key_rr           = NULL;
+    unsigned char           *ecdsa_pub_key_rr       = NULL;
+    EC_KEY                  *ecdsa_key              = NULL, *ecdsa_pub_key = NULL;
+    struct hip_host_id_local ecdsa_lhi, ecdsa_pub_lhi;
+    struct hip_host_id      *ecdsa_host_id     = NULL;
+    struct hip_host_id      *ecdsa_pub_host_id = NULL;
+#endif /* HAVE_EC_CRYPTO */
+
+    if (ecdsa_nid < 0) {
+        err = -1;
+        HIP_ERROR("NID for ECDSA is strange %d\n", ecdsa_nid);
+        goto out_err;
+    }
 
     if (action == ACTION_ADD) {
         numeric_action = HIP_MSG_ADD_LOCAL_HI;
@@ -726,6 +748,7 @@
                     HIP_ERROR("Saving of DSA key failed.\n");
                     goto out_err;
                 }
+#ifdef HAVE_EC_CRYPTO
             } else if (!strcmp(hi_fmt, "ecdsa")) {
                 ecdsa_key = create_ecdsa_key(ecdsa_nid);
                 HIP_IFEL(!ecdsa_key, -EINVAL,
@@ -734,6 +757,7 @@
                     HIP_ERROR("Saving of ECDSA key failed.\n");
                     goto out_err;
                 }
+#endif /* HAVE_EC_CRYPTO */
             } else {             /*RSA*/
                 rsa_key = create_rsa_key(rsa_key_bits);
                 HIP_IFEL(!rsa_key, -EINVAL,
@@ -764,6 +788,7 @@
         HIP_IFEL(!rsa_pub_key, -EINVAL,
                  "Creation of public RSA key failed.\n");
 
+#ifdef HAVE_EC_CRYPTO
         ecdsa_key = create_ecdsa_key(ecdsa_nid);
         HIP_IFEL(!ecdsa_key, -EINVAL,
                  "Creation of ECDSA key failed.\n");
@@ -772,6 +797,17 @@
         HIP_IFEL(!ecdsa_pub_key, -EINVAL,
                  "Creation of public ECDSA key failed.\n");
 
+        if ((err = save_ecdsa_private_key(ecdsa_filenamebase, ecdsa_key))) {
+            HIP_ERROR("Saving of ECDSA key failed.\n");
+            goto out_err;
+        }
+
+        if ((err = save_ecdsa_private_key(ecdsa_filenamebase_pub, ecdsa_pub_key))) {
+            HIP_ERROR("Saving of public ECDSA key failed.\n");
+            goto out_err;
+        }
+#endif /* HAVE_EC_CRYPTO */
+
         if ((err = save_dsa_private_key(dsa_filenamebase, dsa_key))) {
             HIP_ERROR("Saving of DSA key failed.\n");
             goto out_err;
@@ -792,16 +828,6 @@
             goto out_err;
         }
 
-        if ((err = save_ecdsa_private_key(ecdsa_filenamebase, ecdsa_key))) {
-            HIP_ERROR("Saving of ECDSA key failed.\n");
-            goto out_err;
-        }
-
-        if ((err = save_ecdsa_private_key(ecdsa_filenamebase_pub, ecdsa_pub_key))) {
-            HIP_ERROR("Saving of public ECDSA key failed.\n");
-            goto out_err;
-        }
-
         break;
 
     case ACTION_ADD:
@@ -823,6 +849,7 @@
                     HIP_ERROR("Building of host id failed\n");
                     goto out_err;
                 }
+#ifdef HAVE_EC_CRYPTO
             } else if (!strcmp(hi_fmt, "ecdsa")) {
                 if ((err = load_ecdsa_private_key(ecdsa_filenamebase, &ecdsa_key))) {
                     HIP_ERROR("Loading of the ECDSA key failed\n");
@@ -839,6 +866,7 @@
                     HIP_ERROR("Building of host id failed\n");
                     goto out_err;
                 }
+#endif /* HAVE_EC_CRYPTO */
             } else { /*RSA*/
                 if ((err = load_rsa_private_key(hi_file, &rsa_key))) {
                     HIP_ERROR("Failed to load RSA key from file %s\n", hi_file);
@@ -910,6 +938,7 @@
                     goto out_err;
                 }
             }
+#ifdef HAVE_EC_CRYPTO
         } else if (!strcmp(hi_fmt, "ecdsa")) {
             if (anon) {
                 if ((err = load_ecdsa_private_key(ecdsa_filenamebase, &ecdsa_key))) {
@@ -958,6 +987,7 @@
                     goto out_err;
                 }
             }
+#endif /* HAVE_EC_CRYPTO */
         } else if (anon) { /* rsa anon */
             if ((err = load_rsa_private_key(rsa_filenamebase, &rsa_key))) {
                 HIP_ERROR("Loading of the RSA key failed\n");
@@ -1059,41 +1089,47 @@
     if (rsa_filenamebase_pub != NULL) {
         change_key_file_perms(rsa_filenamebase_pub);
     }
-    if (ecdsa_filenamebase_pub != NULL) {
-        change_key_file_perms(ecdsa_filenamebase_pub);
-    }
-    if (ecdsa_filenamebase_pub != NULL) {
-        change_key_file_perms(ecdsa_filenamebase_pub);
-    }
 
     free(dsa_host_id);
     free(dsa_pub_host_id);
-    free(ecdsa_host_id);
-    free(ecdsa_pub_host_id);
     free(rsa_host_id);
     free(rsa_pub_host_id);
     DSA_free(dsa_key);
-    EC_KEY_free(ecdsa_key);
     RSA_free(rsa_key);
     DSA_free(dsa_pub_key);
-    EC_KEY_free(ecdsa_pub_key);
     RSA_free(rsa_pub_key);
     free(dsa_key_rr);
-    free(ecdsa_key_rr);
     free(rsa_key_rr);
     free(dsa_pub_key_rr);
-    free(ecdsa_pub_key_rr);
     free(rsa_pub_key_rr);
     free(endpoint_dsa_hip);
-    free(endpoint_ecdsa_hip);
     free(endpoint_rsa_hip);
     free(endpoint_dsa_pub_hip);
+    free(endpoint_rsa_pub_hip);
+
+#ifdef HAVE_EC_CRYPTO
+    /* We make exeception to the common memory deallocation policy (LIFO)
+     * here to group of all ECDSA deallocations between a single ifdef */
+    if (ecdsa_filenamebase_pub != NULL) {
+        change_key_file_perms(ecdsa_filenamebase_pub);
+    }
+    if (ecdsa_filenamebase_pub != NULL) {
+        change_key_file_perms(ecdsa_filenamebase_pub);
+    }
+    free(ecdsa_host_id);
+    free(ecdsa_pub_host_id);
+    EC_KEY_free(ecdsa_key);
+    EC_KEY_free(ecdsa_pub_key);
+    free(ecdsa_key_rr);
+    free(ecdsa_pub_key_rr);
+    free(endpoint_ecdsa_hip);
     free(endpoint_ecdsa_pub_hip);
-    free(endpoint_rsa_pub_hip);
+#endif /* HAVE_EC_CRYPTO */
 
     return err;
 }
 
+#ifdef HAVE_EC_CRYPTO
 /**
  * Serialize an ECDSA public key.
  *
@@ -1166,6 +1202,7 @@
     free(buffer);
     return err;
 }
+#endif /* HAVE_EC_CRYPTO */
 
 /**
  * create DNS KEY RR record from host DSA key

=== modified file 'lib/core/hostid.h'
--- lib/core/hostid.h	2011-07-18 13:10:26 +0000
+++ lib/core/hostid.h	2011-11-09 06:56:25 +0000
@@ -26,10 +26,14 @@
 #ifndef HIP_LIB_CORE_HOSTID_H
 #define HIP_LIB_CORE_HOSTID_H
 
+#include "config.h"
+
 #include <netinet/in.h>
 #include <openssl/dsa.h>
 #include <openssl/rsa.h>
+#ifdef HAVE_EC_CRYPTO
 #include <openssl/ec.h>
+#endif /* HAVE_EC_CRYPTO */
 
 #include "protodefs.h"
 #include "state.h"
@@ -62,11 +66,14 @@
                          struct hip_ecdsa_keylen *const ret);
 RSA *hip_key_rr_to_rsa(const struct hip_host_id_priv *const host_id, const int is_priv);
 DSA *hip_key_rr_to_dsa(const struct hip_host_id_priv *const host_id, const int is_priv);
-EC_KEY *hip_key_rr_to_ecdsa(const struct hip_host_id_priv *const host_id, const int is_priv);
 
 int dsa_to_dns_key_rr(const DSA *const dsa, unsigned char **const buf);
 int rsa_to_dns_key_rr(const RSA *const rsa, unsigned char **const rsa_key_rr);
+
+#ifdef HAVE_EC_CRYPTO
+EC_KEY *hip_key_rr_to_ecdsa(const struct hip_host_id_priv *const host_id, const int is_priv);
 int ecdsa_to_key_rr(const EC_KEY *const ecdsa, unsigned char **const ec_key_rr);
+#endif /* HAVE_EC_CRYPTO */
 
 int hip_serialize_host_id_action(struct hip_common *msg,
                                  const int action,

=== modified file 'lib/tool/pk.c'
--- lib/tool/pk.c	2011-08-15 14:11:56 +0000
+++ lib/tool/pk.c	2011-11-09 06:56:25 +0000
@@ -8,6 +8,8 @@
  * @brief HIPL wrappers for OpenSSL public key operations.
  */
 
+#include "config.h"
+
 #include <errno.h>
 #include <stdint.h>
 #include <stdlib.h>
@@ -15,7 +17,9 @@
 #include <netinet/in.h>
 #include <openssl/bn.h>
 #include <openssl/dsa.h>
+#ifdef HAVE_EC_CRYPTO
 #include <openssl/ecdsa.h>
+#endif /* HAVE_EC_CRYPTO */
 #include <openssl/objects.h>
 #include <openssl/rsa.h>
 
@@ -26,7 +30,6 @@
 #include "lib/core/performance.h"
 #include "lib/core/prefix.h"
 #include "lib/core/protodefs.h"
-#include "config.h"
 #include "pk.h"
 
 /**
@@ -75,6 +78,7 @@
     return err;
 }
 
+#ifdef HAVE_EC_CRYPTO
 /**
  * Sign a HIP control message with a private ECDSA key.
  *
@@ -128,6 +132,7 @@
 
     return 0;
 }
+#endif /* HAVE_EC_CRYPTO */
 
 /**
  * sign a HIP control message with a private DSA key
@@ -225,8 +230,10 @@
         /* RSA_verify returns 0 on failure */
         err = !RSA_verify(NID_sha1, sha1_digest, SHA_DIGEST_LENGTH,
                           sig->signature, RSA_size(peer_pub), peer_pub);
+#ifdef HAVE_EC_CRYPTO
     } else if (type == HIP_HI_ECDSA) {
         err = impl_ecdsa_verify(sha1_digest, peer_pub, sig->signature);
+#endif /* HAVE_EC_CRYPTO */
     } else {
         err = impl_dsa_verify(sha1_digest, peer_pub, sig->signature);
     }
@@ -259,6 +266,7 @@
     return err;
 }
 
+#ifdef HAVE_EC_CRYPTO
 /**
  * Verify the ECDSA signature from a message.
  *
@@ -271,6 +279,7 @@
 {
     return verify(peer_pub, msg, HIP_HI_ECDSA);
 }
+#endif /* HAVE_EC_CRYPTO */
 
 /**
  * RSA signature verification function

=== modified file 'test/lib/tool/pk.c'
--- test/lib/tool/pk.c	2011-07-18 13:10:10 +0000
+++ test/lib/tool/pk.c	2011-11-09 06:56:25 +0000
@@ -23,17 +23,20 @@
  * OTHER DEALINGS IN THE SOFTWARE.
  */
 
+#include "config.h"
+
 #include <check.h>
 #include <stdlib.h>
 #include <string.h>
 #include <stdio.h>
+#ifdef HAVE_EC_CRYPTO
 #include <openssl/ec.h>
+#endif /* HAVE_EC_CRYPTO */
 #include <openssl/pem.h>
 
 #include "lib/core/debug.h"
 #include "lib/core/crypto.h"
 #include "lib/tool/pk.h"
-#include "config.h"
 #include "test_suites.h"
 
 START_TEST(test_ecdsa_sign_verify)