UpstreamDataInc / goosebit

A simplistic, opinionated remote update server implementing hawkBit™'s DDI API.
https://goosebit.rtfd.io
Apache License 2.0
14 stars 3 forks source link

Encrypted sw-description not supported #144

Open easybe opened 1 week ago

easybe commented 1 week ago

See: https://github.com/UpstreamDataInc/goosebit/pull/109#issuecomment-2380580819

easybe commented 6 days ago

I guess the simplest solution to this problem is to offer the possibility to manually enter the version and such.

tsagadar commented 5 days ago

Likely then also requires the ability to view and modify "Hardware" (ie. combinations of hw_model and hw_revision). Makes it more complex and information becomes redundant (somehow embedded in the image and manually entered in gooseBit).

So yes: could cover this use case and the RAUC use case - but I'd propose to delay this implementation and first cover more central parts.

b-rowan commented 4 days ago

I guess the simplest solution to this problem is to offer the possibility to manually enter the version and such.

I don't think it would be much harder to either add the encryption key as a setting and allow users to specify if a firmware is encrypted when uploading, or allow users to specify the encryption key when uploading the software.

I am preferential to making it an ENV variable/setting, as we have no way to distinguish between devices that want an encrypted image and devices that want a normal image.

My biggest problem here is I don't actually understand how the images are encrypted and decrypted...

sbabic commented 4 days ago

I agree that there should be the possibility to add manually the version. Decrypting sw-description is risky because the key should be given and stored somewhere on the server. And encryption is not only used for sw-description, but even for artifacts to protect IP property. When sw-description is encrypted, or version cannot be read (like the issue when version cannot be parsed as semantic version), user should have a way to introduce the version.

b-rowan commented 4 days ago

Decrypting sw-description is risky because the key should be given and stored somewhere on the server. And encryption is not only used for sw-description, but even for artifacts to protect IP property.

I don't think this is a big deal, considering it is an open source project. We have no tracking or anything on it, and it must be self hosted. This means nothing about that key ever leaves the user's server, thereby protecting IP.

I agree that there should be the possibility to add manually the version.

I don't think this is a bad idea, but it ends up raising sort of the same problem, how do we have a nice UI for setting the version that conforms to semantic versioning properly so the firmware updates can be ordered? Do we allow the user to pass None for the version and then attempt to parse the SWU? What happens if that fails?

I think this also can make some workflows more complex, because the user will have to send the version via the API, instead of just in one place...

jameshilliard commented 4 days ago

Decrypting sw-description is risky because the key should be given and stored somewhere on the server.

AFAIU it's probably not in practice much of an additional risk since the devices actually downloading/installing the actual update images are also required to have access to the decryption key in some form unlike the singing keys.

Maybe the swu format can be modified in a compatible way to have an unencrypted sw-description.meta file appended after sw-description or sw-description.sig(when signature validation is enabled) when sw-description encryption is enabled and which contains only compatibility and version metadata needed by goosebit but which would not contain potentially sensitive info like embedded lua scripts. Presumably swugenerator could simly generate the meta file at the same time the encrypted sw-description is generated.

sbabic commented 4 days ago

AFAIU it's probably not in practice much of an additional risk since the devices actually downloading/installing the actual update images are also required to have access to the decryption key in some form unlike the singing keys.

Devices must store of course the key, but many devices have a safe box (included in the SOC or with a TPM/HSM). Rootfs can be encrypted, too, and this can be done using crypto coprocessors that don't expose the key. And device should be hardened and access to the device is ruled, in most cases access is just guaranteed by the application without a direct Linux access (shell).

On the other side, goosebit runs on a server exposed to public network, and it is also thinkable that it accepts devices from different products, maybe different vendors, and then many different keys, increasing the risks.

Maybe the swu format can be modified in a compatible way to have an unencrypted sw-description.meta file appended after sw-description or sw-description.sig(when signature validation is enabled) when sw-description encryption is enabled and which contains only compatibility and version metadata needed by goosebit but which would not contain potentially sensitive info like embedded lua scripts.

We do not need to change the format for this. SWUpdate ignores files that are not described in sw-description, then a .meta file has no influence in the update and it is simply skipped. However, there is a not solved issue for the compatibility. Version is stored just once, but it is poossible to have multiple targets inside the same sw-description:

                     target-1 {
                                  hw-compatibility = .....
                                  images { ....}
                   }
                   target-2 {
                                 hw-compatibility = ....
                 }
jameshilliard commented 4 days ago

Devices must store of course the key, but many devices have a safe box (included in the SOC or with a TPM/HSM). Rootfs can be encrypted, too, and this can be done using crypto coprocessors that don't expose the key.

Yeah, I've reverse engineered some custom firmware update encryption schemes with protections like these, however those protections can often be bypassed in various ways due to implementation flaws. Does swupdate support doing the actual firmware decryption in a way that doesn't require loading the symmetric AES key into the userspace application memory?

We do not need to change the format for this. SWUpdate ignores files that are not described in sw-description, then a .meta file has no influence in the update and it is simply skipped.

Yeah, that's what I was thinking, basically only need to then change the output format for the tools that generate the swu file. AFAIU as long as someone has the AES decryption key they could even make a tool to rebuild swu files(ie on a separate system) with metadata inserted without actually breaking any existing firmware signatures.

However, there is a not solved issue for the compatibility. Version is stored just once, but it is poossible to have multiple targets inside the same sw-description:

So it sounds like we would just need to have swugenerator or whatever tool is being used to generate the swu files also output those nodes to a meta file?

sbabic commented 4 days ago

Yeah, I've reverse engineered some custom firmware update encryption schemes with protections like these, however those protections can often be bypassed in various ways due to implementation flaws. Does swupdate support doing the actual firmware decryption in a way that doesn't require loading the symmetric AES key into the userspace application memory?

At the moment, this is just possible with WolfSSL and pkcs#11 enabled. I have added under improvement_proposals.rst a list of work packages to be done regarding crypto subsystem in SWUpdate, and the goal will be to have pkcs#11 and TPM support in the future (future=when a company will support this work).

So it sounds like we would just need to have swugenerator or whatever tool is being used to generate the swu files also output those nodes to a meta file?

Yes, and the same should be done for OE that does not use swugenerator. meta will remain an ASCII file (maybe still in libconfig format) and won't be ever encrypted.

tsagadar commented 1 day ago

However, there is a not solved issue for the compatibility. Version is stored just once, but it is poossible to have multiple targets inside the same sw-description:

                     target-1 {
                                  hw-compatibility = .....
                                  images { ....}
                   }
                   target-2 {
                                 hw-compatibility = ....
                 }

@sbabic I think I did not understand this comment. When parsing sw-description we support multiple targets. The example you show matches the following test - correct? https://github.com/UpstreamDataInc/goosebit/blob/94060aea6ba14f93dfccbb8f84e26724b5f1a1dc/tests/updates/test_swdesc.py#L72

Happy to support additional cases if you can point them out.