refactor(console): disable getc() by default

The ability to read a character from the console constitutes an attack
vector into TF-A, as it gives attackers a means to inject arbitrary
data into TF-A. It is dangerous to keep that feature enabled if not
strictly necessary, especially in production firmware builds.

Thus, we need a way to disable this feature. Moreover, when it is
disabled, all related code should be eliminated from the firmware
binaries, such that no remnant/dead getc() code remains in memory,
which could otherwise be used as a gadget as part of a bigger security
attack.

This patch disables getc() feature by default. For legitimate getc()
use cases [1], it can be explicitly enabled by building TF-A with
ENABLE_CONSOLE_GETC=1.

The following changes are introduced when getc() is disabled:

- The multi-console framework no longer provides the console_getc()
  function.

- If the console driver selected by the platform attempts to register
  a getc() callback into the multi-console framework then TF-A will
  now fail to build.

  If registered through the assembly function finish_console_register():
  - On AArch64, you'll get:
      Error: undefined symbol CONSOLE_T_GETC used as an immediate value.
  - On AArch32, you'll get:
      Error: internal_relocation (type: OFFSET_IMM) not fixed up

  If registered through the C function console_register(), this requires
  populating a struct console with a getc field, which will trigger:
    error: 'console_t' {aka 'struct console'} has no member named 'getc'

- All console drivers which previously registered a getc() callback
  have been modified to do so only when ENABLE_CONSOLE_GETC=1.

[1] Example of such use cases would be:
    - Firmware recovery: retrieving a golden BL2 image over the console in
      order to repair a broken firmware on a bricked board.
    - Factory CLI tool: Drive some soak tests through the console.

Discussed on TF-A mailing list here:
https://lists.trustedfirmware.org/archives/list/tf-a@lists.trustedfirmware.org/thread/YS7F6RCNTWBTEOBLAXIRTXWIOYINVRW7/

Change-Id: Icb412304cd23dbdd7662df7cf8992267b7975cc5
Signed-off-by: Sandrine Bailleux <sandrine.bailleux@arm.com>
Acked-by: Baruch Siach <baruch@tkos.co.il>

2 files changed, 16 insertions, 0 deletions

diff --git a/docs/getting_started/build-options.rst b/docs/getting_started/build-options.rst
index 34d83f255..c045a6aa2 100644
--- a/docs/getting_started/build-options.rst
+++ b/docs/getting_started/build-options.rst
@@ -1191,6 +1191,12 @@ Common build options
   per the `PSA Crypto API specification`_. This feature is only supported if
   using MbedTLS 3.x version. By default it is disabled (``0``).
 
+- ``ENABLE_CONSOLE_GETC``: Boolean option to enable `getc()` feature in console
+  driver(s). By default it is disabled (``0``) because it constitutes an attack
+  vector into TF-A by potentially allowing an attacker to inject arbitrary data.
+  This option should only be enabled on a need basis if there is a use case for
+  reading characters from the console.
+
 GICv3 driver options
 --------------------
 
diff --git a/docs/process/security-hardening.rst b/docs/process/security-hardening.rst
index f9618db08..eace467d4 100644
--- a/docs/process/security-hardening.rst
+++ b/docs/process/security-hardening.rst
@@ -135,6 +135,16 @@ Several build options can be used to check for security issues. Refer to the
   it is recommended to develop against ``W=2`` (which will eventually become the
   default).
 
+Additional guidelines are provided below for some security-related build
+options:
+
+- The ``ENABLE_CONSOLE_GETC`` build flag should be set to 0 to disable the
+  `getc()` feature, which allows the firmware to read characters from the
+  console. Keeping this feature enabled is considered dangerous from a security
+  point of view because it potentially allows an attacker to inject arbitrary
+  data into the firmware. It should only be enabled on a need basis if there is
+  a use case for it, for example in a testing or factory environment.
+
 .. rubric:: References
 
 -  `Arm ARM`_