Skip to content

(blog) HTTPS Blog Post#398

Merged
devlead merged 1 commit into
cake-build:developfrom
gep13:feature/https-blog-post
Aug 26, 2017
Merged

(blog) HTTPS Blog Post#398
devlead merged 1 commit into
cake-build:developfrom
gep13:feature/https-blog-post

Conversation

@gep13

@gep13 gep13 commented Aug 25, 2017

Copy link
Copy Markdown
Member

@cake-build/cake-team can you all have a look at this, and let me know if you have any comments?

@dnfclas

dnfclas commented Aug 25, 2017

Copy link
Copy Markdown

@gep13,
Thanks for having already signed the Contribution License Agreement. Your agreement was validated by .NET Foundation. We will now review your pull request.
Thanks,
.NET Foundation Pull Request Bot

@bjorkstromm bjorkstromm left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM:+1:

@devlead devlead left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

@devlead devlead merged commit 721f567 into cake-build:develop Aug 26, 2017
cake-build-bot pushed a commit that referenced this pull request Aug 26, 2017
Merge pull request #398 from gep13/feature/https-blog-post

(blog) HTTPS Blog Post
@matkoch

matkoch commented Aug 26, 2017

Copy link
Copy Markdown

@gep13 you say:

For the most part, the switch to HTTPS doesn't mean anything, ....

That's contradictory to what I see. Looking at the search results I've seen at least 5 repositories within the first 20, that don't have a packages.config file committed.

About the minimal risk situation, I'm not sure, so please take this as a question: Looking at how ToolResolutionStrategy is implemented, every call to a tool that should be contained in %PATH% can potentially call a malicious executable. Or am I wrong?

@devlead

devlead commented Aug 26, 2017

Copy link
Copy Markdown
Member

Looking in path is how any OS works to find binaries.

@matkoch

matkoch commented Aug 26, 2017

Copy link
Copy Markdown

@devlead I'm not following?

@devlead

devlead commented Aug 26, 2017

Copy link
Copy Markdown
Member

Agree

@gep13 gep13 deleted the feature/https-blog-post branch August 26, 2017 15:08
@gep13

gep13 commented Aug 26, 2017

Copy link
Copy Markdown
Member Author

@matkoch said...
That's contradictory to what I see. Looking at the search results I've seen at least 5 repositories within the first 20, that don't have a packages.config file committed.

The guidance on the cakebuild.net website is clear, that these files should be checked into your source control repository:

https://www.cakebuild.net/docs/tutorials/getting-started

What you have us do differently?

@matkoch said...
Looking at how ToolResolutionStrategy is implemented, every call to a tool that should be contained in %PATH% can potentially call a malicious executable. Or am I wrong?

Can you elaborate on what you mean here?

@agc93

agc93 commented Aug 26, 2017

Copy link
Copy Markdown
Member

Chiming in on the "insecure executable search" part:

There is no way for us (a developer-focussed build tool) to pre-empt the possibility of a malicious executable already locally installed on a machine. If we take the hypothetical scenario of a backdoored dotnet.exe already installed and available on PATH, our behaviour is identical to that of a user putting dotnet into PowerShell or Bash.

Compromised local workstations are well and truly outside the scope of a tool like Cake. We've (now) partially pre-empted the risk of a backdoored cake.exe (which could do things like deliberately resolve compromised versions of tools), but it's still up to the developer to secure their own workstation.

There's no practical change we could make to cover that sort of a case, and we should be relying on the tools provided by more fully-fledged systems (in this case the OS) then trying to half-bake our own solution and potentially introducing more risk (for an example of this, see any home-grown crypto algorithm).

Happy to discuss this as security is really my thing :)

@agc93

agc93 commented Aug 26, 2017

Copy link
Copy Markdown
Member

It's also worth noting on the packages.config question that there's no easy change we can make to cover cases like people not checking those files in (despite explicit documentation) that wouldn't break behaviour of existing users.

Given we have no evidence of in-the-wild vectors, vulns, payloads or end-to-end attacks focussed on our tool (or any other .NET build tool outside of VS), there's no impetus at this stage for making the sort of drastic changes that would require.

Also, to be fair, for developer tool guidance, 75%+ takeup of checking in the packages.config is actually a very promising sign. We've covered the guidance of this enough in the past and are still (to this day) recommending it explicitly when the topic of .config handling comes up (for example in Gitter), so the responsibility is now with developers.

If there is/was an actionable, practical solution to this that wouldn't be either a) over-extending our guidance and approach or b) impractical for new and existing users, we're all for it; at this time, there simply isn't much more we can do that would present a meaningful improvement.

@matkoch

matkoch commented Aug 26, 2017

Copy link
Copy Markdown

@agc93

Happy to discuss this as security is really my thing :)

