toradex / meta-toradex-security

MIT License
4 stars 9 forks source link

New Encryption additions should not install service if disabled #21

Closed MMG-DG closed 6 months ago

MMG-DG commented 7 months ago

Looking at the changes added recently, it appears the variable TDX_ENC_ENABLE controls if encryption should be enabled. However, it seems that the encryption service is still being installed to the image regardless. The only thing this controls is the kernel config includes.

Should the IMAGE_INSTALL:append also be conditional?

sergioprado commented 7 months ago

@MMG-DG thanks for reporting.

The way the "user interface" for encryption is being designed is that the user will have to just inherit a class and set a few extra variables to have encryption support enabled. Example:

INHERIT += "tdx-encrypted"
TDX_ENC_KEY_BACKEND = "caam"
TDX_ENC_STORAGE_LOCATION = "/dev/mmcblk0p2"

The TDX_ENC_ENABLE variable is just for internal use. If one wants to disable encryption support, it should not inherit the class. Enabling the class and changing some internal variables might cause unexpected results, as this one you are reporting.

Also, be aware that the encryption feature is still in development, and the user interface might change. In a few weeks, we should have the basic support for CAAM merged, including the documentation.

Anyway, I really appreciate your testing reports. Feel free to keep reporting any issues here. Thanks!

MMG-DG commented 7 months ago

How can we then remove this later.

My example is that I have a minimal image which includes the encryption. I also have a development image, which inherits the minimal image and adds extra tools. This development image I don't want encryption being applied.

Is this possible?

sergioprado commented 7 months ago

I think there are many solutions to this, but the simplest solution is to not inherit the class when building the development image.

Another solution is to have two local.conf templates, one for production images and another for development images, and only the local.conf template for production images would inherit the class.

You could also have an override that would be set when building production images. Then you can have something like this in a configuration file, considering you added prod to OVERRIDES when building production images:

INHERIT:append:prod = " tdx-encrypted"

Does any of that solves your problem?

Obs.: I am reopening this issue until we are able to provide you a good solution for your use case.

Adding @rborn-tx and @jsrc27 to the discussion, in case they want to comment on this.

MMG-DG commented 7 months ago

Ooooh..... Thanks, I think I understand now. I have made the change for the OVERRIDES as I think our use-case this would be cleaner. Will run the builds we have and report back with results.

MMG-DG commented 6 months ago

Sorry it has been a while, I forgot to reply here. My new build seems to work great now that I have the split configs.