diff mbox series

[V4,8/8] doc: Add documentation for asymmetric decryption

Message ID 20240115192845.51530-9-Michael.Glembotzki@iris-sensing.com
State New
Delegated to: Stefano Babic
Headers show
Series Add support for asymmetric decryption | expand

Commit Message

Michael Glembotzki Jan. 15, 2024, 7:26 p.m. UTC
Signed-off-by: Michael Glembotzki <Michael.Glembotzki@iris-sensing.com>
---
 doc/source/asym_encrypted_images.rst | 153 +++++++++++++++++++++++++++
 doc/source/encrypted_images.rst      |   2 +
 doc/source/index.rst                 |   1 +
 doc/source/roadmap.rst               |   5 -
 doc/source/sw-description.rst        |  13 ++-
 5 files changed, 167 insertions(+), 7 deletions(-)
 create mode 100644 doc/source/asym_encrypted_images.rst

Comments

Dominique MARTINET Jan. 16, 2024, 12:57 a.m. UTC | #1
Michael Glembotzki wrote on Mon, Jan 15, 2024 at 08:26:45PM +0100:
> Signed-off-by: Michael Glembotzki <Michael.Glembotzki@iris-sensing.com>

Thanks for this patch series!

I'll check the code more thoroughly as time permits but these points
below aside I didn't see any obviously big problem.


> +Example Asymmetrically Encrypted Image
> +--------------------------------------
> +
> +The image artifacts should be symmetrically encrypted and signed in advance.

Sorry if this has been discussed earlier, but if I read this (and the
code) correctly, we're decrypting the data first and then checking the
signature -- I think it'd make more sense to do it the other way around
even if it means shuffling a bit of code around (cpio's first
extract_file_to_tmp for swdesc would be no encryption and then after
signature has been checked we can decrypt it)

Best practices would be to check signature as soon as possible; we're
already trusting unchecked input with cpio header processing, cms
decrpytion code is considerably harder to validate for
robustness/security and might have vulnerabilities or leak secrets
through error timing on invalid input.

As bonus, this would allow making this feature more easily optional in
configuration (this is $jobs' problem but we're not building images
directly for our customers, so we'll have to support plain text
sw-description forever, but I'd love to upgrade and provide this as an
optional feature) -- since we've checked the signature first, we can
trust a header or whatever (alternate file name?) to decide whether it's
plain text or encrypted and proceed accordingly.


> +Create the new update image (SWU):
> +
> +::
> +
> +        #!/bin/sh
> +
> +        FILES="sw-description sw-description.sig rootfs.ext4.enc"
> +
> +        for i in $FILES; do
> +            echo $i;done | cpio -ov -H crc >  firmware.swu

(code formatting here is a bit weird)

> +Security Considerations
> +-----------------------
> +- Ideally, generate the private key on the device during factory provisioning,
> +  ensuring it never leaves the device. Only the public certificate leaves the
> +  device for encrypting future update packages.

We have a 'secure element' on our boards so I'd love to try it in
combinaison with that...

> +- As a side effect, the size of the update package may significantly increase
> +  in a large-scale deployment. To enhance scalability, consider using group
> +  keys. Smaller groups should be preferred over larger ones.

One could also generate a batch of swu, each for a smaller group of
devices -- this degrades security less than group keys which have to get
out of the device at some point and could leak.

Another consideration here is more of a bug than a problem, but I think
the current core/stream_interface.c save_stream() will break with a few
handful of devices -- it's only reading 16k to the temporary file from
which we extract the sw-description and its signature, but with this and
a very small sw-description I hit 16k with around 35 devices (I expect
to hit it with even less on real world sw-descriptions)
We should probably increase that size quite a bit, or make it pull in
more data dynamically...



Cheers,
Stefano Babic Jan. 16, 2024, 8:31 p.m. UTC | #2
Hi,

On 16.01.24 01:57, Dominique MARTINET wrote:
> Michael Glembotzki wrote on Mon, Jan 15, 2024 at 08:26:45PM +0100:
>> Signed-off-by: Michael Glembotzki <Michael.Glembotzki@iris-sensing.com>
> 
> Thanks for this patch series!
> 
> I'll check the code more thoroughly as time permits but these points
> below aside I didn't see any obviously big problem.
> 

I have not yet checked the code, I just jump in the discussion:

> 
>> +Example Asymmetrically Encrypted Image
>> +--------------------------------------
>> +
>> +The image artifacts should be symmetrically encrypted and signed in advance.
> 
> Sorry if this has been discussed earlier, but if I read this (and the
> code) correctly, we're decrypting the data first and then checking the
> signature -- I think it'd make more sense to do it the other way around
> even if it means shuffling a bit of code around (cpio's first
> extract_file_to_tmp for swdesc would be no encryption and then after
> signature has been checked we can decrypt it)
> 
> Best practices would be to check signature as soon as possible; we're
> already trusting unchecked input with cpio header processing, cms
> decrpytion code is considerably harder to validate for
> robustness/security and might have vulnerabilities or leak secrets
> through error timing on invalid input.

Nevertheless, symmetric encryption is since long time supported, and 
order is that sw-description is first decrypted and then verified. Even 
if you exposed a right concept, changing the order introduces a hard 
backward compatibility.

On the other hand, I do not think that this introduce an issue: the 
major point with decryption is that devices should have a safe box to 
store the keys. Some customers of mine have just use encryption without 
verification, because, they say, there is no way to push a malformed 
software until key is safe. Of course, it is better to have both, but if 
sw-description is encrypted and it contains sha256 for artifacts the 
artifacts are verified as well.

> 
> As bonus, this would allow making this feature more easily optional in
> configuration (this is $jobs' problem but we're not building images
> directly for our customers, so we'll have to support plain text
> sw-description forever, but I'd love to upgrade and provide this as an
> optional feature)

Is it an optional feature, isn't it ? SWUpdate must be explicitly 
configured to enable encryption (via symmetric or asymmetric keys) of 
sw-description.

  -- since we've checked the signature first, we can
> trust a header or whatever (alternate file name?) to decide whether it's
> plain text or encrypted and proceed accordingly.
> 
> 
>> +Create the new update image (SWU):
>> +
>> +::
>> +
>> +        #!/bin/sh
>> +
>> +        FILES="sw-description sw-description.sig rootfs.ext4.enc"
>> +
>> +        for i in $FILES; do
>> +            echo $i;done | cpio -ov -H crc >  firmware.swu
> 
> (code formatting here is a bit weird)
> 
>> +Security Considerations
>> +-----------------------
>> +- Ideally, generate the private key on the device during factory provisioning,
>> +  ensuring it never leaves the device. Only the public certificate leaves the
>> +  device for encrypting future update packages.
> 
> We have a 'secure element' on our boards so I'd love to try it in
> combinaison with that...
> 
>> +- As a side effect, the size of the update package may significantly increase
>> +  in a large-scale deployment. To enhance scalability, consider using group
>> +  keys. Smaller groups should be preferred over larger ones.
> 
> One could also generate a batch of swu, each for a smaller group of
> devices

Yes, possible, it could be a use case.

> -- this degrades security less than group keys which have to get
> out of the device at some point and could leak.
> 
> Another consideration here is more of a bug than a problem, but I think
> the current core/stream_interface.c save_stream() will break with a few
> handful of devices -- it's only reading 16k to the temporary file from
> which we extract the sw-description and its signature, but with this and
> a very small sw-description I hit 16k with around 35 devices (I expect
> to hit it with even less on real world sw-descriptions)
> We should probably increase that size quite a bit, or make it pull in
> more data dynamically...

