Handling of OpenSSL versions > 1.0.x?

[Christian Storm]

Hi,

include/sslapi.h:59 calls CRYPTO_malloc_init() which is only present in
OpenSSL versions 1.0.x. However, there's no message indicating this when
using, e.g., OpenSSL 1.1.0, except for an implicit function declaration
error and some incomplete type complaints.

For the time being until OpenSSL versions > 1.0.x are supported or the
make system complains about this issue, the following patch gives at
least a hint:

```
diff --git a/include/sslapi.h b/include/sslapi.h
index 323c844..915723a 100644
--- a/include/sslapi.h
+++ b/include/sslapi.h
@@ -36,6 +36,11 @@
#include <openssl/hmac.h>
#include <openssl/aes.h>
#include <openssl/cms.h>
+#include <openssl/opensslv.h>
+
+#if OPENSSL_VERSION_NUMBER >= 0x10100000L
+#error OpenSSL versions > 1.0.2 are not supported
+#endif

struct swupdate_digest {
EVP_PKEY *pkey; /* this is used for RSA key */
```

What do you think?


Kind regards,
Christian

[Stefano Babic]

Hi Christian,

On 05/05/2017 17:43, Christian Storm wrote:
> Hi,
>
> include/sslapi.h:59 calls CRYPTO_malloc_init() which is only present in
> OpenSSL versions 1.0.x. However, there's no message indicating this when
> using, e.g., OpenSSL 1.1.0, except for an implicit function declaration
> error and some incomplete type complaints.
>

As far as I can see, CRYPTO_malloc_init() was renamed to
OPENSSL_malloc_init() and it is quite a dumb function for Linux Systems.

> For the time being until OpenSSL versions > 1.0.x are supported or the
> make system complains about this issue, the following patch gives at
> least a hint:

This is quite unsatisfactory. If someone is already testing with 1.1.x
(next Yocto release wil still have 1.0.2), he should provide a fix for it.

>
> ```
> diff --git a/include/sslapi.h b/include/sslapi.h
> index 323c844..915723a 100644
> --- a/include/sslapi.h
> +++ b/include/sslapi.h
> @@ -36,6 +36,11 @@
> #include <openssl/hmac.h>
> #include <openssl/aes.h>
> #include <openssl/cms.h>
> +#include <openssl/opensslv.h>
> +
> +#if OPENSSL_VERSION_NUMBER >= 0x10100000L
> +#error OpenSSL versions > 1.0.2 are not supported
> +#endif
>
> struct swupdate_digest {
> EVP_PKEY *pkey; /* this is used for RSA key */
> ```
>
> What do you think?
>

If we do it, we should do the right thing: that means supporting OpenSSL
1.1.x.

Best regards,
Stefano

--
=====================================================================
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sba...@denx.de
=====================================================================

[Christian Storm]

Hi Stefano,

> > include/sslapi.h:59 calls CRYPTO_malloc_init() which is only present in
> > OpenSSL versions 1.0.x. However, there's no message indicating this when
> > using, e.g., OpenSSL 1.1.0, except for an implicit function declaration
> > error and some incomplete type complaints.
> >
>
> As far as I can see, CRYPTO_malloc_init() was renamed to
> OPENSSL_malloc_init() and it is quite a dumb function for Linux Systems.
>
> > For the time being until OpenSSL versions > 1.0.x are supported or the
> > make system complains about this issue, the following patch gives at
> > least a hint:
>
> This is quite unsatisfactory. If someone is already testing with 1.1.x
> (next Yocto release wil still have 1.0.2), he should provide a fix for it.

> [...]

>
> If we do it, we should do the right thing: that means supporting OpenSSL
> 1.1.x.

OK, I went ahead and made a patch for it which does compile and works
until corelib/swupdate_decrypt.c:swupdate_DECRYPT_final(), more
precisely EVP_DecryptFinal_ex(). This function always returns 0
(=FAILURE) but I have no idea why and weren't able to gather more
sensible information. I played with different crypted inputs and padding
and stuff but came to no solution.

The OpenSSL versions I used are 1.1.0.e and 1.0.2.k.
It's the same for both of them which is a somewhat good sign as the
patch seems to be working then :)

What I did was:

$ openssl enc -aes-256-cbc -k secret -P -md sha1 -nosalt
key=E5E9FA1BA31ECD1AE84F75CAAA474F3A663F05F412028F81DA65D26EE56424B2
iv =E93DA465B309C53FEC5FF93C9637DA58

which I then transferred into SWUpdate's wanted format
E5E9FA1BA31ECD1AE84F75CAAA474F3A663F05F412028F81DA65D26EE56424B2 E93DA465B309C53FEC5FF93C9637DA58

and provided this to SWUpdate via the -K switch. The segfault I got when
I forgot this switch is fixed in another patch I sent to the ML.

Then I ran SWUpdate using this sw-description file:

software =
{
version = "0.0.1";
images: ( {
filename = "dummy.root.crypt";
device = "/dev/mmcblk0p3";
compressed = false;
sha256 = "d5917ed1d56f6714831de7fc7bd92c78639186e3f3244152923535db9fa18144";
encrypted = true;
}
);
}


Do you have any hints for me how to track this down? Maybe I just missed
a simple thing?


As a side note: I noted that documentation for the encryption feature is missing.
Once I get this working I'll extend the documentation for this feature.


Kind regards,
Christian

[Stefano Babic]

Hi Christian,

It is not far away from my test, but I have not used -nosalt:

openssl enc -aes-256-cbc -k secret -P -md sha1

In openssl manual I see:

" -nosalt
don't use a salt in the key derivation routines. This option SHOULD
NOT be used except for test purposes or compatibility with ancient
versions of OpenSSL and SSLeay. "

> which I then transferred into SWUpdate's wanted format
> E5E9FA1BA31ECD1AE84F75CAAA474F3A663F05F412028F81DA65D26EE56424B2 E93DA465B309C53FEC5FF93C9637DA58
>

I did the same.

> and provided this to SWUpdate via the -K switch. The segfault I got when
> I forgot this switch is fixed in another patch I sent to the ML.
>
> Then I ran SWUpdate using this sw-description file:
>
> software =
> {
> version = "0.0.1";
> images: ( {
> filename = "dummy.root.crypt";
> device = "/dev/mmcblk0p3";
> compressed = false;
> sha256 = "d5917ed1d56f6714831de7fc7bd92c78639186e3f3244152923535db9fa18144";
> encrypted = true;
> }
> );
> }
>
>
> Do you have any hints for me how to track this down? Maybe I just missed
> a simple thing?
>

I checked my use cases - the only thing I have seen (apart the -nosalt
option) is that in my test cases I did not had signed images. Of course,
it *must* work, but it is worth to test to check if the issue is correlated.

>
> As a side note: I noted that documentation for the encryption feature is missing.

You're right.

> Once I get this working I'll extend the documentation for this feature.

Thanks !

[Christian Storm]

Yes, that's why I used this to not get the
salt=103367AA3B018821
output line from $ openssl enc -aes-256-cbc -k secret -P -md sha1

However, I tried both variants now, one with and one without -nosalt.
The outcome is the same in both cases.

> > which I then transferred into SWUpdate's wanted format
> > E5E9FA1BA31ECD1AE84F75CAAA474F3A663F05F412028F81DA65D26EE56424B2 E93DA465B309C53FEC5FF93C9637DA58
> >
>
> I did the same.
>
> > and provided this to SWUpdate via the -K switch. The segfault I got when
> > I forgot this switch is fixed in another patch I sent to the ML.
> >
> > Then I ran SWUpdate using this sw-description file:
> >
> > software =
> > {
> > version = "0.0.1";
> > images: ( {
> > filename = "dummy.root.crypt";
> > device = "/dev/mmcblk0p3";
> > compressed = false;
> > sha256 = "d5917ed1d56f6714831de7fc7bd92c78639186e3f3244152923535db9fa18144";
> > encrypted = true;
> > }
> > );
> > }
> >
> >
> > Do you have any hints for me how to track this down? Maybe I just missed
> > a simple thing?
> >
>
> I checked my use cases - the only thing I have seen (apart the -nosalt
> option) is that in my test cases I did not had signed images. Of course,
> it *must* work, but it is worth to test to check if the issue is correlated.

Checked with and without SIGNED_IMAGES, outcome is the same.


Do you have any more ideas I can try out to nail this down?
What exact version of OpenSSL are you using in your use case? Maybe that
makes a difference in logics while not in API...


Kind regards,
Christian

[Stefano Babic]

I have no other idea - I will set up a test and try myself.

Regards,

[Stefano Babic]

Hi Christian,

On 09/05/2017 14:56, Stefano Babic wrote:

>>>> Then I ran SWUpdate using this sw-description file:
>>>>
>>>> software =
>>>> {
>>>> version = "0.0.1";
>>>> images: ( {
>>>> filename = "dummy.root.crypt";
>>>> device = "/dev/mmcblk0p3";
>>>> compressed = false;
>>>> sha256 = "d5917ed1d56f6714831de7fc7bd92c78639186e3f3244152923535db9fa18144";
>>>> encrypted = true;
>>>> }
>>>> );
>>>> }
>>>>
>>>>
>>>> Do you have any hints for me how to track this down? Maybe I just missed
>>>> a simple thing?
>>>>
>>>
>>> I checked my use cases - the only thing I have seen (apart the -nosalt
>>> option) is that in my test cases I did not had signed images. Of course,
>>> it *must* work, but it is worth to test to check if the issue is correlated.
>>
>> Checked with and without SIGNED_IMAGES, outcome is the same.
>>
>>
>> Do you have any more ideas I can try out to nail this down?
>> What exact version of OpenSSL are you using in your use case? Maybe that
>> makes a difference in logics while not in API...
>
> I have no other idea - I will set up a test and try myself.

I ran a simple test on PC. I plan to test on a real device, too. Anyway,
my use case works. I have this sw-description:

software =
{
version = "0.1.0";

pc = {
hardware-compatibility: [ "1.0"];
files: (
{
filename = "examples.tar.enc";
type = "archive";
path = "/tmp/test";
encrypted = true;
}
);
scripts: (
{
filename = "shellscript_logger.sh";
type = "shellscript";
}
);

};
}

And I have encrypted the original examples.tar file with the following
script:

#!/bin/bash

key=`cat ../etc/aeskey | cut -d' ' -f1`
ivt=`cat ../etc/aeskey | cut -d' ' -f2`

echo KEY=${key} iv=${ivt}
openssl enc -aes-256-cbc -in $1 -out $1.enc -K ${key} -iv ${ivt}

openssl is 1.0.2g

and:

./swupdate -v -H pc:1.0 -l 5 -K tests/etc/aeskey -i
tests/auto/boards/pc/test5/software_pc_test5.swu
Swupdate v2017.4.0

Licensed under GPLv2. See source distribution for detailed copyright
notices.