Great, here too :) So first, I'm not talking about compromised executables in the PATH directory. That's of course users fault. What I'm talking about is the previous situation, where it was possible (via man-in-the-middle) to download malicious executables into the tools directory. So that is a fact we can agree on, I guess.

The second thing: I've linked the code from ToolResolutionStrategy where file paths are resolved. Like I said, it was rather a question, but from what I see, it resolves paths like this:

  1. Look in registrations (I'm not aware, what is actually registered)
  2. Look in tools directory (via globbing) <-- that being the issue
  3. Look in PATH

So given the fact, that packages could be downloaded without the user being aware, any call to a executable the user expects to be in PATH, could potentially be loaded from tools... Again: I'm asking.

@matkoch

matkoch commented Aug 26, 2017

Copy link
Copy Markdown

@agc93

At least there is one thing I can think of. You could modify cake.exe to look for build.ps1 and build.sh in the root directory, and look if it still accesses http instead of https. If so, you could intentionally fail the build. (A suggestion)

@matkoch

matkoch commented Aug 26, 2017

Copy link
Copy Markdown

@gep13 about this:

The guidance on the cakebuild.net website is clear, that these files should be checked into your source control repository:

I can't agree. The guidance was mostly about having reproducible builds, not having security. And security is not a feature. It should be ensured in whatever way the users is using a tool.

@matkoch

matkoch commented Aug 26, 2017

Copy link
Copy Markdown

@agc93 @gep13

I did the following:

  1. Download cake-build/cake
  2. Create a folder inside tools with dotnet.exe; (We agree this was possible due to the http issue. Not in the cake repository, but for a random repository without packages.config.)
  3. Execute build

This will fall back to the newly created dotnet.exe. I'm not sure though if this is the same issue as I described, or whether or not it relates to specifics of the .NET core version of build.ps1.

@gep13

gep13 commented Aug 26, 2017

Copy link
Copy Markdown
Member Author

@matkoch just so I am 100% clear on this situation, the compromised exe is coming from where? As a malicious package that has been pushed to, say, nuget.org?

@matkoch

matkoch commented Aug 26, 2017

Copy link
Copy Markdown

@gep13 yes.

@agc93

agc93 commented Aug 26, 2017

Copy link
Copy Markdown
Member

@matkoch Okay there's a few things going here, so going to address individually, and in detail:


ToolResolutionStrategy

Yes, it would be possible for a compromised tool to be placed in the tools/ folder and Cake would shell out and run it. This is still a case of local compromise (i.e. the executable had to be put there somehow) and there's a few things that make this largely non-issue:

  • The executable has to come from somewhere. We're certainly not downloading it (if someone hijacks Roslyn, we've all got bigger problems) so if it's happening in a bootstrapper or from a #addin, that's a user's actions that we can't prevent or overrule.
  • This would require an attack knowing the exact path to place an executable in, in relation to a .cake file at a known location. This is basically impossible without local workstation access (we can't protect against that).
  • Note that any attack would also need to know exactly which executable to compromise and load (simpler with dotnet.exe for example, but there's no guarantee someone would call, say, GitVersion.exe making attacks infeasible)
  • Compromised executables are still loaded by the platform and underlying OS, so we can rely on things like improper manifests (would be stopped by the runtime), assembly/code/certificate signing (will flag compromised environments, prompting further investigation), and OS protections (real-time AV etc) to prevent really major problems.

If people are downloading executables in their bootstrappers from compromised/http:// sources, that's their call, not ours. We can't prevent people making bad security decisions, even if it would make my job a lot easier.

Given the compromised executable has to be put there somehow (and it's not done by cake) we're back at existing local compromise or user error. In short, the risk surface we present here is no lower than what VS (or Rider for that matter) already does through extensions, for example. Plus, there's little we can do here without breaking existing behaviour.


packages.config check-in

Let's look at the attack scenario here:

Attacker (through local or MitM) forces bootstrapper to download compromised packages.config file. That file would then need to download a cake.exe from a non-standard (compromised) source since only the bootstrapper will do the actual invoking. That local cake.exe will need to behave exactly like ours (again, otherwise bootstrapper will fail) but run maliciously.

This is extremely unlikely.

Nonetheless, presuming this actually happens, there is still not much we can do to prevent this given we're not about to start breaking builds because people have chosen a different layout or approach to what we think. What happens if someone is actually pulling their packages.config from a known-good internal server? Or another Git repo accessible only with authentication? There's a lot of complicated edge cases here we can't prepare for (and nor should we necessarily)


Failing the build on http detection

This is still not practical: we should never break builds for existing users*. We could potentially issue a warning, and I will look into this, but by my assessment the risk of this seems broadly similar to the risk of a user receiving an SSL-intercepted malicious copy (such as behind an SSL-rewriting corporate proxy).

If nothing else, a change like this is highly against our approach for the exact same reason as your own comment: we can't guarantee people are checking our blog or Twitter feeds to find out why their builds are suddenly failing (or even showing warnings).

As mentioned, I'll have to think and look at the practicality of this, especially given it would be highly unreliable in CI scenarios (where we see a lot of use in the wild).

*: There are obviously exceptions, but they should be rare and more warranted than these edge cases


An aside on build tool compromise

As part of my job I have done hundreds of investigations, audits, scans and assessments of the security of build environments and the issues we're dealing with here are incredibly obtuse. I have never seen the attack vectors you described used on any build tool in the wild. Attacks are relatively commonplace on platforms like Jenkins, and there have been rare cases of attacks against Maven and Gradle, but these are generally targeted attacks on specific code bases rather than all users of a tool.

The one exception I will give is npm which is a security nightmare in ways I won't get into here. If an attacker wants to compromise .NET build environments, Cake is simply not an effective way of doing so. PowerShell, NuGet, VS, Rider and .NET itself are all drastically simpler, less protected, more commonplace and easier to escalate.

The majority of cases here require some form of local or upstream compromise. An attacker with this would not need to use Cake as a vector: the OS and local environment are plenty powerful for that.


Communication

You bring up a good point here with no real solution: if people never see our blog, Twitter account or Gitter room, there's nothing we can do to reach these users. I'm still not 100% sure on what you expect us to do to support users who have zero contact with any of our public contact points? We've spent years building an excellent trust relationship with our users ranging from support to breaking changes to community conduct, and making highly opinionated changes is difficult without a (currently non-existent) method of reliable communication so we are somewhat limited here.

You'll obviously be familiar with this problem given it's not unique to us..


General security posture

I fully agree that security is not a feature, it's a requirement (security is literally my job). But we also have to remember that our users must assess their own security posture and respond accordingly. This is especially true given we have always recommended users customise their bootstrappers to their environment. High-risk users should rely on checksums and local trusted mirrors for example. We can provide security for a reasonable environment, while allowing for users to tailor to the risk to their specific use case.

As before, I'm happy to discuss this further, but I don't think there's much more to be gained or discussed on this specific front. As with our team's earlier communication with you, I believe any further development in this space is probably best redirected to a new issue, or a Gitter room or similar as we have long since left this PRs topic behind us.

@agc93

agc93 commented Aug 26, 2017

Copy link
Copy Markdown
Member

I will also add the final note that it's 3am here, so I won't be responding to anything more tonight. See my last paragraph above: happy to discuss further, but maybe in another context and (preferably) another time 😄

@matkoch

matkoch commented Aug 26, 2017

Copy link
Copy Markdown

ToolResolutionStrategy

It's not about an existing local compromise or anything you've mentioned about having to know where an executable is located or not. The attack vector is

  1. MitM returns packages.config with additional package that contains malicious code
  2. NuGet.exe restores this package as part of the bootstrapping (so this is also not about a compromised Cake.exe)
  3. Cake.exe executes and tries to resolve a tool like dotnet
  4. Executable from PATH gets overwritten from executable in tools.

That's the only issue that remains after bootstrappers and NuGet.exe have been made to be downloaded via HTTPS.

The part about manifest and stuff doesn't seem relevant to me.

If people are downloading executables in their bootstrappers from compromised/http:// sources, that's their call, not ours.

Like I said, until recently, the default bootstrappers downloaded packages.config (and also build.ps1 and build.sh) via HTTP. So how is it their call then? I don't remember anyone saying, the bootstrapper must be customized. And besides, why not provide a meaningful default in the first place? Adding 's' is not very hard.

packages.config check-in

Like I said it's not about downloading a compromised Cake.exe.

Failing the build on http detection / Communication

Well, You've asked what to do except blogging/tweeting, so I've given you a suggestion :) I might be putting more value into security to justify breaking it like that, but of course, a warning will do too. The actual advantage is, that 100% of the affected users will immediately get this warning (if not compromised already :D). Since they didn't commit a packages.config. Good thing I guess. And you could even make it explicit to the full URI, if required.

An aside on build tool compromise

I guess there's always a first time for an attack vector. You surely know :) But when it gets public, it should be communicated in a timely manner, don't you agree? It's not my intention to get too academic in this matter. After all, I created the issue as a simple one-liner with the intention to close a little security gap. But well, first you closed it as a duplicate - which it was not, but okay. Then closed the so-called original issue, but didn't fix the actual issue I was talking about. Then you claimed that redirection from HTTP to HTTPS is enough - which is not. And then didn't make a tweet or anything as you promised to do.

To be absolutely clear: if you don't want any more input on that, just say so. Some replies made the impression that you don't. Otherwise, given the mentioned on-topic responses, I feel obliged to give further feedback, as it seemed you've overlooked/misunderstood something.

Now there is this wrong statement in the blog post about:

To be clear, this is not a file that is executed, but rather placed on your file system, and packages are restored from that file using NuGet.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants