[U-Boot] [RFC PATCH] fastboot: Implement fetching uboot env variables via fastboot getvar

[Sam Protsenko]

 Hi Priit,

On Wed, Jul 10, 2019 at 11:42 AM Eugeniu Rosca <erosca at de.adit-jv.com> wrote:
>
> Hi Priit,
>
> On Tue, Jul 09, 2019 at 02:52:56PM +0300, Priit Laes wrote:
> > From: Priit Laes <priit.laes at paf.com>
> >
> > Add u-boot specific getvar "extension" to fetch u-boot environment
> > variables via `fastboot getvar uboot:var`. This makes it possible
> > to gather certain data (like mac addresses) in an automated way
> > during initial fastboot flashing for inventory purposes:
> >
> > $ fastboot getvar uboot:ethaddr
> > uboot:ethaddr: 12:23:45:56:78:90
> >
> > Output is currently truncated at 64 bytes, but this is good enough
> > for my own requirements.
> >
> > Signed-off-by: Priit Laes <priit.laes at paf.com>
> > ---
> >  drivers/fastboot/fb_getvar.c | 14 ++++++++++++++
>
> [..]
>
> > +static void getvar_ubootenv(char *var_parameter, char *response)
> > +{
> > +     const char *value = env_get(var_parameter);
>
> This would bring a lot of flexibility to the users. My only concern is
> that it exposes to the outside world an internal U-Boot API (env_get)
> which might have weaknesses (or might acquire them in time). I am not
> sure how env_get() behaves in below corner cases:
>  - NULL pointer
>  - empty string
>  - hugely long string
>
> IMHO the internal APIs are usually not designed to sustain high level of
> stress/fuzziness in their input parameters. Once made available to the
> users of fastboot tool, this will open room for more experiments to the
> CVE seekers.
>
> Another observation is that this patch will contribute with a deviation
> from the vanilla fastboot protocol (which might be fine).
>
> Since there are already multiple getvar functions which fetch the
> information from the U-Boot environment, I wonder if all of these could
> be centralized in a table like below:
>
>  {
>         /* fastboot    U-Boot */
>         { "serialno", "serial#" },
>         { "product",  "board" },
>         { "platform", "platform" },
>         { "ethaddr",  "ethaddr" },
>         { "anything", "anything" },
>  }
>
> The upsides of this approach:
>  - Unification, readability, decreased code size
> The downsides:
>  - Inflexible for getting arbitrary environment variables
>

I agree with Eugeniu on his points about security and deviation from
fastboot spec (can be found in AOSP). Instead of exposing the whole
U-Boot environment, I can suggest you to expose only actually needed
variables for your platform. In fact, we already have a mechanism
exactly for that, called "variable overrides". It's documented in [1].
Basically you just need to add "fastboot.xxx" variables to your
environment (can be don e.g. in your board_late_init() function), and
you'll be able to obtain those via "fastboot getvar xxx". You can see
an example in patches [2,3].

[1] https://gitlab.denx.de/u-boot/u-boot/blob/v2019.07/doc/README.android-fastboot#L90
[2] https://gitlab.denx.de/u-boot/u-boot/commit/fa24eca1f20a037d2dcbd1eae7ac8b2ecb1b0423
[3] https://gitlab.denx.de/u-boot/u-boot/commit/8bd29623b5223e880e7be475243a2bdb987aba38

> --
> Best Regards,
> Eugeniu.