Author Archives: Daniel Bachhuber

Daniel Bachhuber 19:45 on April 17, 2015  

Information disclosure security vulnerability debrief

Now that we’ve had time for the dust to settle, we’d like to publicly debrief on the information disclosure security vulnerability from last Thursday, April 9th.

What the problem was:

  • Any user, whether authenticated or unauthenticated, could access any post revision. Post revisions should only be accessible to authenticated users who have permission to edit the post.

What happened:

  • Thursday morning, 6:39 am PT, a WP-API user submitted a public GH issue with the information disclosure security vulnerability.
  • Daniel saw the issue, saw it for what it was, and had a bit of an “oh shit moment”. His first step was to attempt to reach out to the WordPress.org security team for guidance.
  • Around 7:45 am PT, Nacin responded and created a private Slack channel to coordinate the response. He suggested releasing a fix as quick as we could, and being loud about the release.
  • Daniel created reasonably private release branches on his own fork to share with testing. Joe and Rachel helped out with testing. We set 10:30 am PT as the announcement time.
  • As we got closer to announcement, we added a variety of WordPress community members to the private Slack channel to help verify the fix.
  • In this process, Joe found a third, less critical but still severe bug to fix. This delayed the release by an hour. We had already shipped v1.1.2, v1.0.1, v0.9.1 and v0.8.1 to WordPress.org, which necessitated creating new versions. v1.2.1 wasn’t affected by the third bug.
  • Fixed versions of WP-API and announcement post were live by 1:39 pm PT, exactly 7 hours after the Github issue was opened. Auto-updates started rolling shortly after that.

What went well:

  • Daniel: We shipped the fix out in a timely manner.
  • Daniel: Private Slack room to gather applicable parties. As the issue progressed, we added representatives from some hosts, etc.
  • Daniel: One person dedicated to committing fixes, and another dedicated to testing.
  • Daniel: Multiple people that could’ve taken the lead on getting the fixes shipped.
  • Daniel: Auto-updates greatly increase confidence of getting the fix deployed to live sites.
  • Daniel: Representative from hosts were attentive once we had them identified.
  • Joe: Prioritised other commitments, Daniel especially – gave up a whole day to fix the issue. Not possible for everyone to do at the drop of a hat, but as a team, we reacted well.
  • Joe: Communication throughout was very good – the private group on slack provided a good way to converse. Also, external comms with numerous stakeholders was very good.
  • Joe: Reviewing / testing – lots of people were fast to react and test fixes, I also put in a bunch of time to verify, this made the push live much smoother when it finally happened.

What can be done better next time

  • Daniel: Bug shouldn’t have been publicly disclosed. It was only a matter of timing that Daniel saw and acted on it promptly. Could’ve been much worse.
  • Daniel: Difficulty getting ahold of the security team. There was an hour delay before we had Nacin involved and a security backchannel going. Next time need to email security@wordpress.org
  • Daniel: Would be nice to have some advance list of sites, people, etc. to reach out in advance of publicly disclosing.
  • Joe: Having a more structured approach / run book document on what we should have done. For example, who can commit to the SVN repo, list of people we should reach out to.
  • Joe: Perhaps a definition of how many version back we decide to patch – perhaps case by case depending on the severity of the issue though.
  • Rachel: Explicitly mention everyone should update, even those who have checked it out via version control. The language of the announcement post could’ve communicated this better.
  • Rachel: Identify a dedicated “press” contact for everyone who wants to write about it.

Feel free to ask questions, add comments, etc. in the comments.

Daniel Bachhuber 00:00 on April 7, 2015
Tags: meetingnotes   

WP-API chat summary – April 6th, 2015

  • On Friday, we started going through 2.0-beta1 issues. Asked Rachel why we did crazy things where we did crazy things. Some new issues opened.
  • Daniel bonked hard on the password handling code. Setting a cookie to view a password-protected post in REST API is pretty bad.
  • Core’s cookie handling is per COOKIE_HASH, not post, so viewing a second password-protected post nullifies the first.
  • We can’t pass the password as a GET parameter because it could expose the password in the logs.
  • WordPress.com accepts the password as a GET parameter. This is likely an unacceptable risk for core.
  • Ryan suggested authorization headers instead of parameters.
  • Another open question: what fields are expected to be password-protected?
  • We should err on the side of less data (e.g. not in the loop) for the time being.
  • Discussed #1023 (returning error when returned data mismatches declared schema)
  • Meech suggested we try not to solve the problem of “anyone with access to the source code can break the API”
  • Ryan’s suggestion is we encourage clients to write robust clients, and not segfault on bad data. Likes the idea of including useful header data in the request, but not in the response.
  • Q: Should plugin developers be able to remove fields from a response?
  • Rachel has used our existing filters in v1 of the plugin to add and remove fields.
  • Ryan: it’s important our documentation emphasizes removing fields would break things.
  • Meech: Consider who we’re protecting, and what we’re protecting them from. This seems to be protecting clients from bad plugins. But it really seems we’re trying to force people to use our ideas. If I’m a plugin and start breaking clients, I’ll get reports on how to fix it.
  • Rachel: Users can disable WordPress themes with a constant. We should make sure WP-API is as extensible as WordPress.
  • Rachel: 80% of users don’t know how to write the PHP code to break a response. The 20% that can, also know how to refer to documentation and best practices.
  • Joe: Agree with the consensus. Get really pissed off when someone locks down what he knows how to work around.
  • Decision to not restrict removal of fields. We can’t easily report the distinction either.

