Salt-bootstrap: Multiple MiTM security issues

[gdm85]

There are multiple places in the bootstrap-salt.sh script where a text file is downloaded via HTTP and its integrity is never checked with a hash or GPG signature. This is an open door for MITM attacks and puts the salt-bootstrap script no less than at the same level of any curl http://get-some-cool-stuff.sh | bash one-liner advices that are so popular on the interwebs these days, with the added cleartext insecurity.

I gathered following examples at current master commit ab3dac2c275308e06739450b7ab10bf1f75a97b4

Examples

https://github.com/saltstack/salt-bootstrap/blob/develop/bootstrap-salt.sh#L2034 (PGP public key)
https://github.com/saltstack/salt-bootstrap/blob/develop/bootstrap-salt.sh#L2676 (repo file)
https://github.com/saltstack/salt-bootstrap/blob/develop/bootstrap-salt.sh#L2684 (repo file)
https://github.com/saltstack/salt-bootstrap/blob/develop/bootstrap-salt.sh#L4086 (repo file)
https://github.com/saltstack/salt-bootstrap/blob/develop/bootstrap-salt.sh#L4279 (repo file)

There are possibly others that can come up with an extensive analysis.

Hardening tips

Instead of moving these files to a HTTPS source, I suggest to include them inline in the script using heredoc syntax with cat.

[vutny]

There is also an issue with downloading bootstrap script itself in the "official" way: #691

[vutny]

691 was fixed.

This is no longer a case, was removed in develop:
https://github.com/saltstack/salt-bootstrap/blob/ab3dac2c275308e06739450b7ab10bf1f75a97b4/bootstrap-salt.sh#L2684
This is also has been fixed:
https://github.com/saltstack/salt-bootstrap/blob/ab3dac2c275308e06739450b7ab10bf1f75a97b4/bootstrap-salt.sh#L2676

Dealing with Debian and Suse becomes more complicated....
Repositories layouts change quite often these days, so I think using heredocs is not an option, at least for now.

[gdm85]

@vutny yes, sorry for not including the commit hash in OP links..

Shall I review the current version of the scripts? Or are there other issues open to track the remaining problems?

Edit: I see there are still plenty of cleartext HTTP downloads of yum repository sources (which are not verified against a signature/hash AFAIK), thus I would be inclined to not close this issue as it's not fixed/addressed. Technically it's more of a bad practice problem rather than an individual issue or collection of issues.

As per the fast pace of changes: using heredocs or including the files or adding some hash/signature verification is equivalent. I think there can be an elegant/compact way to implement proper secure/validated downloads and yet allow the many dependencies here without hindering updates.

[vutny]

@gdm85 Feel free to study current develop version and put on the table any concern you will find.
I believe many things we have for now could (and should) be deprecated, and if there would be more reasons to do that, the sooner we'll get the improvements.
I am personally interested in making bootstrap secure and reliable. You have all good points.

[gdm85]

@vutny I will make a pull request to show how this could be tackled.

My perspective is the one of anybody willing to integrate this in their system; if you have a requirement of security that prevents this type of MiTM to be in the code anywhere, then he/she will have to fix it themselves anyway via a fork or patch.

[gdm85]

I have added a pull request that adds a FIXME for each of the remaining MiTM vulnerabilities and closes the gap for two of them by using:

1. md5sum + size validation
2. HTTPS over HTTP (for anonscm.debian.org access)

The first one is exemplary to show how issue could be tackled with pinning.

[vutny]

@gdm85 I love an idea of checking remote file hash. By the way, Salt does exactly the same in file.manage state module. I think it would be better to use SHA256 or even SHA512 checksum. With these strong hashes we don't need to perform additional size validation.

Or maybe, do you have some concerns about the file size?

[gdm85]

@vutny the combination of MD5 + file length was used there exclusively because it is usually available on many systems out-of-the-box, but otherwise I would use SHA256 for hash pinning

[gdm85]

@vutny feel free to close this when issues have been addressed. Shall I update or close #788? I guess you could cherry-pick and adjust as needed, or just reproduce some parts, no need to credit me for such small things :)

[vutny]

@gdm85 :+1: Your feedback was much appreciated. Personally I would like to keep both this issue and PR #788 open as a reminder and some kind of TODO list :-)

[gdm85]

Alright! Will keep an eye on this then :)

[vutny]

I guess the only thing left to be secured is SUSE stuff...

[jfindlay]

@vutny, that has also been fixed with #791. In my testing, HTTPS is supported by repo.saltstack.com, but not by download.opensuse.org. Is there anything else that needs to be done here?

[gdm85]

@jfindlay If you look for FIXMEs in the code, I see the OpenSuse one and one about the mirrors list:

# FIXME: cleartext download over unsecure protocol (HTTP)
    MIRRORS_LIST_URL=http://www.openbsd.org/ftp.html

Perhaps this is not an issue if every download coming from any OpenBSD FTP server is also verified with signatures? I do not know about that.

[vutny]

@gdm85 I think this issue finally has been resolved. Salt packages for SUSE are hosted at that distro's repository and they are signed with appropriate GPG key bundled with OS.

OpenBSD installation function has been rewritten to get mirrors list from local configuration instead for plain HTTP URL.

I believe this can be closed. Thanks again @gdm85 for raising this.

[rallytime]

I think that sounds good @vutny. The only thing I would say is that if @gdm85 is okay with closing this, we should clean up the #FIXME comments. These could be converted to comments that say what you've stated above.

[vutny]

@rallytime Yeah, there is only one "FIXME" about SUSE left in current develop. I will address this in a PR soon.

[gdm85]

Oh, very nice work! Yes I agree with @rallytime, then this could be closed. The issue was indeed initially opened to raise awareness aside from the actual problems. I'll leave it to you guys to close it.

Thanks all!

[gdm85]

I noticed there is a single FIXME left, about this URL: http://download.opensuse.org/repositories/systemsmanagement:saltstack

Which returns a 404; is this expected?
Edit: there seems to be a typo, but since multiple similarly-named directories are being listed there I cannot guess which one would be the correct one.

[rallytime]

@gdm85 The 404 error is not expected. I wonder if they have changed something on their end. I'll look into it. Thanks for checking!

[rallytime]

@gdm85 Please see PR #990 and let me know what you think.