[JumpCrisscross]

Given that:

(1) the nature of the suspension was not communicated to Egor at the onset of the situation, nor,

(2) noted in the blog post [1] describing how Github "detected the attack",

I am inclined to believe that this is a response to the furious reaction to their suspension decision and was not, as this post implies, the game plan from the beginning.

It's healthy that they've reversed their suspension but the lack of transparency (not to mention potential dissembling) on the decision process regarding the revocation is still troubling.

[1] https://github.com/blog/1068-public-key-security-vulnerabili...

[maaku]

If you're investigating a security situation, you don't have to say squat. Whether you were in the right or the wrong, explain what happened and apologize to those where were affected once the facts are understood, as they have done here.

It takes time and energy to come up with responses such as this (not a lot, but every bit counts in an emergency), and those are resources that you should be using the solve the problem. Not to mention that saying the wrong thing is worse than saying nothing at all.

It's basic emergency management: 1) stabilize the situation, 2) figure out what's going on, 3) fix it, and then 4) explain what happened to the stakeholders. I see nothing wrong with github's actions here.

[why-el]

Not to mention that this happened on a Sunday.

[stanleydrew]

That is irrelevant. The world doesn't stop turning on the weekend. It was in Github's best interests to get out ahead of this, and they did. They don't need to be commended for it just because it's Sunday.

[freehunter]

Actually, the world _does_ stop turning because it's Sunday. Anything happening on the weekend is emergency management, which takes time to scramble. I work in network/information security, and if we had a security incident on a Sunday, it would go to our backup team (of one). If they (he) deemed it critical, they would notify the security director, who would notify my boss, who would notify the rest of the team. This would take 3-5 hours, plus the amount of time it took us to get to the office and actually investigate the emergency. While we were investigating, we would likely shut down everything (including user accounts that were involved).

Weekends are time away from the office. I don't understand how you could expect the same amount of service on a weekend opposed to a week day. Do they deserve to be commended just because it was a weekend? Internally maybe, yes, externally maybe not. Fact remains that the weekend is when they are most likely to be short-staffed.

[bmj]

I agree. But we're talking about software as a service here, and that kinda makes it a different ballgame. Folks are paying money (in some cases) to use your software living on your servers (or, at least servers that you manage). I would certainly hope that someone who can deal with outages or penetrations is actually working on the weekend (perhaps that person doesn't work Monday and Tuesday?).

Would you say "eh, but it was the weekend" if an attacker purged your paid enterprise repo on a Sunday morning?

[freehunter]

No, it would still be an emergency, but the response time (unless guaranteed by a SLA) would likely vary. Does GitHub have a SLA?

[davyjones]

> Actually, the world _does_ stop turning because it's Sunday.

Incorrect. In Islamic countries, which are a part of this world, like Saudi Arabia and Oman have Thu-Fri weekend.

http://en.wikipedia.org/wiki/Workweek_and_weekend#Islamic_co...

[why-el]

Not all Muslim countries do though. All north african countries have a saturday/sunday weekend, except Egypt. and even in Egypt some companies make sure a subset of their employees work on their weekend because, well, most of their partners are working.

[freehunter]

I think it could be reasonably inferred that when someone says "the world stops turning", it's being used metaphorically. Kind of like "stop the presses".

[philwelch]

Playing nice with a hacker who just broke into your service shouldn't take priority over:

1. Making sure he doesn't continue breaking into your service (by suspending his account)

2. Fixing the security flaw he used to break into your service

3. Appraising your users to the situation.

I feel for the kid--he's just 18, and if he gets some good judgment to go along with his technical skill he'll go far. But I don't understand the nerdrage at Github. People trust Github's service and their software to protect proprietary code; their response has been everything you could hope for in the interests of 100%-1 of Github stakeholders, at the expense of not communicating well to Egor why and how long they were suspending him for breaking into their service.

[Lazare]

I agree with you up to a point; those certainly should be their priorities. However, suspending his account didn't help them accomplish any of those goals.

I think we can all agree that it certainly didn't help them fix the vulnerability or communicate with users, right? (Actually, it arguably did the reverse...) But it also did absolutely nothing to stop him from breaking into the service; the exploit works for any user, and Github allows anyone to create an account instantly and for free. Until the exploit was fixed, anyone including him could have created an account and exploited their service.

A better defence of Github would be that they couldn't have been expected to know that, and so they shouldn't be slammed for doing something unproductive and pointless that distracted them from the three core priorities you list above. And I agree! If something looks fishy, banning everyone involved, and sorting it out later is actually a pretty decent idea...even if (as here) it proves to be a complete waste of time in retrospect.

[icebraining]

How does suspending his account ensure he doesn't continue breaking into your service?

[timcosgrove]

Because it's trivial for him to set up another account and break the service from there. His ability to do this exploit wasn't tied to his specific account. His point was that anyone could be doing this.

=edit= Sorry, I am apparently agreeing with the comment I replied to.

[philwelch]

In retrospect, it wasn't that helpful unless they also suspended new account creation, or at least kept an eye on it. But they didn't have that information at that time.

During a security breach, is suspending a malicious user who's currently attacking you really something you sit around and discuss beforehand? Or do you just do it?

[Me1000]

I'm inclined to believe GitHub did exactly what they thought was best. That is to say anyone's knee jerk reaction would be to suspend the account as quickly as possible, for whatever good it might do... I can't fault them for what they did. When someone hacks you, you need to act very quickly because your customers are more important than your ego. I do believe they should provide incentive for responsible disclosure though.

Edit: spelling

[MartinMond]

I agree with you, but even if this was a reaction to public outcry the real reason to be disturbed is that top-notch Ruby devs like the GitHub guys didn't use attr_accessible. I can't wrap my mind around that.

That you have to use attr_accessible is known throughout the Rails community since "ever". Only toy apps don't use it.

It's like saving passwords in plaintext, only arguably even worse.

[technoweenie]

This is true. We actually don't use mass assignment that often. He happened to catch 2 our of 3 spots that still used it. Everywhere else is explicit about what to accept.

[MartinMond]

Ok, that is good to hear. Trust restored :) I was fearing you were using unsafe mass assignment everywhere...

Would you mind sharing any patters you use to DRY up explicit assignment?

[technoweenie]

We use this (posted by @dhh) https://gist.github.com/1975644 in some spots, or simply just Hash#slice. We have some other thoughts on making params access more explicit. The problem with explicit patterns is they can get left out if a developer forgets.

[petercooper]

Or more specifically, since the public keys objects require an associated user, the old chestnut:

    @user.public_keys.build(...)

.. where @user is retrieved in a role based manner (that is, you only get the right @user if you are authorized to get it.)

Ultimately, this is less an issue of mass assignment specifically and more an overarching one of allowing a user to perform an action in the guise of another. But, of course, these mistakes are commonly made by developers of all skill levels! :-) (me included)

[hitonagashi]

No, that's not entirely correct.

You can do @user.public_keys.find(params[:id]).update_attributes(:user_id => 25)

Its the mass assignment protection on foreign keys that prevents you assigning one of your public keys to someone else, ensuring the chain is correct doesn't necessarily help with this scenario.

[technoweenie]

We saw some weird stuff on the account that he wasn't upfront about. Though, our first priority was clearing the bug up.

[cdcarter]

> (1) the temporary nature of the suspension was not communicated to Egor at the onset of the situation, nor,

Are suspensions not by definition temporary?

[JumpCrisscross]

Sorry - updated. I was focusing more on the notion of GitHub suspending his account with the intention to re-instate it after clearing uncertainty. Their lack of communication with him is inconsistent with a rational reaction to that assumption.

[mvanveen]

Cut 'em a break. I sympathize with your statement, but I give Github a lot of credit for correcting their error and reinstating his account.

As a general rule, it's extremely hard for organizations to admit their own mistakes.

[hythloday]

I'm inclined to agree with you, but, to play the devil's advocate, "we're temporarily suspending you...oops, no, it's permanent after all" is not a position they want to find themselves in.

[nknight]

The usual phrase would be along the lines of "pending a full investigation", which is what they should have said, but people often fail to. PR is hard enough when you're calm and collected, in the middle of a shitstorm it's a miracle if you can fully articulate what is and will be happening.

[chrisrhoden]

The account was suspended, not deleted. That was made clear to everyone. Suspended does not mean banned, it does not mean canceled, it is impermanent by definition.

[einhverfr]

I am with the others who say that you don't want to be too transparent over security handling.

One thing we do in LSMB is to "declassify" security issues when we do a full disclosure (usually two weeks after the patch is released).

The idea is that we fix, patch, announce the patch, wait, then issue full disclosure.