{"action":"created","issue":{"url":"https://api.github.com/repos/atmos/warden-github/issues/43","labels_url":"https://api.github.com/repos/atmos/warden-github/issues/43/labels{/name}","comments_url":"https://api.github.com/repos/atmos/warden-github/issues/43/comments","events_url":"https://api.github.com/repos/atmos/warden-github/issues/43/events","html_url":"https://github.com/atmos/warden-github/pull/43","id":53712407,"number":43,"title":"Make token interception more difficult","user":{"login":"atmos","id":38,"avatar_url":"https://avatars.githubusercontent.com/u/38?v=3","gravatar_id":"","url":"https://api.github.com/users/atmos","html_url":"https://github.com/atmos","followers_url":"https://api.github.com/users/atmos/followers","following_url":"https://api.github.com/users/atmos/following{/other_user}","gists_url":"https://api.github.com/users/atmos/gists{/gist_id}","starred_url":"https://api.github.com/users/atmos/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/atmos/subscriptions","organizations_url":"https://api.github.com/users/atmos/orgs","repos_url":"https://api.github.com/users/atmos/repos","events_url":"https://api.github.com/users/atmos/events{/privacy}","received_events_url":"https://api.github.com/users/atmos/received_events","type":"User","site_admin":false},"labels":[],"state":"closed","locked":false,"assignee":null,"milestone":null,"comments":10,"created_at":"2015-01-08T04:01:38Z","updated_at":"2015-11-11T12:41:15Z","closed_at":"2015-01-13T04:56:54Z","pull_request":{"url":"https://api.github.com/repos/atmos/warden-github/pulls/43","html_url":"https://github.com/atmos/warden-github/pull/43","diff_url":"https://github.com/atmos/warden-github/pull/43.diff","patch_url":"https://github.com/atmos/warden-github/pull/43.patch"},"body":"Right now it's fairly easy to intercept or mitm a rack session cookie and grab the user's oauth token from it. If you inspect the request headers for a browser session and look at the `rack.session` cookie you can pretty trivially get the token with something like this.\r\n\r\n```ruby\r\nrequire 'cgi'\r\nrequire 'base64'\r\nrequire 'pp'\r\nrequire 'ostruct'\r\n\r\ndef unmarshal(raw)\r\n  begin\r\n    Marshal.load raw\r\n  rescue ArgumentError => e\r\n    klass = e.message.split(' ').last.sub(/:+$/, '')\r\n    klass.split('::').inject(Object) do |parent, k|\r\n      parent.const_defined?(k) ? parent.const_get(k) : parent.const_set(k, Class.new(OpenStruct))\r\n    end\r\n    unmarshal raw\r\n  end\r\nend\r\n\r\nSTDOUT.write \"Enter a session cookie: \"\r\ncookie = STDIN.gets.chomp\r\ndecoded = Base64.decode64(CGI.unescape(cookie.split('--').first))\r\nunmarshaled = unmarshal decoded\r\n\r\nPP.pp unmarshaled\r\n```\r\n\r\nThere's a couple of questions for how this gets adopted. Do we make a new warden config option for it? Conditionally enable it if an environmental variable is set? Make it a breaking required change in order to get future support? Are there any downsides to requiring it?\r\n\r\n@fphilipe Any drawbacks you can see bubbling this up to warden-github-rails?\r\n@mastahyeti is this a correct fix to what we discussed previously around this issue? I can't extract the sessions anymore with burp."},"comment":{"url":"https://api.github.com/repos/atmos/warden-github/issues/comments/155766427","html_url":"https://github.com/atmos/warden-github/pull/43#issuecomment-155766427","issue_url":"https://api.github.com/repos/atmos/warden-github/issues/43","id":155766427,"user":{"login":"mastahyeti","id":1144197,"avatar_url":"https://avatars.githubusercontent.com/u/1144197?v=3","gravatar_id":"","url":"https://api.github.com/users/mastahyeti","html_url":"https://github.com/mastahyeti","followers_url":"https://api.github.com/users/mastahyeti/followers","following_url":"https://api.github.com/users/mastahyeti/following{/other_user}","gists_url":"https://api.github.com/users/mastahyeti/gists{/gist_id}","starred_url":"https://api.github.com/users/mastahyeti/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/mastahyeti/subscriptions","organizations_url":"https://api.github.com/users/mastahyeti/orgs","repos_url":"https://api.github.com/users/mastahyeti/repos","events_url":"https://api.github.com/users/mastahyeti/events{/privacy}","received_events_url":"https://api.github.com/users/mastahyeti/received_events","type":"User","site_admin":true},"created_at":"2015-11-11T12:41:15Z","updated_at":"2015-11-11T12:41:15Z","body":"I don't see a big problem with passing the class name along. \r\n\r\nReading back over this PR though, I'm wondering if we meant to use MessageEncrypter instead of MesageVerifier though. MessageVerifier doesn't prevent someone without the secret from decoding the value, it just prevents it from being tampered with. "}}