Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Secure extension updates from infrastructure attacks #746

Closed
JDGrimes opened this issue Nov 8, 2017 · 4 comments
Closed

Secure extension updates from infrastructure attacks #746

JDGrimes opened this issue Nov 8, 2017 · 4 comments
Assignees
Milestone

Comments

@JDGrimes
Copy link
Member

JDGrimes commented Nov 8, 2017

Background: https://core.trac.wordpress.org/ticket/39309

Working based on the patch on that ticket, we should be able to digital signature verification of downloaded extension updates. I have already built the necessary code for the .org side of things, see https://github.com/JDGrimes/edd-sl-cryptographic-signatures.

Essentially, all we need to do is:

  • Hardcode the public key in the source code.
  • Utilize the package signatures now offered via the API, and the public key, to verify extension update packages.

We'll bundle the sodium_compat library to handle all the heavy lifting of actually verifying the package contents. It relies on random_compat, which is already bundled in WordPress core, so that is no problem.

@JDGrimes JDGrimes added this to the 2.5.0 milestone Nov 8, 2017
@JDGrimes JDGrimes self-assigned this Nov 8, 2017
@JDGrimes
Copy link
Member Author

JDGrimes commented Nov 8, 2017

In the case that the secret key was ever compromised, we would be able to replace the public key that is hardcoded in WordPoints via forced-pushed security updates for the plugin. Since the plugin updates are offered through the entirely-separate WordPress.org infrastructure, I consider this to be a sufficient back-up plan in case of the worst possible event. It could be improved, of course, but it isn't a reason not to go ahead and get this out now, knowing that we can improve it in the future if needed.

@JDGrimes
Copy link
Member Author

JDGrimes commented Nov 8, 2017

What should the upgrader do when the server API doesn't support signed packages (doesn't implement the interface)? And likewise, when it does implement the interface, but doesn't return a public key for a particular extension?

I doubt that any APIs have been created since 2.4.0 was released. But I still feel like we shouldn't make this a hard requirement right off. I was thinking that maybe we would just give some kind of error for now (_doing_it_wrong()), but allow the update to proceed. Then in the future we'd change to giving an error. The logic behind that being that it is more likely that we'd be preventing important security updates than blocking a malicious update. Also, if a user doesn't update an extension that is bundling an alternative API, then they won't be able to update it at all after WordPoints 2.5.0 is released. They'd have to update manually.

So I think we'll just give a _doing_it_wrong() for now, and in 3.0.0 make it a hard requirement. That should leave plenty of time for custom APIs to implement this. (I guess it also leaves plenty of time for new APIs to be introduced that don't implement this, but the notice should be sufficient to prevent that; if someone ignores the notice, they have it coming anyway.)

@JDGrimes
Copy link
Member Author

JDGrimes commented Nov 8, 2017

In the Trac ticket, it was proposed not to enforce signature failures until the next release. Instead, the code would go on ahead with the update, but in the process WordPress.org would be notified. That would allow for any failures to be monitored, and any issues resolved.

In this case though, I don't think that is necessary. There we are talking about a package updating itself, and so issues with that process are a big problem. In this case if there are any issues we can fix it in WordPoints, which uses separate update infrastructure. So I see no need to wait, we can go ahead and abort the update on signature verification failures from the first release. An error will be shown to the user, and they'll probably report it to us, if there are any issues.

However, there are two concerns there. The first and lesser is that the user might just ignore the update and not report the issue. So if it was a security update, they'd remain vulnerable. Thus, it would be a good idea to make it as easy for them to report the issue as possible (like including a link).

There is also a bigger concern, in regard to cases where the signature actually is invalid (not just that their site can't verify it), that is, that the package is compromised. In that case, WordPoints.org may have been hacked. If so, we probably don't want to direct the user there, since it may not be safe. And we also would like to know about it ASAP.

So I think perhaps the best thing to do is to ask the user to report the issue in the error message, and include a link to the WordPress.org support forums for the plugin. Since that is completely separate infrastructure, an attacker won't be able to harm them through that or prevent us from receiving the information about the error (as they could if the user tried to contact us through WordPoints.org).

@JDGrimes
Copy link
Member Author

JDGrimes commented Nov 9, 2017

Here is what an update no looks like on success (note the "Verifying update" part):

screenshot-2017-11-8 update wordpoints extension wordpress develop wordpress

And on failure:

screenshot-2017-11-9 update wordpoints extension wordpress develop wordpress 1

I think that the wording is urgent enough to encourage a user to report, but hopefully won't cause them to panic that their site may be compromised.

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

No branches or pull requests

1 participant