You hit an issue and this should be then fixed - save_stream already 
detect the size for sw-description, it should then copy until the end 
and get again the size of the signature from the cpio header.

Best regards,
Stefano
Dominique MARTINET Jan. 16, 2024, 11:19 p.m. UTC | #3
Stefano Babic wrote on Tue, Jan 16, 2024 at 09:31:52PM +0100:
> > > +Example Asymmetrically Encrypted Image
> > > +--------------------------------------
> > > +
> > > +The image artifacts should be symmetrically encrypted and signed in advance.
> > 
> > Sorry if this has been discussed earlier, but if I read this (and the
> > code) correctly, we're decrypting the data first and then checking the
> > signature -- I think it'd make more sense to do it the other way around
> > even if it means shuffling a bit of code around (cpio's first
> > extract_file_to_tmp for swdesc would be no encryption and then after
> > signature has been checked we can decrypt it)
> > 
> > Best practices would be to check signature as soon as possible; we're
> > already trusting unchecked input with cpio header processing, cms
> > decrpytion code is considerably harder to validate for
> > robustness/security and might have vulnerabilities or leak secrets
> > through error timing on invalid input.
> 
> Nevertheless, symmetric encryption is since long time supported, and order
> is that sw-description is first decrypted and then verified. Even if you
> exposed a right concept, changing the order introduces a hard backward
> compatibility.

Yes, symmetric encryption cannot be changed, so if we do this it'll be
somewhat confusing in that we'll have symmetric do decryption then
signature check and assymetric do it the other way around.

I still think it's worth it.

> On the other hand, I do not think that this introduce an issue: the major
> point with decryption is that devices should have a safe box to store the
> keys. Some customers of mine have just use encryption without verification,
> because, they say, there is no way to push a malformed software until key is
> safe. Of course, it is better to have both, but if sw-description is
> encrypted and it contains sha256 for artifacts the artifacts are verified as
> well.

I am sorry for them...
The current sw-description symmetric encryption is using AES with a
fixed key/iv (yes, it's possible to change iv after each update -- how
many users actually do it?) in cbc mode;