#meetingnotes

Daniel Bachhuber 00:24 on February 12, 2015  

WP-API chat notes – February 11th

Daniel, Joe, and Ryan chatted over Skype. Ryan was unavailable for 20 minutes in the middle of it. Rachel couldn’t make it.

  • Main thing want to fix is that we get continual questions about meta.
  • Ryan has made a doc change to explain why we don’t expose meta by default.
  • Joe: Can we just go through and fix all of the bugs?
  • Daniel: Some are fixable bugs, other are debatable bugs. Let’s rename “Feedback” to “Discussion” as an indicator for debatable bugs.
  • Discussed DELETE requests https://github.com/WP-API/WP-API/issues/789
  • Problem is that it’s inconsistent: calling DELETE twice shouldn’t have different behaviors.
  • Ryan’s proposal: DELETE always trashes the post. force=true must be passed to permanently deleted.
  • Discussed invalid arguments: https://github.com/WP-API/WP-API/issues/871
  • Post format needs to be validated against a set of potential options.
  • parent_id and author_id need to be validated against their respective objects
  • post_title and post_content also should check string types because saving an object would be bad
  • One potential implementation is some validate argument of the schema that is automatically checked before hitting create_item()
  • Discussed read-only fields: https://github.com/WP-API/WP-API/issues/747
  • Might be another argument for preflighting data so we can throw an error if you try to supply the field
  • Possible rule to follow for the API: if invalid data has been provided for a valid field, throw an error. If invalid data has been provided for an invalid field, we can silently ignore.
  • Joe and Daniel are generally against nested object for discussion. Count should be an attribute of the link to the collection, and it doesn’t make sense to have statuses for posts and pings. We should avoid nested objects as much as possible because they make for painful cURL requests.
  • Title (raw|rendered) and content (raw|rendered) are unaddressed beasts in this regard. Some discussion of their inconsistencies.
  • Discussed named routes: https://github.com/WP-API/WP-API/issues/577
  • One useful use for this is shorthand for referring to other routes. Avoids having to update a bunch of paths.
Daniel Bachhuber 00:13 on January 22, 2015  

WP-API chat notes – January 21st

  • Comments pull request is looking pretty good.
  • Daniel’s point about renaming fields seems valid, although we should do it all at once.
  • Once we have everything working to some degree, we can have the masses file their bug reports
  • Rachel’s concern is that pull requests can get caught up in the minutia; solution: file an issue and come back to it. Issues are cheap.
  • Q: What about other idiosyncrasies, like JSmith’s concern that you can’t create a new post with post_author=0?
  • Joe suggests we have a label for v1 vs v2 to better distinguish pull requests.
  • Joe wants to discuss the capabilities pull request. Two contexts: options request to see if they can do something (don’t have necessary context), or a request with the specific context (e.g. specific author or post).
  • Daniel apologized for being an asshole. He got frustrated, and didn’t communicate his frustration appropriately. It’s important to remember that we’re all volunteering our team, and should treat each other with the most respect possible.
  • Joe suggested we do one video chat a week, and one text chat a week. Everyone thinks this is a good idea.
  • Ryan is having internet difficulties at home. He will be having trouble making the meeting on time for another couple weeks.
  • We reviewed the core merging plan. Capabilities PR is the last item for the server aspect.
  • Issue #717 is ripe for taking. Ryan offered to itemize what he was thinking.
  • Ryan asks whether we can move schema down and out of the server, so we can have core developers start looking at the server components.
  • Daniel thinks that’s fine — a good sign that the server is relatively stable is that we haven’t been making many changes.
  • Ryan’s been looking at different implementations of batch requests. Both Facebook and WordPress.com use JSON pointers, but both are hairy. Still TBD what this could be.
  • Joe: Presumably you need to worry about the dependency graph too.
  • Daniel: What about Github? What if you want to create an issue with labels?
  • Ryan: Think label is just a string, and they run a search against it. Assigning labels is just an array of strings
  • WordPress.com supports tags but doesn’t scale up to custom taxonomies, etc.
  • We have the opportunity to do it right, but it will be a lot of work. Let’s punt until Step 3.

Between now and Monday we want to:

  • Ryan: Triage and catch up with what’s going on
  • Daniel: Taking the file breakout, #717
  • Joe: Might take a swing at mapping out the options API.
  • Rachel: Getting pretty tired of rewriting endpoints. Going to take a bit of a break

Next week: Monday video chat. Wednesday text chat.