BMC update via TFTP

[Vernon Mauery]

On 04-Dec-2019 09:47 AM, Joseph Reynolds wrote:
>On 12/3/19 1:55 PM, Adriana Kobylak wrote:
>>On 2019-12-03 11:08, Gunnar Mills wrote:
>>>On 12/3/2019 10:12 AM, Gunnar Mills wrote:
>>>
>>>>>In BMC WebUI under "Download image file from TFTP server" section,
>>>>>
>>>>>we have text fields "TFTP Server IP address" and "File name".
>>>>>"File name" doesn't take folders in path. Is this a bug or
>>>>>expected behavior?
>>>>>
>>>>>TFTP downloads work only if file is kept in root of tftp share.
>>>>This is expected.
>>>>
>>>https://github.com/openbmc/phosphor-bmc-code-mgmt/blob/b0ce996ac60cf80487d71c3cdb7165d065079377/download_manager.cpp#L33
>>>
>>>>[1]
>>>
>>>As long as we continue to sanitize the local filename, I don't see why
>>>we need to limit the source file name.
>>> Patrick, Adriana, Any objection? Remember why we wrote it this way? I
>>>assume for simplicity..?
>>
>>It was done for security to prevent users from specifying a file 
>>outside the tftp directory, such as /mydir/../root/system-file.
>>But seems the current file name handling breaks the ability to get 
>>files from a subdir like Raj pointed out, we should be able to fix 
>>that out, tftp supports having a file in subdirs such as 
>>/tftpboot/subdirectory/file, and passing /subdirectory/file to it as 
>>the path.

While we are on the topic of security, should we do something about 
unsecured protocols for image transfers? Adding an HMAC and key along 
with the IP/path would then make sure the file that was fetched contains 
the contents that were expected.

Even if the BMC only accepts signed images, we want to make sure that 
the signed image the BMC fetches is the one that the administrator 
*wants* to be fetched. With tftp or http (or any insecure transport), 
one possible MiTM attack would be to substitute an alternate valid 
image. Let's say the admin wants to go from 1.18 to 1.20, bu the 
attacker wants to go to 1.16, which has a known vulnerability. The 
image would be authenticated by the signature, and will be accepted.

With an added HMAC, the MiTM attack would be thwarted. The HMAC and key 
would be transmitted to the BMC over a secure channel (https) so the 
attacker would not be able create a suitable replacement for the image 
in the insecure channel.

--Vernon

>It seems to me that the burden must be on the TFTP server to protect 
>itself.  See the "Security Considerations" section of RFC 1350 - 
>https://tools.ietf.org/html/rfc1350
>Specifically, OpenBMC only needs read access (it does not need to 
>write files to the TFTP server) and only needs to be able to access 
>files the TFTP server intends to provide (typically: all the files 
>under a specific directory).
>
>To be more clear:
>- OpenBMC does not provide a TFTP server, so this is guidance for 
>someone setting a TFTP server for use with an OpenBMC system
>- The TFTP server only needs to offer READ access.
>- The TFTP server ought to prevent clients from using 
>cleverly-constructed pathnames (for example, any file which begins 
>with "/" or contains an ".." element) to access files outside of its 
>sandbox.
>- That said, I am not a TFTP expert, and whichever TFTP server is 
>used, its security guidance should be followed.
>
>What all of the above, I think it would be okay for OpenBMC to 
>validate the pathname it sends to the TFTP server, but don't think it 
>is necessary.
>
>Finally, I hope we can move away from TFTP and more toward a secure 
>image distribution approach.
>Note that TFTP is disabled in OpenBMC by default.  See 
>https://github.com/openbmc/bmcweb/blob/master/CMakeLists.txt
>
>- Joseph
>
>>>
>>> Thanks,
>>> Gunnar
>>>
>>>
>>>Links:
>>>------
>>>[1]
>>>https://github.com/openbmc/phosphor-bmc-code-mgmt/blob/b0ce996ac60cf80487d71c3cdb7165d065079377/download_manager.cpp#L33
>>>
>