- the key is 100% shared and symmetric so if an attacker gets their hand
on any device they can create updates that'll be installable on all
devices freely!
- even if iv wasn't fixed, AES cbc mode without hmac has no tampering
protections so attacks such as this one are possible:
https://crypto.stackexchange.com/a/66086
It might take a few attempts to get the location right but in our case I
think it wouldn't be too difficult to change just a hash somewhere to
install only part of a update (that'll fail the update but if part of it
isn't A/B-safe it might have some impact), or with a few more attempts
change the version of a component so it's skipped with a successful
install.
(I didn't spend a lot of time studying this, but I've read enough bad
things about AES CBC to not trust it with authentifying data)

Anyway, we're not here to discuss about bad practices in current code -
let them do as they want.


For these patches at hand, assuming signature check is enabled, what I'm
worried about is that the code for CMS decryption is much larger than
simple AES CBC code -- there's ASN.1 parsing and a lot of fun x509
stuff which has had vulnerabilities in them, so I'd be much more at ease
knowing the data has been verified to be trusted first.
(Admittedly, the .sig file parsing should be pretty similar, so if there
are such bugs there they might already be reachable... Hopefully that
part has had more scrutinity though as it's untrusted data by design)

I'm curious so I'll try to setup AFL++ on something that calls
extract_file_to_tmp() directly with various options when I can find
time... Might take a week or two but I'll come back on this.


> > As bonus, this would allow making this feature more easily optional in
> > configuration (this is $jobs' problem but we're not building images
> > directly for our customers, so we'll have to support plain text
> > sw-description forever, but I'd love to upgrade and provide this as an
> > optional feature)
> 
> Is it an optional feature, isn't it ? SWUpdate must be explicitly configured
> to enable encryption (via symmetric or asymmetric keys) of sw-description.

It's optional at compile time - this is my eternal problem that I'm not
the final integrator but just providing "easy to use" blocks, so to
allow this new usecase while preserving backwards compatibility I'd need
to ship two swupdate binaries with either mode enable (plaintext or
asym).
In this particular case it'd be easy enough to add a setting in
swupdate.cfg to toggle that instead, but that's still less flexible than
what I'm suggesting here: 
if we check signature first, there should be no problem trusting the
file itself to decide if it wants to be encrypted or not, and decide
based on what was given.
This is already what we do for artifact encryption (for anything else
than sw-description), the sw-description decides if files are encrypted
or not so it's possible to have a decyption key configured but still
install an update where nothing is encrypted.

> > -- this degrades security less than group keys which have to get
> > out of the device at some point and could leak.
> > 
> > Another consideration here is more of a bug than a problem, but I think
> > the current core/stream_interface.c save_stream() will break with a few
> > handful of devices -- it's only reading 16k to the temporary file from
> > which we extract the sw-description and its signature, but with this and
> > a very small sw-description I hit 16k with around 35 devices (I expect
> > to hit it with even less on real world sw-descriptions)
> > We should probably increase that size quite a bit, or make it pull in
> > more data dynamically...
> 
> You hit an issue and this should be then fixed - save_stream already detect
> the size for sw-description, it should then copy until the end and get again
> the size of the signature from the cpio header.

Yes, up to some (configurable?) limit though or that opens up a new
attack (making TMPDIR full) because data hasn't been verified yet at
this point.
Michael Glembotzki Jan. 17, 2024, 12:01 p.m. UTC | #4
Hi,

Am Mi., 17. Jan. 2024 um 00:19 Uhr schrieb Dominique MARTINET
<dominique.martinet@atmark-techno.com>:
>
> Stefano Babic wrote on Tue, Jan 16, 2024 at 09:31:52PM +0100:
> > > > +Example Asymmetrically Encrypted Image
> > > > +--------------------------------------
> > > > +
> > > > +The image artifacts should be symmetrically encrypted and signed in advance.
> > >
> > > Sorry if this has been discussed earlier, but if I read this (and the
> > > code) correctly, we're decrypting the data first and then checking the
> > > signature -- I think it'd make more sense to do it the other way around
> > > even if it means shuffling a bit of code around (cpio's first
> > > extract_file_to_tmp for swdesc would be no encryption and then after
> > > signature has been checked we can decrypt it)
> > >
> > > Best practices would be to check signature as soon as possible; we're
> > > already trusting unchecked input with cpio header processing, cms
> > > decrpytion code is considerably harder to validate for
> > > robustness/security and might have vulnerabilities or leak secrets
> > > through error timing on invalid input.
> >
> > Nevertheless, symmetric encryption is since long time supported, and order
> > is that sw-description is first decrypted and then verified. Even if you
> > exposed a right concept, changing the order introduces a hard backward
> > compatibility.
>
> Yes, symmetric encryption cannot be changed, so if we do this it'll be
> somewhat confusing in that we'll have symmetric do decryption then
> signature check and assymetric do it the other way around.
Switching the order only for asymmetric decryption seems off to me. We could
fix this by using a Compile Time Config. New projects go for the secure way
(verify first, then decrypt), while existing ones stick to the old
method (decrypt
first, then verify) to avoid issues. This way, we don't need to resolve it
immediately within this patch series.

> For these patches at hand, assuming signature check is enabled, what I'm
> worried about is that the code for CMS decryption is much larger than
> simple AES CBC code -- there's ASN.1 parsing and a lot of fun x509
> stuff which has had vulnerabilities in them, so I'd be much more at ease
> knowing the data has been verified to be trusted first.
> (Admittedly, the .sig file parsing should be pretty similar, so if there
> are such bugs there they might already be reachable... Hopefully that
> part has had more scrutinity though as it's untrusted data by design)
What about CMS verify? If you consider the possibility of similar
vulnerabilities in CMS decrypt, there could also be potential issues in
CMS verify. So, changing the order might not yield any immediate benefits.

> I'm curious so I'll try to setup AFL++ on something that calls
> extract_file_to_tmp() directly with various options when I can find
> time... Might take a week or two but I'll come back on this.
I would be interested in the results.

> > > +- As a side effect, the size of the update package may significantly increase
> > > +  in a large-scale deployment. To enhance scalability, consider using group
> > > +  keys. Smaller groups should be preferred over larger ones.
> >
> > One could also generate a batch of swu, each for a smaller group of
> > devices
> Yes, possible, it could be a use case
By the term group keys I actually meant group sizes of perhaps e.g. 20..50
devices per group. In the end, you have to estimate the maximum expected
size of the update packages depending on your expected total number of
devices (or batches) and key type/size.

> > > Another consideration here is more of a bug than a problem, but I think
> > > the current core/stream_interface.c save_stream() will break with a few
> > > handful of devices -- it's only reading 16k to the temporary file from
> > > which we extract the sw-description and its signature, but with this and
> > > a very small sw-description I hit 16k with around 35 devices (I expect
> > > to hit it with even less on real world sw-descriptions)
> > > We should probably increase that size quite a bit, or make it pull in
> > > more data dynamically...
> >
> > You hit an issue and this should be then fixed - save_stream already detect
> > the size for sw-description, it should then copy until the end and get again
> > the size of the signature from the cpio header.
>
Good finding. A multiple of 16K is read (depending on the size of
sw-description). This is a problem if the sw-desc is just a few bytes below the
16K limit. Then the signature no longer fits. Will you fix it Dominique?

Best regards,
Michael
Stefano Babic Jan. 21, 2024, 2:20 p.m. UTC | #5
Hi Dominique,

On 17.01.24 00:19, Dominique MARTINET wrote:
> Stefano Babic wrote on Tue, Jan 16, 2024 at 09:31:52PM +0100:
>>>> +Example Asymmetrically Encrypted Image
>>>> +--------------------------------------
>>>> +
>>>> +The image artifacts should be symmetrically encrypted and signed in advance.
>>>
>>> Sorry if this has been discussed earlier, but if I read this (and the
>>> code) correctly, we're decrypting the data first and then checking the
>>> signature -- I think it'd make more sense to do it the other way around
>>> even if it means shuffling a bit of code around (cpio's first
>>> extract_file_to_tmp for swdesc would be no encryption and then after
>>> signature has been checked we can decrypt it)
>>>
>>> Best practices would be to check signature as soon as possible; we're
>>> already trusting unchecked input with cpio header processing, cms
>>> decrpytion code is considerably harder to validate for
>>> robustness/security and might have vulnerabilities or leak secrets
>>> through error timing on invalid input.
>>
>> Nevertheless, symmetric encryption is since long time supported, and order
>> is that sw-description is first decrypted and then verified. Even if you
>> exposed a right concept, changing the order introduces a hard backward
>> compatibility.
> 
> Yes, symmetric encryption cannot be changed, so if we do this it'll be
> somewhat confusing in that we'll have symmetric do decryption then
> signature check and assymetric do it the other way around.
> 
> I still think it's worth it.

I agree that reverting the order makes sense, but it should be done in a 
way that does not break devices in field.

> 
>> On the other hand, I do not think that this introduce an issue: the major
>> point with decryption is that devices should have a safe box to store the
>> keys. Some customers of mine have just use encryption without verification,
>> because, they say, there is no way to push a malformed software until key is
>> safe. Of course, it is better to have both, but if sw-description is
>> encrypted and it contains sha256 for artifacts the artifacts are verified as
>> well.
> 
> I am sorry for them...
> The current sw-description symmetric encryption is using AES with a
> fixed key/iv (yes, it's possible to change iv after each update -- how
> many users actually do it?) in cbc mode;

I will happy to provide statistics if users will share their experience, 
but I got no answers each time I asked to share own experience (like 
just to report who is using the project...)

I can just tell that some customers of mine are using it.


> 
> - the key is 100% shared and symmetric so if an attacker gets their hand
> on any device they can create updates that'll be installable on all
> devices freely!

But this is the very old story with keys. Of course it is, but it always 
happened when the secrets are not secret anymore.

It is best practice in SWupdate to upgrade the symmetric key with any 
update, but as drawback a downgrading doesn't work anymore.

> - even if iv wasn't fixed, AES cbc mode without hmac has no tampering
> protections so attacks such as this one are possible:
> https://crypto.stackexchange.com/a/66086
> It might take a few attempts to get the location right but in our case I
> think it wouldn't be too difficult to change just a hash somewhere to
> install only part of a update (that'll fail the update but if part of it
> isn't A/B-safe it might have some impact),

If update fails, the installed artifact is not activated. If it is, the 
integrator has done a bad work.

> or with a few more attempts
> change the version of a component so it's skipped with a successful
> install.

Version of artifacts in the SWU are put inside sw-description and they 
are verified. I don't think it is easy as you describe.

> (I didn't spend a lot of time studying this, but I've read enough bad
> things about AES CBC to not trust it with authentifying data)

Anyway, independently of this, I would like to add more decryption 
algorithms to SWUpdate, and not just rely on AES-CBC. I would like to 
add, if some company will finance the development, AES-CTR to have 
encrypted delta updates, for example. But I see that the "encrypted" 
attribute could switch from boll value to a string with the selected 
crypto alg.

> 
> Anyway, we're not here to discuss about bad practices in current code -
> let them do as they want.
> 
> 
> For these patches at hand, assuming signature check is enabled, what I'm
> worried about is that the code for CMS decryption is much larger than
> simple AES CBC code 

Sorry, this is not a reason. There is a lot of complicated features, and 
of course this requires code. If there will be a leak / bug, it should 
be fixed. What shouldn't be done, is to reimplement the algorithms 
inside SWUpdate instead of using well proven libraries, and this is not 
done.

>-- there's ASN.1 parsing and a lot of fun x509
> stuff which has had vulnerabilities in them, so I'd be much more at ease
> knowing the data has been verified to be trusted first.
> (Admittedly, the .sig file parsing should be pretty similar, so if there
> are such bugs there they might already be reachable

Right, this is not an argument. Verification takes place, and it is also 
not easy.

>... Hopefully that
> part has had more scrutinity though as it's untrusted data by design)

I think this is just your feeling.

> 
> I'm curious so I'll try to setup AFL++ on something that calls
> extract_file_to_tmp() directly with various options when I can find
> time... Might take a week or two but I'll come back on this.
> 
> 
>>> As bonus, this would allow making this feature more easily optional in
>>> configuration (this is $jobs' problem but we're not building images
>>> directly for our customers, so we'll have to support plain text
>>> sw-description forever, but I'd love to upgrade and provide this as an
>>> optional feature)
>>
>> Is it an optional feature, isn't it ? SWUpdate must be explicitly configured
>> to enable encryption (via symmetric or asymmetric keys) of sw-description.
> 
> It's optional at compile time - this is my eternal problem that I'm not
> the final integrator but just providing "easy to use" blocks, so to
> allow this new usecase while preserving backwards compatibility I'd need
> to ship two swupdate binaries with either mode enable (plaintext or
> asym).

Yes, understood - not easy to be solved. SWUpdate requires that the 
integrator understands what is going on, and adjusts SWupdate for the 
specific project. It is not thought to deliver N binaries and to let the 
integrator just to pick one up.

> In this particular case it'd be easy enough to add a setting in
> swupdate.cfg to toggle that instead, but that's still less flexible than
> what I'm suggesting here:

The reason that these security features are defined at compile time is 
to avoid that in some way an attacker can circumvent the chosen way. If 
it is not compiled, it cannot run. So I already rejected patches that 
allows to skip verification process, or to change verification / 
decryption at runtime.

> if we check signature first, there should be no problem trusting the
> file itself to decide if it wants to be encrypted or not, and decide
> based on what was given.
> This is already what we do for artifact encryption (for anything else
> than sw-description), the sw-description decides if files are encrypted
> or not so it's possible to have a decyption key configured but still
> install an update where nothing is encrypted.


Verification first should be the path to do, and I cannot say why I did 
the opposite in the past for sw-description. Rather, nobody raised the 
point when I pushed it. I will agree to change it if we maintain a 
compiler switch (at least for some versions) that guarantees 
compatibility with the past, else there will be a lot of complaints.

> 
>>> -- this degrades security less than group keys which have to get
>>> out of the device at some point and could leak.
>>>
>>> Another consideration here is more of a bug than a problem, but I think
>>> the current core/stream_interface.c save_stream() will break with a few
>>> handful of devices -- it's only reading 16k to the temporary file from
>>> which we extract the sw-description and its signature, but with this and
>>> a very small sw-description I hit 16k with around 35 devices (I expect
>>> to hit it with even less on real world sw-descriptions)
>>> We should probably increase that size quite a bit, or make it pull in
>>> more data dynamically...
>>
>> You hit an issue and this should be then fixed - save_stream already detect
>> the size for sw-description, it should then copy until the end and get again
>> the size of the signature from the cpio header.
> 
> Yes, up to some (configurable?) limit though or that opens up a new
> attack (making TMPDIR full) because data hasn't been verified yet at
> this point.
> 

Best regards,
Stefano
Stefano Babic Jan. 21, 2024, 2:23 p.m. UTC | #6
Hi Michael,

On 17.01.24 13:01, Michael Glembotzki wrote:
> Hi,
> 
> Am Mi., 17. Jan. 2024 um 00:19 Uhr schrieb Dominique MARTINET
> <dominique.martinet@atmark-techno.com>:
>>
>> Stefano Babic wrote on Tue, Jan 16, 2024 at 09:31:52PM +0100:
>>>>> +Example Asymmetrically Encrypted Image
>>>>> +--------------------------------------
>>>>> +
>>>>> +The image artifacts should be symmetrically encrypted and signed in advance.
>>>>
>>>> Sorry if this has been discussed earlier, but if I read this (and the
>>>> code) correctly, we're decrypting the data first and then checking the
>>>> signature -- I think it'd make more sense to do it the other way around
>>>> even if it means shuffling a bit of code around (cpio's first
>>>> extract_file_to_tmp for swdesc would be no encryption and then after
>>>> signature has been checked we can decrypt it)
>>>>
>>>> Best practices would be to check signature as soon as possible; we're
>>>> already trusting unchecked input with cpio header processing, cms
>>>> decrpytion code is considerably harder to validate for
>>>> robustness/security and might have vulnerabilities or leak secrets
>>>> through error timing on invalid input.
>>>
>>> Nevertheless, symmetric encryption is since long time supported, and order
>>> is that sw-description is first decrypted and then verified. Even if you
>>> exposed a right concept, changing the order introduces a hard backward
>>> compatibility.
>>
>> Yes, symmetric encryption cannot be changed, so if we do this it'll be
>> somewhat confusing in that we'll have symmetric do decryption then
>> signature check and assymetric do it the other way around.
> Switching the order only for asymmetric decryption seems off to me. We could
> fix this by using a Compile Time Config. New projects go for the secure way
> (verify first, then decrypt), while existing ones stick to the old
> method (decrypt
> first, then verify) to avoid issues. This way, we don't need to resolve it
> immediately within this patch series.

I agree with this approach to guarantee backward compatibility. 
Dominique is right to ask to revert the order, but well, there are a lot 
of devices in field.

> 
>> For these patches at hand, assuming signature check is enabled, what I'm
>> worried about is that the code for CMS decryption is much larger than
>> simple AES CBC code -- there's ASN.1 parsing and a lot of fun x509
>> stuff which has had vulnerabilities in them, so I'd be much more at ease
>> knowing the data has been verified to be trusted first.
>> (Admittedly, the .sig file parsing should be pretty similar, so if there
>> are such bugs there they might already be reachable... Hopefully that
>> part has had more scrutinity though as it's untrusted data by design)
> What about CMS verify? If you consider the possibility of similar
> vulnerabilities in CMS decrypt, there could also be potential issues in
> CMS verify. So, changing the order might not yield any immediate benefits.

It is also my point.

> 
>> I'm curious so I'll try to setup AFL++ on something that calls
>> extract_file_to_tmp() directly with various options when I can find
>> time... Might take a week or two but I'll come back on this.
> I would be interested in the results.
> 
>>>> +- As a side effect, the size of the update package may significantly increase
>>>> +  in a large-scale deployment. To enhance scalability, consider using group
>>>> +  keys. Smaller groups should be preferred over larger ones.
>>>
>>> One could also generate a batch of swu, each for a smaller group of
>>> devices
>> Yes, possible, it could be a use case
> By the term group keys I actually meant group sizes of perhaps e.g. 20..50
> devices per group. In the end, you have to estimate the maximum expected
> size of the update packages depending on your expected total number of
> devices (or batches) and key type/size.
> 
>>>> Another consideration here is more of a bug than a problem, but I think
>>>> the current core/stream_interface.c save_stream() will break with a few
>>>> handful of devices -- it's only reading 16k to the temporary file from
>>>> which we extract the sw-description and its signature, but with this and
>>>> a very small sw-description I hit 16k with around 35 devices (I expect
>>>> to hit it with even less on real world sw-descriptions)
>>>> We should probably increase that size quite a bit, or make it pull in
>>>> more data dynamically...
>>>
>>> You hit an issue and this should be then fixed - save_stream already detect
>>> the size for sw-description, it should then copy until the end and get again
>>> the size of the signature from the cpio header.
>>
> Good finding. A multiple of 16K is read (depending on the size of
> sw-description). This is a problem if the sw-desc is just a few bytes below the
> 16K limit. Then the signature no longer fits. Will you fix it Dominique?
> 

Agree that this must be fixed, too, else we can quickly get into trouble 
when sw-description's size increases.


Best regards,
Stefano
Dominique MARTINET Jan. 22, 2024, 5:40 a.m. UTC | #7
Michael Glembotzki wrote on Wed, Jan 17, 2024 at 01:01:59PM +0100:
> > Yes, symmetric encryption cannot be changed, so if we do this it'll be
> > somewhat confusing in that we'll have symmetric do decryption then
> > signature check and assymetric do it the other way around.
> Switching the order only for asymmetric decryption seems off to me. We could
> fix this by using a Compile Time Config. New projects go for the secure way
> (verify first, then decrypt), while existing ones stick to the old
> method (decrypt first, then verify) to avoid issues. This way, we
> don't need to resolve it immediately within this patch series.

Hmm, I feel it's a bit suboptimal to allow the (decrypt async, verify)
order in the first place if we're going to do that, but it's probably
better than delaying this feature.

Okay, let's go with that (if someone works on it)

> > For these patches at hand, assuming signature check is enabled, what I'm
> > worried about is that the code for CMS decryption is much larger than
> > simple AES CBC code -- there's ASN.1 parsing and a lot of fun x509
> > stuff which has had vulnerabilities in them, so I'd be much more at ease
> > knowing the data has been verified to be trusted first.
> > (Admittedly, the .sig file parsing should be pretty similar, so if there
> > are such bugs there they might already be reachable... Hopefully that
> > part has had more scrutinity though as it's untrusted data by design)
>
> What about CMS verify? If you consider the possibility of similar
> vulnerabilities in CMS decrypt, there could also be potential issues in
> CMS verify. So, changing the order might not yield any immediate benefits.

Yes, that's what I pointed at :)
I'd assume verify is more battle-tested than decrypt because the point
is checking untrusted data, but I agree the code will partly be shared
between the two.

> > I'm curious so I'll try to setup AFL++ on something that calls
> > extract_file_to_tmp() directly with various options when I can find
> > time... Might take a week or two but I'll come back on this.
>
> I would be interested in the results.

I've let it run over the weekend (1 day with just cpio/signature, a few
hours with symmetric decryption and 2.5 days with asymmetric decryption)
and was happy to not see any obvious failure.

If anyone cares to check, I've used the attached swupdate-fuzz.c as a
drop-in replacement to swupdate.c + patch and built with afl
(`CC=afl-clang-fast make`), running afl-fuzz as documented (with a few
variant builds and slave processes) after checking my sample input
passed all the way through verifying the signature.

I'm told blind fuzzing isn't actually that well suited for this kind of
cryptography checks (the input will almost always just fail to validate
something obvious) so this doesn't guarantee much, but it's better than
nothing.

> > > > +- As a side effect, the size of the update package may significantly increase
> > > > +  in a large-scale deployment. To enhance scalability, consider using group
> > > > +  keys. Smaller groups should be preferred over larger ones.
> > >
> > > One could also generate a batch of swu, each for a smaller group of
> > > devices
> > Yes, possible, it could be a use case
> By the term group keys I actually meant group sizes of perhaps e.g. 20..50
> devices per group. In the end, you have to estimate the maximum expected
> size of the update packages depending on your expected total number of
> devices (or batches) and key type/size.

I'm not exactly sure what this clarifies. From my understanding there
are two main ways this could be split to limit the number of
certificates in a given "recip" list pass:
- share a key between multiple devices, so your update file will still
be installable on all your fleet
- build multiple update files, so each device still have their own key
but a given update fille will only be installable on part of the fleet
and your update mechanism will need to send the right file to the right
place

Given you were suggesting to not have the key leave the device at
install time (generated on the device), the later is probably more
appropriate to recommend, even if it's more work to setup.
(Well, I have no problem with both or either being listed, each vendor
should make their own informed decision ultimately)

> > > > Another consideration here is more of a bug than a problem, but I think
> > > > the current core/stream_interface.c save_stream() will break with a few
> > > > handful of devices -- it's only reading 16k to the temporary file from
> > > > which we extract the sw-description and its signature, but with this and
> > > > a very small sw-description I hit 16k with around 35 devices (I expect
> > > > to hit it with even less on real world sw-descriptions)
> > > > We should probably increase that size quite a bit, or make it pull in
> > > > more data dynamically...
> > >
> > > You hit an issue and this should be then fixed - save_stream already detect
> > > the size for sw-description, it should then copy until the end and get again
> > > the size of the signature from the cpio header.
> >
> Good finding. A multiple of 16K is read (depending on the size of
> sw-description). This is a problem if the sw-desc is just a few bytes below the
> 16K limit. Then the signature no longer fits. Will you fix it Dominique?

Sorry I was just mis-reading the code there, I missed the part where it
takes into account the sw-description size
(but now you're pointing this out I also think we could get the align
call to make the sig not fit if we try, I've never had any
sw-description close enough to 16k to hit this)


This brings in the issue I gave in an earlier reply which is we're
filling TMPDIR with as much data as an attacker wants:
---
$ dd if=/dev/zero of=sw-description bs=1M count=1000
$ echo sw-description | cpio -H newc -o > test.cpio
$ swupdate -i test.cpio
[...]
[ERROR] : SWUPDATE failed [0] ERROR : cannot write 16384 bytes: No space left on device
[ERROR] : SWUPDATE failed [1] Image invalid or corrupted. Not installing ...
---

Thanksfully swupdate cleans up sw-description immediately after that,
but if update can be triggered openly it's still possible to cause
trouble, so I'll send a patch to limit the size in config with some
arbitrary default value we can discuss there.

I'll have a look at the 16K boundary while I'm at it, it's probably not
hard to reproduce by just inserting an appropriate number of
whitespaces...

(But either way I probably won't have time this week and will have a
busy start of Feb -- so that'll probably be mid-Feb. If you want this
earlier feel free to not wait for me)


Thanks,
Michael Glembotzki March 1, 2024, 1:11 p.m. UTC | #8
Hi,
it's been a while. What is the next step?
Is there still interest in this feature?
I would appreciate a code review.

Best regards
Michael
Dominique MARTINET schrieb am Montag, 22. Januar 2024 um 06:41:14 UTC+1:

> Michael Glembotzki wrote on Wed, Jan 17, 2024 at 01:01:59PM +0100:
> > > Yes, symmetric encryption cannot be changed, so if we do this it'll be
> > > somewhat confusing in that we'll have symmetric do decryption then
> > > signature check and assymetric do it the other way around.
> > Switching the order only for asymmetric decryption seems off to me. We 
> could
> > fix this by using a Compile Time Config. New projects go for the secure 
> way
> > (verify first, then decrypt), while existing ones stick to the old
> > method (decrypt first, then verify) to avoid issues. This way, we
> > don't need to resolve it immediately within this patch series.
>
> Hmm, I feel it's a bit suboptimal to allow the (decrypt async, verify)
> order in the first place if we're going to do that, but it's probably
> better than delaying this feature.
>
> Okay, let's go with that (if someone works on it)
>
> > > For these patches at hand, assuming signature check is enabled, what 
> I'm
> > > worried about is that the code for CMS decryption is much larger than
> > > simple AES CBC code -- there's ASN.1 parsing and a lot of fun x509
> > > stuff which has had vulnerabilities in them, so I'd be much more at 
> ease
> > > knowing the data has been verified to be trusted first.
> > > (Admittedly, the .sig file parsing should be pretty similar, so if 
> there
> > > are such bugs there they might already be reachable... Hopefully that
> > > part has had more scrutinity though as it's untrusted data by design)
> >
> > What about CMS verify? If you consider the possibility of similar
> > vulnerabilities in CMS decrypt, there could also be potential issues in
> > CMS verify. So, changing the order might not yield any immediate 
> benefits.
>
> Yes, that's what I pointed at :)
> I'd assume verify is more battle-tested than decrypt because the point
> is checking untrusted data, but I agree the code will partly be shared
> between the two.
>
> > > I'm curious so I'll try to setup AFL++ on something that calls
> > > extract_file_to_tmp() directly with various options when I can find
> > > time... Might take a week or two but I'll come back on this.
> >
> > I would be interested in the results.
>
> I've let it run over the weekend (1 day with just cpio/signature, a few
> hours with symmetric decryption and 2.5 days with asymmetric decryption)
> and was happy to not see any obvious failure.
>
> If anyone cares to check, I've used the attached swupdate-fuzz.c as a
> drop-in replacement to swupdate.c + patch and built with afl
> (`CC=afl-clang-fast make`), running afl-fuzz as documented (with a few
> variant builds and slave processes) after checking my sample input
> passed all the way through verifying the signature.
>
> I'm told blind fuzzing isn't actually that well suited for this kind of
> cryptography checks (the input will almost always just fail to validate
> something obvious) so this doesn't guarantee much, but it's better than
> nothing.
>
> > > > > +- As a side effect, the size of the update package may 
> significantly increase
> > > > > + in a large-scale deployment. To enhance scalability, consider 
> using group
> > > > > + keys. Smaller groups should be preferred over larger ones.
> > > >
> > > > One could also generate a batch of swu, each for a smaller group of
> > > > devices
> > > Yes, possible, it could be a use case
> > By the term group keys I actually meant group sizes of perhaps e.g. 
> 20..50
> > devices per group. In the end, you have to estimate the maximum expected
> > size of the update packages depending on your expected total number of
> > devices (or batches) and key type/size.
>
> I'm not exactly sure what this clarifies. From my understanding there
> are two main ways this could be split to limit the number of
> certificates in a given "recip" list pass:
> - share a key between multiple devices, so your update file will still
> be installable on all your fleet
> - build multiple update files, so each device still have their own key
> but a given update fille will only be installable on part of the fleet
> and your update mechanism will need to send the right file to the right
> place
>
> Given you were suggesting to not have the key leave the device at
> install time (generated on the device), the later is probably more
> appropriate to recommend, even if it's more work to setup.
> (Well, I have no problem with both or either being listed, each vendor
> should make their own informed decision ultimately)
>
> > > > > Another consideration here is more of a bug than a problem, but I 
> think
> > > > > the current core/stream_interface.c save_stream() will break with 
> a few
> > > > > handful of devices -- it's only reading 16k to the temporary file 
> from
> > > > > which we extract the sw-description and its signature, but with 
> this and
> > > > > a very small sw-description I hit 16k with around 35 devices (I 
> expect
> > > > > to hit it with even less on real world sw-descriptions)
> > > > > We should probably increase that size quite a bit, or make it pull 
> in
> > > > > more data dynamically...
> > > >
> > > > You hit an issue and this should be then fixed - save_stream already 
> detect
> > > > the size for sw-description, it should then copy until the end and 
> get again
> > > > the size of the signature from the cpio header.
> > >
> > Good finding. A multiple of 16K is read (depending on the size of
> > sw-description). This is a problem if the sw-desc is just a few bytes 
> below the
> > 16K limit. Then the signature no longer fits. Will you fix it Dominique?
>
> Sorry I was just mis-reading the code there, I missed the part where it
> takes into account the sw-description size
> (but now you're pointing this out I also think we could get the align
> call to make the sig not fit if we try, I've never had any
> sw-description close enough to 16k to hit this)
>
>
> This brings in the issue I gave in an earlier reply which is we're
> filling TMPDIR with as much data as an attacker wants:
> ---
> $ dd if=/dev/zero of=sw-description bs=1M count=1000
> $ echo sw-description | cpio -H newc -o > test.cpio
> $ swupdate -i test.cpio
> [...]
> [ERROR] : SWUPDATE failed [0] ERROR : cannot write 16384 bytes: No space 
> left on device
> [ERROR] : SWUPDATE failed [1] Image invalid or corrupted. Not installing 
> ...
> ---
>
> Thanksfully swupdate cleans up sw-description immediately after that,
> but if update can be triggered openly it's still possible to cause
> trouble, so I'll send a patch to limit the size in config with some
> arbitrary default value we can discuss there.
>
> I'll have a look at the 16K boundary while I'm at it, it's probably not
> hard to reproduce by just inserting an appropriate number of
> whitespaces...
>
> (But either way I probably won't have time this week and will have a
> busy start of Feb -- so that'll probably be mid-Feb. If you want this
> earlier feel free to not wait for me)
>
>
> Thanks,
> -- 
> Dominique
>
Stefano Babic March 4, 2024, 9:16 p.m. UTC | #9
Hi Michael,

On 01.03.24 14:11, Michael Glembotzki wrote:
>
> Hi,
> it's been a while. What is the next step?

I was thinking about...

> Is there still interest in this feature?

Sure - I think it is a nice feature.

There are some issues shown with this series and I checked deeply the
current status for encryption / decryption, see this thread, too:

	https://groups.google.com/g/swupdate/c/z4oYcaJv5Rs

This convinces me that a rework of current crypto integration will be
necessary.

Nevertheless, this set has also discovered that sw-description.sig is
currently limited to 16KB. This is a topic I set in my TODO list, and I
will solve it.

> I would appreciate a code review.

Sure - I will review soon your patches.

Best regards,
Stefano

>
> Best regards
> Michael
> Dominique MARTINET schrieb am Montag, 22. Januar 2024 um 06:41:14 UTC+1:
>
>     Michael Glembotzki wrote on Wed, Jan 17, 2024 at 01:01:59PM +0100:
>      > > Yes, symmetric encryption cannot be changed, so if we do this
>     it'll be
>      > > somewhat confusing in that we'll have symmetric do decryption then
>      > > signature check and assymetric do it the other way around.
>      > Switching the order only for asymmetric decryption seems off to
>     me. We could
>      > fix this by using a Compile Time Config. New projects go for the
>     secure way
>      > (verify first, then decrypt), while existing ones stick to the old
>      > method (decrypt first, then verify) to avoid issues. This way, we
>      > don't need to resolve it immediately within this patch series.
>
>     Hmm, I feel it's a bit suboptimal to allow the (decrypt async, verify)
>     order in the first place if we're going to do that, but it's probably
>     better than delaying this feature.
>
>     Okay, let's go with that (if someone works on it)
>
>      > > For these patches at hand, assuming signature check is enabled,
>     what I'm
>      > > worried about is that the code for CMS decryption is much
>     larger than
>      > > simple AES CBC code -- there's ASN.1 parsing and a lot of fun x509
>      > > stuff which has had vulnerabilities in them, so I'd be much
>     more at ease
>      > > knowing the data has been verified to be trusted first.
>      > > (Admittedly, the .sig file parsing should be pretty similar, so
>     if there
>      > > are such bugs there they might already be reachable...
>     Hopefully that
>      > > part has had more scrutinity though as it's untrusted data by
>     design)
>      >
>      > What about CMS verify? If you consider the possibility of similar
>      > vulnerabilities in CMS decrypt, there could also be potential
>     issues in
>      > CMS verify. So, changing the order might not yield any immediate
>     benefits.
>
>     Yes, that's what I pointed at :)
>     I'd assume verify is more battle-tested than decrypt because the point
>     is checking untrusted data, but I agree the code will partly be shared
>     between the two.
>
>      > > I'm curious so I'll try to setup AFL++ on something that calls
>      > > extract_file_to_tmp() directly with various options when I can
>     find
>      > > time... Might take a week or two but I'll come back on this.
>      >
>      > I would be interested in the results.
>
>     I've let it run over the weekend (1 day with just cpio/signature, a few
>     hours with symmetric decryption and 2.5 days with asymmetric
>     decryption)
>     and was happy to not see any obvious failure.
>
>     If anyone cares to check, I've used the attached swupdate-fuzz.c as a
>     drop-in replacement to swupdate.c + patch and built with afl
>     (`CC=afl-clang-fast make`), running afl-fuzz as documented (with a few
>     variant builds and slave processes) after checking my sample input
>     passed all the way through verifying the signature.
>
>     I'm told blind fuzzing isn't actually that well suited for this kind of
>     cryptography checks (the input will almost always just fail to validate
>     something obvious) so this doesn't guarantee much, but it's better than
>     nothing.
>
>      > > > > +- As a side effect, the size of the update package may
>     significantly increase
>      > > > > + in a large-scale deployment. To enhance scalability,
>     consider using group
>      > > > > + keys. Smaller groups should be preferred over larger ones.
>      > > >
>      > > > One could also generate a batch of swu, each for a smaller
>     group of
>      > > > devices
>      > > Yes, possible, it could be a use case
>      > By the term group keys I actually meant group sizes of perhaps
>     e.g. 20..50
>      > devices per group. In the end, you have to estimate the maximum
>     expected
>      > size of the update packages depending on your expected total
>     number of
>      > devices (or batches) and key type/size.
>
>     I'm not exactly sure what this clarifies. From my understanding there
>     are two main ways this could be split to limit the number of
>     certificates in a given "recip" list pass:
>     - share a key between multiple devices, so your update file will still
>     be installable on all your fleet
>     - build multiple update files, so each device still have their own key
>     but a given update fille will only be installable on part of the fleet
>     and your update mechanism will need to send the right file to the right
>     place
>
>     Given you were suggesting to not have the key leave the device at
>     install time (generated on the device), the later is probably more
>     appropriate to recommend, even if it's more work to setup.
>     (Well, I have no problem with both or either being listed, each vendor
>     should make their own informed decision ultimately)
>
>      > > > > Another consideration here is more of a bug than a problem,
>     but I think
>      > > > > the current core/stream_interface.c save_stream() will
>     break with a few
>      > > > > handful of devices -- it's only reading 16k to the
>     temporary file from
>      > > > > which we extract the sw-description and its signature, but
>     with this and
>      > > > > a very small sw-description I hit 16k with around 35
>     devices (I expect
>      > > > > to hit it with even less on real world sw-descriptions)
>      > > > > We should probably increase that size quite a bit, or make
>     it pull in
>      > > > > more data dynamically...
>      > > >
>      > > > You hit an issue and this should be then fixed - save_stream
>     already detect
>      > > > the size for sw-description, it should then copy until the
>     end and get again
>      > > > the size of the signature from the cpio header.
>      > >
>      > Good finding. A multiple of 16K is read (depending on the size of
>      > sw-description). This is a problem if the sw-desc is just a few
>     bytes below the
>      > 16K limit. Then the signature no longer fits. Will you fix it
>     Dominique?
>
>     Sorry I was just mis-reading the code there, I missed the part where it
>     takes into account the sw-description size
>     (but now you're pointing this out I also think we could get the align
>     call to make the sig not fit if we try, I've never had any
>     sw-description close enough to 16k to hit this)
>
>
>     This brings in the issue I gave in an earlier reply which is we're
>     filling TMPDIR with as much data as an attacker wants:
>     ---
>     $ dd if=/dev/zero of=sw-description bs=1M count=1000
>     $ echo sw-description | cpio -H newc -o > test.cpio
>     $ swupdate -i test.cpio
>     [...]
>     [ERROR] : SWUPDATE failed [0] ERROR : cannot write 16384 bytes: No
>     space left on device
>     [ERROR] : SWUPDATE failed [1] Image invalid or corrupted. Not
>     installing ...
>     ---
>
>     Thanksfully swupdate cleans up sw-description immediately after that,
>     but if update can be triggered openly it's still possible to cause
>     trouble, so I'll send a patch to limit the size in config with some
>     arbitrary default value we can discuss there.
>
>     I'll have a look at the 16K boundary while I'm at it, it's probably not
>     hard to reproduce by just inserting an appropriate number of
>     whitespaces...
>
>     (But either way I probably won't have time this week and will have a
>     busy start of Feb -- so that'll probably be mid-Feb. If you want this
>     earlier feel free to not wait for me)
>
>
>     Thanks,
>     --
>     Dominique
>
> --
> You received this message because you are subscribed to the Google
> Groups "swupdate" group.
> To unsubscribe from this group and stop receiving emails from it, send
> an email to swupdate+unsubscribe@googlegroups.com
> <mailto:swupdate+unsubscribe@googlegroups.com>.
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/swupdate/b0c55eb3-7996-4025-a1cb-c6e9b95f4732n%40googlegroups.com <https://groups.google.com/d/msgid/swupdate/b0c55eb3-7996-4025-a1cb-c6e9b95f4732n%40googlegroups.com?utm_medium=email&utm_source=footer>.
diff mbox series

Patch

diff --git a/doc/source/asym_encrypted_images.rst b/doc/source/asym_encrypted_images.rst
new file mode 100644
index 0000000..aa7bc5c
--- /dev/null
+++ b/doc/source/asym_encrypted_images.rst
@@ -0,0 +1,153 @@ 
+.. SPDX-FileCopyrightText: 2023 Michael Glembotzki <michael.glembotzki@iris-sensing.com>
+.. SPDX-License-Identifier: GPL-2.0-only
+
+Asymmetrically Encrypted Update Images
+======================================
+
+Asymmetrically encrypted update images are realized by an asymmetrical
+encrypted sw-description, making it possible to decrypt images device specific.
+The artifacts themselves are still encrypted symmetrically. An AES key can
+optionally be provided in the sw-description, or the default AES key will be
+used. Cryptographic Message Syntax (CMS) is used for decryption.
+
+
+Use Cases
+---------
+
+- Asymmetrically encrypted update images, with individual device key pairs, are
+  inherently more secure than a purely symmetrical solution, because one
+  compromised private device key does not affect the security of the others.
+- If ``CONFIG_SIGNED_IMAGES`` is enabled too and a device's private key is
+  compromised, the key pair can be excluded from the list of eligible devices
+  for receiving new update images.
+- The AES key can be securely **exchanged** with each new update image, as it is
+  part of the sw-description, even in the absence of direct access to the
+  device.
+
+
+Create a Self-Signed Device Key Pair
+------------------------------------
+
+As an example, an elliptic curve key pair (PEM) is generated for a single
+device. These steps must be repeated for all other devices. An RSA key pair
+could be used in the same way.
+
+::
+
+        # Create a private key and a self-signed certificate
+        openssl ecparam -name secp521r1 -genkey -noout -out device-key-001.pem
+        openssl req -new -x509 -key device-key-001.pem -out device-cert-001.pem -subj "/O=SWUpdate /CN=target"
+
+        # Combine the private key and the certificate into a single file
+        cat device-key-001.pem device-cert-001.pem > device-001.pem
+
+
+Symmetric Encryption of Artifacts
+---------------------------------
+
+Generate an AES key and IV, as familiar from
+:ref:`symmetric image encryption <sym-encrypted-images>`. The encryption
+process for the artifacts remains unchanged.
+
+
+Encryption of sw-description for Multiple Devices
+-------------------------------------------------
+
+All device certificates togther are used for encryption.
+
+::
+
+        # Encrypt sw-description for multiple devices
+        openssl cms -encrypt -aes-256-cbc -in <INFILE> -out <OUTFILE> -outform DER -recip <CERT_1> <CERT_2> <CERT_X>
+
+Replace ``<INFILE>`` with the plain `sw-description` (e.g.
+`sw-description.in`) and the encrypted ``<OUTFILE>`` with `sw-description`.
+``<CERT_1>``, ``<CERT_2>``, [...] ``<CERT_X>`` constitute the comprehensive
+list of devices intended for encryption.
+
+
+Decryption of sw-description for a Single Device
+------------------------------------------------
+
+The combined key pair (private key and certificate) is used for decryption.
+SWUpdate handles the decryption process autonomously. Manually executing this
+step is not necessary and is provided here solely for development purposes.
+
+::
+
+        # Decrypt sw-description for a single device
+        openssl cms -decrypt -in <INFILE>  -out ``<OUTFILE>`` -inform DER -inkey <PRIVATE_KEY_1> -recip <CERT_1>
+
+Replace the encrypted ``<INFILE>`` with `sw-description` and the
+``<OUTFILE>`` with plain `sw-description` (e.g. `sw-description.in`).
+``<PRIVATE_KEY_1>`` and ``<CERT_1>`` are used for the decryption.
+
+
+Example Asymmetrically Encrypted Image
+--------------------------------------
+
+The image artifacts should be symmetrically encrypted and signed in advance.
+Now, create a plain `sw-description.in` file. The ``encrypted`` attribute is
+necessary for encrypted artifacts. While it is strongly recommended to provide
+the attributes ``aes-key`` (global) and ``ivt`` (artifact-specific), they are
+not mandatory. If no ``aes-key`` or ``ivt`` is provided, the provided default
+``aes-key``/``ivt`` will be used.
+
+::
+
+        software =
+        {
+            version = "0.0.1";
+            aes-key = "ed73b9d3bf9c655d5a0b04836d8be48660a4a4bb6f4aa07c6778e00e342881ac";
+            images: ({
+                filename = "rootfs.ext4.enc";
+                device = "/dev/mmcblk0p3";
+                sha256 = "131159df3a4efaa890ff80173664a125c496c458dd432a8a6acae18872e35822";
+                encrypted = true;
+                ivt = "ea34a55a0c3476ed78f238ac87a7970c";
+            });
+        }
+
+
+Asymmetrically encrypt the `sw-description` for multiple devices:
+::
+
+        openssl cms -encrypt -aes-256-cbc -in sw-description.in -out sw-description -outform DER -recip device-cert-001.pem device-cert-002.pem device-cert-003.pem
+
+
+Create the new update image (SWU):
+
+::
+
+        #!/bin/sh
+
+        FILES="sw-description sw-description.sig rootfs.ext4.enc"
+
+        for i in $FILES; do
+            echo $i;done | cpio -ov -H crc >  firmware.swu
+
+
+Running SWUpdate with Asymmetrically Encrypted Images
+-----------------------------------------------------
+
+Asymmetric encryption support can be enabled by configuring the compile-time
+option ``CONFIG_ASYM_ENCRYPTED_SW_DESCRIPTION``. To pass the combined key pair
+(PEM) generated earlier to SWUpdate, use the ``-a`` argument. Alternatively,
+use the ``asym-decryption-keypair`` parameter in the ``swupdate.cfg``.
+
+
+Security Considerations
+-----------------------
+- Ideally, generate the private key on the device during factory provisioning,
+  ensuring it never leaves the device. Only the public certificate leaves the
+  device for encrypting future update packages.
+- This feature should be used in conjunction with signature verification
+  (``CONFIG_SIGNED_IMAGES``) to ensure data integrity. In principle, anyone
+  with the corresponding device certificate can create update packages.
+- As a side effect, the size of the update package may significantly increase
+  in a large-scale deployment. To enhance scalability, consider using group
+  keys. Smaller groups should be preferred over larger ones.
+- Exchange the AES key in the sw-description with each update package.
+- Avoid encrypting new update packages for compromised devices, if there is no
+  direct access to the device or if unauthorized users have access to new update
+  packages.
diff --git a/doc/source/encrypted_images.rst b/doc/source/encrypted_images.rst
index 2b7c1ee..bc23681 100644
--- a/doc/source/encrypted_images.rst
+++ b/doc/source/encrypted_images.rst
@@ -1,6 +1,8 @@ 
 .. SPDX-FileCopyrightText: 2013-2021 Stefano Babic <sbabic@denx.de>
 .. SPDX-License-Identifier: GPL-2.0-only
 
+.. _sym-encrypted-images:
+
 Symmetrically Encrypted Update Images
 =====================================
 
diff --git a/doc/source/index.rst b/doc/source/index.rst
index c3a8e88..3ed531a 100644
--- a/doc/source/index.rst
+++ b/doc/source/index.rst
@@ -41,6 +41,7 @@  SWUpdate Documentation
    sw-description.rst
    signed_images.rst
    encrypted_images.rst
+   asym_encrypted_images.rst
    handlers.rst
    mongoose.rst
    suricatta.rst
diff --git a/doc/source/roadmap.rst b/doc/source/roadmap.rst
index dc7d547..4e6caf4 100644
--- a/doc/source/roadmap.rst
+++ b/doc/source/roadmap.rst
@@ -138,11 +138,6 @@  BTRFS supports subvolume and delta backup for volumes - supporting subvolumes is
 to move the delta approach to filesystems, while SWUpdate should apply the deltas
 generated by BTRFS utilities.
 
-Security
-========
-
-- add support for asymmetryc decryption
-
 Support for evaluation boards
 =============================
 
diff --git a/doc/source/sw-description.rst b/doc/source/sw-description.rst
index 480ff4d..6e7e9bb 100644
--- a/doc/source/sw-description.rst
+++ b/doc/source/sw-description.rst
@@ -1441,8 +1441,17 @@  There are 4 main sections inside sw-description:
    |             |          | scripts    | and must be decrypted before          |
    |             |          |            | installing.                           |
    +-------------+----------+------------+---------------------------------------+
-   | ivt         | string   | images     | IVT in case of encrypted artefact     |
-   |             |          | files      | It has no value if "encrypted" is not |
+   | aes-key     | string   |            | Optional AES key for encrypted        |
+   |             |          |            | artefacts. It has no effect if not    |
+   |             |          |            | compiled with                         |
+   |             |          |            | `CONFIG_ASYM_ENCRYPTED_SW_DESCRIPTION`|
+   |             |          |            | or if attribute "encrypted" is not    |
+   |             |          |            | set. If no AES key is provided the    |
+   |             |          |            | default AES key is used. It is an     |
+   |             |          |            | ASCII hex string of 16/24/32 chars.   |
+   +-------------+----------+------------+---------------------------------------+
+   | ivt         | string   | images     | Optional IVT for encrypted artefacts. |
+   |             |          | files      | It has no effect if "encrypted" is not|
    |             |          | scripts    | set. Each artefact can have an own    |
    |             |          |            | IVT to avoid attacker can guess the   |
    |             |          |            | the key.                              |