Machine name in Code update

[Vijay Khemka]

On 9/20/19, 8:55 AM, "Joseph Reynolds" <jrey at linux.ibm.com> wrote:

    
    On 9/19/19 9:21 PM, Lei YU wrote:
    > On Fri, Sep 20, 2019 at 2:15 AM Vijay Khemka <vijaykhemka at fb.com> wrote:
    >> Team,
    >>
    >> I am trying to add a feature of verifying machine name while upgrading BMC image. I have submitted a couple of patches, 1. Which adds machine name in manifest file and 2. Which verifies machine name from manifest to os-release file. Below are 2 gerrit review patch.
    >>
    >> https://urldefense.proofpoint.com/v2/url?u=https-3A__gerrit.openbmc-2Dproject.xyz_-23_c_openbmc_meta-2Dphosphor_-2B_25324_&d=DwIDaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=v9MU0Ki9pWnTXCWwjHPVgpnCR80vXkkcrIaqU7USl5g&m=arMHhrzzBHUMyI_30avg7eppPTw9DIZJoNVYq20Hro0&s=7fog-3ezFneiNbNeFTsBxOq5eciSgB21JCovcFMEJXk&e= 
    >>
    >> https://urldefense.proofpoint.com/v2/url?u=https-3A__gerrit.openbmc-2Dproject.xyz_-23_c_openbmc_phosphor-2Dbmc-2Dcode-2Dmgmt_-2B_25344_&d=DwIDaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=v9MU0Ki9pWnTXCWwjHPVgpnCR80vXkkcrIaqU7USl5g&m=arMHhrzzBHUMyI_30avg7eppPTw9DIZJoNVYq20Hro0&s=Z-RwTQ2KomMzSaMUGqyarcfRf9EfKeQE5CGtfeqMfRg&e= 
    >>
    >>
    >> I have received some valuable feedback on design approach and I agreed with some of them. Let me explain complete thought here and please provide your valuable feedback as well as new ideas.
    >>
    >>
    >>
    >> Currently available:
    >>
    >> ================
    >>
    >> Currently, Software updater updates image based on version reading and purpose from manifest file.
    >>
    >> I find here a security issues of upgrading an image which was built for different machine and upgraded to BMC with different platform.
    >>
    > + @Adriana Kobylak
    >
    > And here is my thought:
    > * Initially I wanted to add such check to make sure a BMC is not updated with
    >    a different BMC build.
    > * Later the signature and verification code is added, and the tarball contains
    >    signatures.
    >    In practice and in field, the deployed BMCs will not be using the "default"
    > openbmc key, and thus any update shall be using a signed tarball. The one who
    > generates the signed tarball shall make sure the build matches the machine.
    >
    > So I think the "security issue" is not really a problem.
    
    There can be two mechanisms to prevent the wrong image from being loaded 
    onto the BMC.
    1. Check the new firmware image's digital signature.
    2. Checking the new firmware image's machine type.
    
    Both of these can work together.   For example, consider an environment 
    where two machine types are supported.  Would each machine type have its 
    own digital signing key?  If the key was shared, you might still be able 
    to load a firmware image onto the wrong BMC, and even though its 
    signature checked out okay, it would be for the wrong machine type.
    
    Finally, have we worked out how to recover when your private signing key 
    has been compromised?  Or the public key stored in the BMC has been 
    damaged (altered)?  It might be nice to have the "machine type" check 
    independent of the digital signature check.
    
    
    I understand this email is focused on trying to prevent code update with 
    unintended firmware images, but I see additional security 
    considerations.  For example, attackers can downgrade the BMC to an 
    older release that has known security vulnerabilities (a "downgrade 
    attack") and exploit them.  To help prevent that, we want to include a 
    version check as part of code update, and require additional authority 
    to downgrade.  Some advocate for semantic versioning: 
    https://urldefense.proofpoint.com/v2/url?u=https-3A__en.wikipedia.org_wiki_Software-5Fversioning&d=DwIDaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=v9MU0Ki9pWnTXCWwjHPVgpnCR80vXkkcrIaqU7USl5g&m=arMHhrzzBHUMyI_30avg7eppPTw9DIZJoNVYq20Hro0&s=gUVAKbT346hy1mkt2Qhe-2i5owHPOeb-mRkz_1tHTgk&e= 
    
    >
    > But I still like the idea to check the machine name to prevent incorrect
    > updates accidentally.
    >
    >> Design approach:
    >>
    >> ==============
    >>
    >> As I see that while building image, there is a /etc/os-release file which gets created and includes version, machine name as OPENBMC_TARGET_MACHINE. This machine name is nothing but a MACHINE defined in bitbake environment. So I thought of using same MACHINE value to be appended in MANIFEST file which is being parsed by updater. And verify this against running image release file before validating image. There are following question comes here.
    >>
    >>
    >>
    >> Backward compatibility : For this we can allow image upgrade if machine name is not defined in MANIFEST file or if it is defined than it should match to current running image.
    > Yes, I prefer to keep backward compatibility for a few release cycles.
    > E.g. in 2.7 and 2.8, let's make it support tarballs without machine name.
    > And in 2.9, make machine name mandatory.
    > What do you think?
    
    The staging plan makes sense, but we may want to consider more use 
    cases.  For example, consider what happens when we scrap or re-purpose a 
    system.  The BMC gets pulled out and may be re-used for a different 
    target machine.  To support this, we might consider a way to override or 
    force a new image onto the BMC regardless of the machine type it 
    currently supports.
We can certainly add this feature of forceful upgrade without any check 
of machine and version.
    
    An alternate conflicting viewpoint is to lock down security so there is 
    no way to update the BMC with an image that supports the wrong machine 
    type.  In this case, flashing the BMC is the only escape. We can do that 
    in the development lab, but not all facilities have that capability.
We can add another feature of NO_MACHINE_CHECK at build time
    
    >> Validation level: Current code updater supports upgrade for host, bmc and psu all firmware. So I am not sure if machine name is going to be same for all components or it is different. In my understanding, all components should have same machine name if they are part of single machine. I am open to discuss this point as I am not sure how everyone else is defining it.
    > I agree with this one.
    > Adding machine name for all tarballs (BMC, host, PSU) makes sure the image is
    > for a specific machine.
    > Even if an image is meant to support multiple systems (e.g. a PSU image may be
    > used on different systems with the same PSU), the update image could be
    > generated (and signed) for the specific machine as well.
    
    What I would like to see (but don't have time to propose) is a check 
    made during code update that checks:
    1. Is the new image in the same series as the current code?  That's your 
    machine type check.
    2. If so, is it a higher-numbered image?  That helps prevent accidental 
    downgrades.
    
    If these checks fail, we can either (A) generate warnings, or (B) 
    require additional confirmation from the operator to proceed, or (C) 
    refuse to load the new image.
Certainly we can add more flexibility.
    
    I see the check for machine type as a step in the right direction, so I 
    am in favor of it.  Thanks!

Thanks, I will start with current plan and add more feature further.
    
    - Joseph