On 13/12/2019 22:04, Julius Werner via TF-A wrote:
> On Fri, Dec 13, 2019 at 6:20 AM Joanna Farley <Joanna.Farley(a)arm.com> wrote:
>> On the subject of DebugFS's purpose it was envisages and is today as Sandrine describes as a debug build only capability. Saying that though there has been some early thoughts that it could evolve into a Secure Debug feature where this type of capability or something like it is always on requiring debug certificates for authenticated access. This is something very much for a possible future evolution and is not in the patches available today. We would welcome any thoughts on such an evolution in this space.
>
> I guess this gets into a bit of a philosophy discussion and becomes a
> matter of opinion, so there's probably no one right answer.
> Personally, adding authentication on top of this doesn't really
> resolve my concerns and adds yet more on top. I'm a strong proponent
> of the concept of a minimal Trusted Computing Base, i.e. keeping the
> amount of code executing at the highest privilege level as small and
> low-complexity as possible. Any code can have bugs, so the idea is
> that the more complicated the code you run in EL3 is (and the more
> complicated APIs it exposes), the more likely it becomes that you
> accidentally have an exploitable vulnerability in there. Like a p9
> filesystem driver, a certificate-based authentication system
> (especially if it's based on x509/ASN.1 which are notoriously hard to
> implement safely) is a pretty complex piece of code with a pretty
> large attack surface that I'd rather not have in my EL3 firmware if I
> can avoid it. I understand that for certain use cases you may need
> something like this (if you really want a very extensive and
> extensible debugging API that must be restricted to a few
> authenticated actors), but in my use case I really just need the
> ability to trigger one small debugging feature and that feature itself
> doesn't need to be restricted, so a minimal SMC interface would work
> much better for that case.
Hi Julius,
Just to trying to understand, if TF-A were to expose a crash inducing
SMC, this would still be restricted to special builds for your test runs
? This would not make it to production for Chromebook right ?
I agree 9p filesystem is not desirable in a EL3 runtime firmware. We
could enhance it to use a more tight data structure, if there is a
desire in that direction.
If that is the case, leaving aside the 9p filesystem issues, can
DebugFS serve this requirement (we can remove the limitation that it is
restricted to only Debug builds) ?
The intention that DebugFS can prove useful atleast in the
verification/testing space and if there is more we can do to get there,
it would be good to know.
>
>> On 13/12/2019, 13:01, "TF-A on behalf of Sandrine Bailleux via TF-A" <tf-a-bounces(a)lists.trustedfirmware.org on behalf of tf-a(a)lists.trustedfirmware.org> wrote:
>> Going back to the SMC-based solution then, I am not quite convinced
>> SYSTEM_RESET2 is the right interface for intentionally triggering a
>> panic in TF-A. I think the semantics do not quite match. If anything, a
>> firmware crash seems more like a shutdown operation to me rather than a
>> reset (we don't recover from a firmware crash). I am not even sure we
>> should look into the PSCI SMC range, as it's not a power-management
>> operation.
>
> Crash recovery behavior is platform dependent (via
> plat_panic_handler()). On all the platforms we use in Chrome OS we
> have that implemented as a system reboot. I think for most systems
> (whether it's a Chromebook, a server or some embedded device) that's
> probably what you want for random runtime crashes (and least in a
> production environment), but I agree that TF doesn't enforce any
> standard behavior so it's hard to clearly match it to one or the other
> SMC.
>
>> So it sounds like it's not the first time that you hit this issue, is
>> it? Do you have any other example of Normal World OS feature you would
>> have liked to expose through a generic SMC interface? I am wondering
>> whether this could help choosing the right SMC range, if we can identify
>> some common criteria among a set of such features.
>
> No, it's the first time I've really run into this. But I think we
> might quickly come up with more uses for a "non-secure OS" SMC range
> if we had one. We often see roughly the same SMC again on different
> platforms, because fundamentally they usually need to do the same
> kinds of things -- for example, most platforms have some kind of DDR
> frequency scaling which always needs part of it implemented in EL3, so
> they all need some kind of SMC to switch to a new DDR frequency. Many
> also need some kind of "write value to secure register" SMC that just
> allows the non-secure OS to write a few whitelisted registers that are
> only accessible in EL3 for some reason. If we could standardize these
> interfaces in a non-vendor-specific SMC range, we might be able to
> reduce some code duplication both on the TF and the Linux side.
>
> I guess none of these things are really Linux-specific, now that I
> think of it. So really, I guess the problem is that it would be great
> to have a range of "generic" SMC IDs that can be easily and
> unbureaucratically allocated to try out new features, without having
> to ask Arm to write a big specification document about it every time.
> It's sort of a development velocity issue.
>
We have utilized the ARM SiP range for some "generic" purposes in the
past (see PMF and the execution state switch SMCs). This could be
direction for the some of use-cases. But if the SMCs are meant to be
truly generic and to be relied on for use by generic normal world
software components, it would need to be properly specified I would think.
For dynamically modifying some EL3 registers, it would be good to get
these requirements out. Perhaps there is scope for architecting some of
them as an ARM specification. If not, we could revert to a TF-A standard
if there is enough pull for them (perhaps utilizing the ARM SiP range).
Best Regards
Soby Mathew
Hi Sumit,
I've started providing some early review comments on your patches in
Gerrit but these are mainly about implementation details. I would like
to continue the higher-level design discussions here in parallel on the
mailing list, if that's fine with you.
On 12/6/19 1:29 PM, Sumit Garg via TF-A wrote:
> Hi Sandrine,
>
> On Thu, 5 Dec 2019 at 20:31, Sandrine Bailleux
> <Sandrine.Bailleux(a)arm.com> wrote:
>>>> Also, I am still trying to get my head around how this would integrate
>>>> with a cryptographic engine where the key does not leave the chip. I can
>>>> imagine that we could get the address of the encrypted firmware image
>>>> from the FIP, pass that to a cryptographic engine, request it to decrypt
>>>> it and store the result somewhere in Trusted RAM. In this case, we
>>>> wouldn't call plat_get_fip_encryption_key(). Do you have any idea how we
>>>> would pull this off? Like how the different modules (IO layer, crypto
>>>> module, image parser module, ...) would integrate together?
>>>
>>> In this case, I would expect platform to provide key identifier rather
>>> than actual key as part of plat_get_fip_encryption_key() which is then
>>> passed onto auth_decrypt() that is implementation specific to
>>> cryptographic engine in similar terms as currently done for mbedTLS
>>> backend.
>>
>> Ah I see, so plat_get_fip_encryption_key() could return either the key
>> itself or a key identifier. Just like plat_get_rotpk_info() can return
>> either the key or a hash of it today. However, in the case of
>> plat_get_rotpk_info(), it also returns some flags indicating which one
>> it is. Don't we need something similar for
>> plat_get_fip_encryption_key()? How will the crypto module be able to
>> tell the difference between a key and a key ID otherwise? Or would you
>> expect a given crypto module backend to always use either keys or key
>> IDs, but not both?
>>
>
> It perfectly makes sense for crypto module backend to support both key
> ID and actual key. But I was thinking from use-case perspective that
> why would one like to expose key to the firmware if the crypto module
> could better protect it. And I could come up with one use-case where
> the SSK (burnt in SoC fuses) is provided as an actual key and BSSK
> (accessible only to crypto module) is provided via a key ID.
Yes, that makes sense to me.
> So yes I agree with you to provide additional flags field along with
> key buffer. Also, I think it makes sense to rename
> "plat_get_fip_encryption_key()" to "plat_get_fip_enc_key_info()".
Agree.
>>>> I have some concerns around the generation of the initialization vectors
>>>> in the encrypt_fw tool. Right now, IVs are simply a random sequence of
>>>> bytes (obtained through a call to OpenSSL's RAND_bytes() API). Now, I
>>>> would imagine that RAND_bytes() is typically based on a good random
>>>> number generator and thus will generate different sequences every time
>>>> it is called. At least, as long as it is called from the same machine
>>>> every time. But what if we encrypt a new FIP bundle from a different
>>>> machine, say in the context of a firmware update? Is it not possible
>>>> that it might choose the same IV out of bad luck?
>>>>
>>>> Perhaps that's an issue left to provisioning/manufacturing time and is
>>>> out of the scope here. But it worries me because AFAIU, the security of
>>>> AES-GCM is critically undermined if the same nonce is used multiple
>>>> times with the same key (see section 5.1.1. "Nonce reuse" in RFC 5116).
>>>> If the encryption key is the SSK (rather than the BSSK) then I guess the
>>>> probability is even higher, as it is shared amongst a class of devices.
>>>>
>>>
>>> Agree that "nonce" should be unique and using a random number
>>> generator available on build machine was an effort towards that. But
>>> thinking about the case that you have mentioned, I think we could have
>>> an optional user provided "nonce" as an input to "encrypt_fw" tool, so
>>> that the user is aware to randomly generate and provide a unique
>>> nonce.
>>
>> Yes, I like your suggestion of specifying the nonce to the tool. But I
>> think it should be the default behaviour then. You mention an *optional*
>> user-provided nonce, I would like to suggest we make this mandatory
>> instead. We could provide an option to request the tool to generate the
>> nonce, intended for development purposes.
>>
>
> Making nonce as default and mandatory command line option seems to be
> overkill for a user. It may also lead to unsafe practices of
> hardcoding a nonce in Makefile, shell scripts etc. So I think we
> should keep this as optional only for a narrow use-case where people
> might not trust uniqueness property of RNG from multiple build
> machines.
I don't know... Hard-coding a nonce in the makefile is unsafe for sure
but I would think that a nonce generated by the tool is also unsafe
without proper management of these nonces across keys and devices, but
in a much more subtle manner. I still think the nonce generation would
be better left outside of the scope of this tool.
> BTW, I have already added nonce command line option as part of v3
> patch-set [1]. Please have a look.
>
> [1] https://review.trustedfirmware.org/c/TF-A/trusted-firmware-a/+/2496/2..3
>
>>>> Impact on memory footprint and performance
>>>> ------------------------------------------
>>>>
>>>> Do you know what the performance impact is when this feature is enabled
>>>> in TF-A, to decrypt images at boot time? Obviously it depends on the
>>>> platform and whether there is a dedicated cryptographic engine, and I
>>>> suppose you cannot really get any relevant measurements out of QEMU but
>>>> I would be interested if you've got any rough numbers.
>>>
>>> Following are measurements based on qemu for mbedTLS software library
>>> based authenticated decryption:
>>>
>>> BL31 plain:
>>> NOTICE: Load image time: 137us, size: 28KB
>>> BL31 encrypted:
>>> NOTICE: Load image time: 3979us, size: 28KB
>>>
>>> BL32 plain:
>>> NOTICE: Load image time: 1791us, size: 360KB
>>> BL32 encrypted:
>>> NOTICE: Load image time: 36339us, size: 360KB
>>
>> Thanks. So it's a 29% increase for BL31 and 20% for BL32. I would have
>> naively expected similar percentages, any idea why we get such a
>> difference between BL31 and BL32? I am just curious. Maybe it's down to
>> how the crypto algorithm/mode of operation works under the hood?
>
> Yes this additional increase for BL31 seems to be due to one time
> initialization operations carried on first invocation of mbedTLS gcm
> APIs, especially time taken to generate AES tables (see call to
> aes_gen_tables() in mbedtls/library/aes.c +576).
Ah I see, thanks for the explanation.
Regarding the memory impact, given that it's adding quite a bit of code
and data (around 9KB for BL1 and 12KB for BL2 from what you were saying
in a previous email), I think we need to make this feature optional and
wrap it under a build flag. Not everyone will need to encrypt firmware
images, having only authentication is a valid use case as well. If I am
not mistaken, your patches right now unconditionally pull the mbedTLS
primitives into BL1 and BL2 (as long as TBBR is enabled of course).
As a matter of fact, I am not able to build BL1 on FVP platform with
your patches, presumably due to the unconditional size increase. The
following build command:
> make TRUSTED_BOARD_BOOT=1 ARM_ROTPK_LOCATION=devel_rsa
MBEDTLS_DIR=mbedtls bl1
... fails at link time:
aarch64-linux-gnu-ld.bfd: build/fvp/release/bl1/bl1.elf section
`xlat_table' will not fit in region `RAM'
aarch64-linux-gnu-ld.bfd: BL1's RW section has exceeded its limit.
aarch64-linux-gnu-ld.bfd: region `RAM' overflowed by 4096 bytes
Makefile:889: recipe for target 'build/fvp/release/bl1/bl1.elf' failed
Regards,
Sandrine
Hi Sheetal,
Your remark/question makes sense.
The original requirement is described here: https://github.com/ARM-software/tf-issues/issues/232
We may get further update from Dan, but I sense this was to provision for intermediate log levels (as stated by Sandrine on phabricator).
Will you be able to submit a patch for this?
Regards,
Olivier.
________________________________________
From: TF-A <tf-a-bounces(a)lists.trustedfirmware.org> on behalf of Sheetal Tigadoli via TF-A <tf-a(a)lists.trustedfirmware.org>
Sent: 20 December 2019 15:48
To: tf-a(a)lists.trustedfirmware.org
Subject: [TF-A] [Question] Why are the log related macros defined in multiple of 10's
Hi All,
I wanted to understand the reason behind defining LOG_LEVEL_* macros as
multiple of 10's, also same goes with LOG_MARKER_* macros.
If we set the LOG macros to 1,2,3 and so on, we can avoid the mod and
division operation when printing the msgs to console.
Please let me your thoughts on this.
Link to this question on Phabricator -
https://developer.trustedfirmware.org/T624
Thanks
Sheetal
--
TF-A mailing list
TF-A(a)lists.trustedfirmware.org
https://lists.trustedfirmware.org/mailman/listinfo/tf-a
Hi All,
I wanted to understand the reason behind defining LOG_LEVEL_* macros as
multiple of 10's, also same goes with LOG_MARKER_* macros.
If we set the LOG macros to 1,2,3 and so on, we can avoid the mod and
division operation when printing the msgs to console.
Please let me your thoughts on this.
Link to this question on Phabricator -
https://developer.trustedfirmware.org/T624
Thanks
Sheetal
Hi All,
I wanted to understand the reason behind defining LOG_LEVEL_* macros as
multiple of 10's, also same goes with LOG_MARKER_* macros.
If we set the LOG macros to 1,2,3 and so on, we can avoid the mod and
division operation when printing the msgs to console.
Please let me your thoughts on this.
Link to this question on Phabricator -
https://developer.trustedfirmware.org/T624
Thanks
Sheetal
>
> Subject: [PATCH] rpi3/4: Add support for offlining CPUs
> From: Jan Kiszka <jan.kiszka(a)siemens.com>
>
> The hooks were populated but the power down left the CPU in limbo-land.
> What we need to do - until there is a way to actually power off - is to
> turn off the MMU and enter the spinning loop as if we were cold-booted.
> This allows the on-call to pick up the CPU again.
>
> Signed-off-by: Jan Kiszka <jan.kiszka(a)siemens.com>
> ---
> plat/rpi/common/rpi3_pm.c | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/plat/rpi/common/rpi3_pm.c b/plat/rpi/common/rpi3_pm.c
> index 8c2d070c4..2a6bf076b 100644
> --- a/plat/rpi/common/rpi3_pm.c
> +++ b/plat/rpi/common/rpi3_pm.c
> @@ -123,6 +123,15 @@ static void rpi3_pwr_domain_off(const psci_power_state_t *target_state)
> #endif
> }
>
> +void __dead2 plat_secondary_cold_boot_setup(void);
> +
> +static void __dead2
> +rpi3_pwr_domain_pwr_down_wfi(const psci_power_state_t *target_state)
> +{
> + disable_mmu_el3();
> + plat_secondary_cold_boot_setup();
> +}
> +
> /*******************************************************************************
> * Platform handler called when a power domain is about to be turned on. The
> * mpidr determines the CPU to be turned on.
> @@ -224,6 +233,7 @@ static void __dead2 rpi3_system_off(void)
> static const plat_psci_ops_t plat_rpi3_psci_pm_ops = {
> .cpu_standby = rpi3_cpu_standby,
> .pwr_domain_off = rpi3_pwr_domain_off,
> + .pwr_domain_pwr_down_wfi = rpi3_pwr_domain_pwr_down_wfi,
> .pwr_domain_on = rpi3_pwr_domain_on,
> .pwr_domain_on_finish = rpi3_pwr_domain_on_finish,
> .system_off = rpi3_system_off,
> --
> 2.16.4
>
Hi Jan,
I have put in a suggestion for the change. Basically, if you are able to
do a `reset` before spinning in `plat_secondary_cold_boot_setup()`, that
would be ideal. Also, if `plat_secondary_cold_boot_setup()` can trap the
primary CPU as well if it were to be offlined, then I think your changes
should be good.
Best Regards
Soby Mathew
Hi Jisheng,
This is due to the power management requirement of GICv3. In section 7.1 of GICv3 specification : https://static.docs.arm.com/ihi0069/c/IHI0069C_gic_architecture_specificati…
It says the that " Before powering down the CPU interface and the PE when the Redistributor is powered up, software must put the interface between the CPU interface and the Redistributor into the quiescent state or the system will become UNPREDICTABLE".
In order to put the Redistributor in a quiescent state, all the group enables in CPU interface must be disabled , otherwise "setting GICR_WAKER.ProcessorSleep to 1 when the physical group enables in the CPU interface are set to 1 results in UNPREDICTABLE behavior."
Hope that clarifies.
Best Regards
Soby Mathew
> -----Original Message-----
> From: TF-A <tf-a-bounces(a)lists.trustedfirmware.org> On Behalf Of Jisheng
> Zhang via TF-A
> Sent: 05 December 2019 10:31
> To: tf-a(a)lists.trustedfirmware.org
> Subject: [TF-A] About disable gic cpuif in psci suspend or off
>
> Hi,
>
> In ATF-A, I usually see below code in psci suspend or off code path:
>
> /* Prevent interrupts from spuriously waking up this cpu */
> plat_arm_gic_cpuif_disable();
>
> But per my understanding, before calling psci_suspend(), the NW, e.g linux
> kernel has disabled all interrupts from cpu level, so here preventing interrupt is
> to prevent the interrupts from secure world?
>
> Another question is: for Cortex A55, this is not necessary. Because
> CA55 TRM says when the core_pwrdn_en bit is set, executing WFI
> automatically masks all interrupts and wake-up events in the core. Am I right?
>
> Thanks in advance
> --
> TF-A mailing list
> TF-A(a)lists.trustedfirmware.org
> https://lists.trustedfirmware.org/mailman/listinfo/tf-a
Julius,
On the subject of DebugFS's purpose it was envisages and is today as Sandrine describes as a debug build only capability. Saying that though there has been some early thoughts that it could evolve into a Secure Debug feature where this type of capability or something like it is always on requiring debug certificates for authenticated access. This is something very much for a possible future evolution and is not in the patches available today. We would welcome any thoughts on such an evolution in this space.
Joanna
On 13/12/2019, 13:01, "TF-A on behalf of Sandrine Bailleux via TF-A" <tf-a-bounces(a)lists.trustedfirmware.org on behalf of tf-a(a)lists.trustedfirmware.org> wrote:
Hi Julius,
OK, in that case I can see that a solution based on TF-A's DebugFS
interface might not be desirable. Indeed, our original intention was to
make the whole DebugFS system a debug-only feature (hence its name!). As
such, I agree that it is likely not to get the same level of scrutiny
and testing as other features intended for production systems.
One of the main use cases we have in mind for DebugFS is, being able to
peek and poke into the firmware for testing purposes. Today, when doing
functional testing from the normal world (for example, using TF-A
Tests), we are limited to what's exposed through the SMC interface. And
even then, we have limited visibility on what really happened in the
firmware, as we can only deduce so much from the SMC return value(s).
DebugFS could be used to bridge this gap, by providing a side channel
for getting internal firmware state information.
Going back to the SMC-based solution then, I am not quite convinced
SYSTEM_RESET2 is the right interface for intentionally triggering a
panic in TF-A. I think the semantics do not quite match. If anything, a
firmware crash seems more like a shutdown operation to me rather than a
reset (we don't recover from a firmware crash). I am not even sure we
should look into the PSCI SMC range, as it's not a power-management
operation.
Julius, you wrote:
> It's the same problem that the SMC/PSCI spec and the TF repository layout is only designed to deal with generic vs. SoC-vendor-specific differentiation. If the normal world OS needs a feature, we can only make it generic or duplicate it across all vendors running that OS.
So it sounds like it's not the first time that you hit this issue, is
it? Do you have any other example of Normal World OS feature you would
have liked to expose through a generic SMC interface? I am wondering
whether this could help choosing the right SMC range, if we can identify
some common criteria among a set of such features.
Regards,
Sandrine
--
TF-A mailing list
TF-A(a)lists.trustedfirmware.org
https://lists.trustedfirmware.org/mailman/listinfo/tf-a
Hi Louis,
The term PAL is also used for platform abstraction layer so it "could" be confusing to have this being used for something different.
Not necessarily suggesting to change anything but it might be worth considering this aspect.
Thanks,
Abhishek
-----Original Message-----
From: TF-A <tf-a-bounces(a)lists.trustedfirmware.org> On Behalf Of Louis Mayencourt via TF-A
Sent: 19 November 2019 13:40
To: tf-a(a)lists.trustedfirmware.org
Subject: [TF-A] Design proposal: Property Access Layer
Hi everyone,
Please let me introduce the `Property Access Layer` prototype:
The Property Access Layer (PAL) is an abstraction layer for platform specific data, allowing a "property" to be queried and a value retrieved without the requesting entity knowing what backing store is being used to hold the data. It is used to bridge new and old ways of providing platform-specific data:
Today, information like the Chain of Trust is held within several, nested platform-defined tables. In the future, it may be provided as part of a device tree blob, along with the information about images to load.
Introducing this abstraction layer will make migration easier and will preserve functionality for platforms that cannot / don't want to use device tree.
Please have a look at the patches: https://review.trustedfirmware.org/c/TF-A/trusted-firmware-a/+/2559/1
Regards,
Louis
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
--
TF-A mailing list
TF-A(a)lists.trustedfirmware.org
https://lists.trustedfirmware.org/mailman/listinfo/tf-a