Searching for custom LUA handlers :[string "require
("swupdate_handlers")"]:1: module 'swupdate_handlers' not found:
no field package.preload['swupdate_handlers']
no file '/usr/local/share/lua/5.2/swupdate_handlers.lua'
no file '/usr/local/share/lua/5.2/swupdate_handlers/init.lua'
no file '/usr/local/lib/lua/5.2/swupdate_handlers.lua'
no file '/usr/local/lib/lua/5.2/swupdate_handlers/init.lua'
no file '/usr/share/lua/5.2/swupdate_handlers.lua'
no file '/usr/share/lua/5.2/swupdate_handlers/init.lua'
no file './swupdate_handlers.lua'
no file '/usr/local/lib/lua/5.2/swupdate_handlers.so'
no file '/usr/lib/x86_64-linux-gnu/lua/5.2/swupdate_handlers.so'
no file '/usr/lib/lua/5.2/swupdate_handlers.so'
no file '/usr/local/lib/lua/5.2/loadall.so'
no file './swupdate_handlers.so'
Custom handlers not found, no error, skipping...

Running on pc Revision 1.0
Registered handlers:
archive
tar
raw
rawfile
lua
shellscript
preinstall
postinstall
remote
uboot
bootloader
Main loop Daemon
[NOTIFY] : SWUPDATE running : [extract_sw_description] : Found file:
filename sw-description
size 308
checksum 0x538d VERIFIED

Version 0.1.0
[NOTIFY] : SWUPDATE running : [parse_hw_compatibility] : Accepted Hw
Revision : 1.0
[NOTIFY] : SWUPDATE running : [parse_scripts] : Found Script:
shellscript_logger.sh

[NOTIFY] : SWUPDATE running : [parse_files] : Found File :
examples.tar.enc --> /tmp/test (ROOTFS)

[NOTIFY] : SWUPDATE running : [check_hw_compatibility] : Hardware pc
Revision: 1.0
[NOTIFY] : SWUPDATE running : [check_hw_compatibility] : Hardware
compatibility verified
[NOTIFY] : SWUPDATE running : [cpio_scan] : Found file:
filename examples.tar.enc
size 1689616
REQUIRED

[NOTIFY] : SWUPDATE running : [cpio_scan] : Found file:
filename shellscript_logger.sh
size 261
REQUIRED

Cannot parse config file '/etc/fw_env.config': No such file or directory
Error: environment not initialized
[NOTIFY] : SWUPDATE running : [extract_next_file] : Copied file:
filename shellscript_logger.sh
size 261
checksum 0x5476 VERIFIED

[NOTIFY] : SWUPDATE running : [execute_shell_script] : Calling shell
script /tmp/shellscript_logger.sh preinst: return with 0
[NOTIFY] : SWUPDATE running : [install_single_image] : Found installer
for stream examples.tar.enc archive
[NOTIFY] : SWUPDATE running : [install_archive_image] : Installing file
examples.tar.enc on /tmp/test

[NOTIFY] : SWUPDATE running : [execute_shell_script] : Calling shell
script /tmp/shellscript_logger.sh postinst: return with 0
Cannot parse config file '/etc/fw_env.config': No such file or directory
Error: environment not initialized
Software updated successfully
Please reboot the device to start the new software
[NOTIFY] : SWUPDATE successful !


so everything is ok - and if I change the key I passed to SWUpdate, I
get of course an error by decryption:

[NOTIFY] : SWUPDATE running : [execute_shell_script] : Calling shell
script /tmp/shellscript_logger.sh preinst: return with 0
[NOTIFY] : SWUPDATE running : [install_single_image] : Found installer
for stream examples.tar.enc archive
[NOTIFY] : SWUPDATE running : [install_archive_image] : Installing file
examples.tar.enc on /tmp/test

[NOTIFY] : SWUPDATE failed [0] ERROR corelib/swupdate_decrypt.c :
swupdate_DECRYPT_final : 86 : Decryption error 0x(null)

[NOTIFY] : SWUPDATE failed [0] ERROR handlers/archive_handler.c :
install_archive_image : 213 : Error copying extracted file

[NOTIFY] : SWUPDATE running : [install_single_image] : Installer for
archive not successful !
Software updated failed

So I cannot reproduce it. I see you add explicitely "compressed = false"
in sw-description. I do not need to add it if fileis not compressed
(default is false). Does it change if you drop the line ?

[Stefano Babic]

As a further idea: do you have both CONFIG_HASH_VERIFY and
CONFIG_ENCRYPTED_IMAGES in your .config ?

[Christian Storm]

Hi Stefano,

Ah, what's your actual output of

$ openssl enc -aes-256-cbc -k secret -P -md sha1

? I get, for example,
salt=0F01BB8E4D6E3264
key=307CFE2826588116CFCFFAAC11A087B93E2266129A800D2F1C20B84C8222A1FA
iv =910B57C2EACCFAF524B98C87285B341B
so that I massaged the aes-keyfile for SWUpdate manually.
This is using openssl 1.0.2.k as well as 1.1.0.e.
Judging from your shell snippet it seems that your opennssl command
gives a different output. Did you generate this with a pre-1.0 openssl?

> and:
>
> ./swupdate -v -H pc:1.0 -l 5 -K tests/etc/aeskey -i
> tests/auto/boards/pc/test5/software_pc_test5.swu
> Swupdate v2017.4.0
>

> [...]

>
> So I cannot reproduce it. I see you add explicitely "compressed = false"
> in sw-description. I do not need to add it if fileis not compressed
> (default is false). Does it change if you drop the line ?

No, it didn't make a change but with your example I was able to
successfully run it! Thanks for the support!

One thing I noticed was that -nosalt is actually required.



Kind regards,
Christian

[Christian Storm]

Hi Stefano,

Yes, I do and this variant also worked with your help as written in my
previous mail.

Thanks again for suggestions!


Kind regards,
Christian

[Stefano Babic]

On 10/05/2017 10:09, Christian Storm wrote:

>> echo KEY=${key} iv=${ivt}
>> openssl enc -aes-256-cbc -in $1 -out $1.enc -K ${key} -iv ${ivt}
>>
>> openssl is 1.0.2g
>
> Ah, what's your actual output of
> $ openssl enc -aes-256-cbc -k secret -P -md sha1
> ? I get, for example,
> salt=0F01BB8E4D6E3264
> key=307CFE2826588116CFCFFAAC11A087B93E2266129A800D2F1C20B84C8222A1FA
> iv =910B57C2EACCFAF524B98C87285B341B

Same:

openssl enc -aes-256-cbc -k secret -P -md sha1

salt=5DB94E4109598840
key=4BB138994C3C958C2471E3EE62B0C1208E0E6E8C48A54A824259C488A9A56412
iv =50A66D34B23781B8921F588D0A6F9EFD

> so that I massaged the aes-keyfile for SWUpdate manually.

I did the same.

> This is using openssl 1.0.2.k as well as 1.1.0.e.
> Judging from your shell snippet it seems that your opennssl command
> gives a different output. Did you generate this with a pre-1.0 openssl?

No, I built on Ubuntu-16.04 (1.0.2g) and on the target I have Yocto
versions.

>
>> and:
>>
>> ./swupdate -v -H pc:1.0 -l 5 -K tests/etc/aeskey -i
>> tests/auto/boards/pc/test5/software_pc_test5.swu
>> Swupdate v2017.4.0
>>
>> [...]
>>
>> So I cannot reproduce it. I see you add explicitely "compressed = false"
>> in sw-description. I do not need to add it if fileis not compressed
>> (default is false). Does it change if you drop the line ?
>
> No, it didn't make a change

ok, better, else there is bug in parser..

> but with your example I was able to
> successfully run it! Thanks for the support!
>
> One thing I noticed was that -nosalt is actually required.

Ok - do you still think we have issues with OpenSSL versions ? Even if
Yocto guarantees to use the same version for -native and target, it is
bad if it is true.

Best regards,

[Christian Storm]

Hi Stefano,

> On 10/05/2017 10:09, Christian Storm wrote:
>
> >> echo KEY=${key} iv=${ivt}
> >> openssl enc -aes-256-cbc -in $1 -out $1.enc -K ${key} -iv ${ivt}
> >>
> >> openssl is 1.0.2g
> >
> > Ah, what's your actual output of
> > $ openssl enc -aes-256-cbc -k secret -P -md sha1
> > ? I get, for example,
> > salt=0F01BB8E4D6E3264
> > key=307CFE2826588116CFCFFAAC11A087B93E2266129A800D2F1C20B84C8222A1FA
> > iv =910B57C2EACCFAF524B98C87285B341B
>
> Same:
>
> openssl enc -aes-256-cbc -k secret -P -md sha1
> salt=5DB94E4109598840
> key=4BB138994C3C958C2471E3EE62B0C1208E0E6E8C48A54A824259C488A9A56412
> iv =50A66D34B23781B8921F588D0A6F9EFD
>
> > so that I massaged the aes-keyfile for SWUpdate manually.
>
> I did the same.

Ah, OK, good. Then I can write a consistent documentation which I
already started writing.

> > This is using openssl 1.0.2.k as well as 1.1.0.e.
> > Judging from your shell snippet it seems that your opennssl command
> > gives a different output. Did you generate this with a pre-1.0 openssl?
>
> No, I built on Ubuntu-16.04 (1.0.2g) and on the target I have Yocto
> versions.
>
> >
> >> and:
> >>
> >> ./swupdate -v -H pc:1.0 -l 5 -K tests/etc/aeskey -i
> >> tests/auto/boards/pc/test5/software_pc_test5.swu
> >> Swupdate v2017.4.0
> >>
> >> [...]
> >>
> >> So I cannot reproduce it. I see you add explicitely "compressed = false"
> >> in sw-description. I do not need to add it if fileis not compressed
> >> (default is false). Does it change if you drop the line ?
> >
> > No, it didn't make a change
>
> ok, better, else there is bug in parser..
>
> > but with your example I was able to
> > successfully run it! Thanks for the support!
> >
> > One thing I noticed was that -nosalt is actually required.
>
> Ok - do you still think we have issues with OpenSSL versions ? Even if
> Yocto guarantees to use the same version for -native and target, it is
> bad if it is true.

No. The only remaining issue is the required -nosalt switch for (at
least) my openssl versions 1.1.0.e and 1.0.2.k. If we want to supply
salt to SWUpdate, then the format (and parsing) of the key-aes file will
have to change as, e.g., salt has to be appended as a 3rd value and
parsed accordingly. Or we rely on openssl's output and parse that which
is more openssl-conforming but breaks compatibility with existing
deployments.


I'd suggest to move forward by
1) writing documentation for the state as is
2) implementing openssl 1.1.x compatibility
3) implement the decision on the salt topic

[Stefano Babic]

Hi Christian,

On 10/05/2017 16:56, Christian Storm wrote:

>>
>> I did the same.
>
> Ah, OK, good. Then I can write a consistent documentation which I
> already started writing.

:-)

>>
>> Ok - do you still think we have issues with OpenSSL versions ? Even if
>> Yocto guarantees to use the same version for -native and target, it is
>> bad if it is true.
>
> No. The only remaining issue is the required -nosalt switch for (at
> least) my openssl versions 1.1.0.e and 1.0.2.k. If we want to supply
> salt to SWUpdate, then the format (and parsing) of the key-aes file will
> have to change as, e.g., salt has to be appended as a 3rd value and
> parsed accordingly. Or we rely on openssl's output and parse that which
> is more openssl-conforming but breaks compatibility with existing
> deployments.
>
>
> I'd suggest to move forward by
> 1) writing documentation for the state as is
> 2) implementing openssl 1.1.x compatibility
> 3) implement the decision on the salt topic
>
> What do you think?
>

Fully agree on this small roadmap.

Regards,