Add devel@edk2.groups.io.
> -----Original Message-----
> From: Yao, Jiewen <jiewen....@intel.com>
> Sent: Friday, February 7, 2020 6:13 AM
> To: brbar...@microsoft.com; r...@edk2.groups.io
> Cc: edk2-de...@lists.01.org
> Subject: RE: [edk2-rfc] [edk2-devel] [RFC] VariablePolicy - Protocol,
> Libraries,
> and Implementation for VariableLock Alternative
>
> Thanks, Bret.
>
> Comments below.
>
> > -----Original Message-----
> > From: brbarkel via [] <brbarkel=microsoft.com@[]>
> > Sent: Friday, February 7, 2020 5:49 AM
> > To: Yao; Yao, Jiewen <jiewen....@intel.com>; r...@edk2.groups.io
> > Subject: Re: [edk2-rfc] [edk2-devel] [RFC] VariablePolicy - Protocol,
> > Libraries,
> > and Implementation for VariableLock Alternative
> >
> > Sorry, Jiewen. I missed this email when it came in...
> >
> > > 1. We have 2 variable related protocol - EDKII_VARIABLE_LOCK_PROTOCOL
> > and EDKII_VAR_CHECK_PROTOCOL. Do you want to deprecate both? Or only
> > deprecate EDKII_VARIABLE_LOCK_PROTOCOL?
> >
> > I think we would recommend deprecating both. Is there much consumption of
> > the VarCheck protocol? A platform could always opt to include both (or all
> > 3!),
> > but they wouldn't be able to immediately tell which engine rejected the
> > SetVariable.
>
> [Jiewen] Right. That is my understanding- deprecate both.
> I saw you only mention to deprecate VarLock, but not VarCheck. That is why I
> get confused.
>
> I don’t recommend we have 3 engines. Too burden to maintain the code.
>
> One possible option is to implement VarLockProtocol and VarCheckProtocol on
> top of VarPolicyProtocol. (like a thunk)
> As such, there is no impact on the existing private platform code, with the
> benefit that one central engine controls everything.
> Later, we can check platform one by one. After the last one is migrated, we
> can
> remove VarLock and VarCheck thunk.
>
>
>
>
> > > 2. The Function - DumpVariablePolicy() - makes me confused in the
> beginning.
> > In my thought, "Dump" means to show some debug message. But here, you
> > want to return the policy. Can we change the name to GetVariablePolicy()?
> >
> > I see your point about possible confusion. I think we would try to clarify
> > this in
> > the function documentation. Due to the code-first approach we've taken on
> this,
> > we already have partners who have implemented the policy with
> > DumpVariablePolicy as the name and it doesn't seem like a huge problem.
> >
> > > 3. The function - DisableVariablePolicy(). Does it disable current policy
> engine?
> > Does it disable any future policy engine? Does it block
> > RegisterVariablePolicy()
> > call?
> >
> > Disable turns off the enforcement. You should still be able to register new
> > policies for auditing purposes (they will still be returned by
> DumpVariablePolicy),
> > but no SetVariables will be rejected.
> >
> > > 4. The function - LockVariablePolicy() - Can it lock the
> > > DisableVariablePolicy()
> > call?
> >
> > Correct. Once the interface is locked, it cannot be disabled. Locking
> > should be
> > performed at EndOfDxe or ReadyToBoot or wherever the platform TCB is.
> >
> > > 5. The use case "In MFG Mode Variable Policy Engine is disabled, thus
> > > these
> > VPD variables can be created. These variables are locked with lock policy
> > type
> > LockNow, so that these variables can't be tampered with in Customer Mode."
> >
> > Correct. The MfgMode is just a suggestion and is entirely up to the
> > platform.
> Our
> > MfgMode (which has not been open-sourced yet, but we're working on it) will
> > call DisableVariablePolicy because our threat model indicates that
> compromising
> > MfgMode is a total compromise (there are many other attack vectors once
> > MfgMode is compromised). As such, we protect it extensively. But there is
> > nothing in the core or extra code that would automatically disable the
> > policy
> > engine for any platform that didn't want that. It is up to the platform,
> > however,
> > to make sure they lock the policy engine (similar to locking SMM).
>
> [Jiewen] OK, if my understanding is correct, the DisablePolicy() should only
> be
> called in some special environment, but NOT a normal feature involved in the
> normal boot.
>
> Then I am feeling we should separate the DisablePolicy from this protocol,
> because this interface is very dangerous. I don’t want any driver call it by
> mistake.
>
> Can we split the VARIABLE_POLICY to VARIABLE_POLICY (normal boot usage) +
> VARIABLE_POLICY_CONTROL (very special usage)
>
> Like below:
>
> typedef struct {
> UINT64 Revision;
> REGISTER_VARIABLE_POLICY RegisterVariablePolicy;
> DUMP_VARIABLE_POLICY DumpVariablePolicy;
> LOCK_VARIABLE_POLICY LockVariablePolicy;
> } _VARIABLE_POLICY_PROTOCOL;
>
> typedef struct {
> UINT64 Revision;
> DISABLE_VARIABLE_POLICY DisableVariablePolicy;
> IS_VARIABLE_POLICY_ENABLED IsVariablePolicyEnabled;
> } _VARIABLE_POLICY_CONTROL_PROTOCOL;
>
> VARIABLE_POLICY_PROTOCOL is always installed.
>
> In normal boot, the VARIABLE_POLICY_CONTROL_PROTOCOL is NOT installed.
> As such, the policy is always enforced.
> The VARIABLE_POLICY_CONTROL_PROTOCOL is only installed in some special
> mode, controlled by PCD - maybe.
> A platform may choose to:
> 1) Use static PCD to always disable VARIABLE_POLICY_CONTROL_PROTOCOL
> installation.
> 2) Use dynamic PCD to control VARIABLE_POLICY_CONTROL_PROTOCOL
> installation only in special mode.
>
>
> Thank you
> Yao Jiewen
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#53880): https://edk2.groups.io/g/devel/message/53880
Mute This Topic: https://groups.io/mt/71035502/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
