7 ms·

GitHub's SAML implementation is useless. The idea is that you can bring your own account into an enterprise, and that sort of works on the site itself, but it d

by _cenw 2y ago

GitHub's SAML implementation is useless. The idea is that you can bring your own account into an enterprise, and that sort of works on the site itself, but it does not prevent apps where you log in with GitHub from reading your organization membership once you have authorized an app on the organization level (and if you didn't, it hides the membership from oauth tokens, so it has this capability!). A SAML session is only required if said app fetches data via a token obtained from that user - and in my glance around, this was almost never the case - SAST tools almost always use app instance tokens and are happy to show anyone with a GitHub account in your organization your code. Tailscale fixed this when I pointed it out, Sonarcloud told me to please don't tell anyone and GitHub took a few weeks to say this is totally expected behavior - when no vendor I told did, and their docs contradicted them. I swear, reporting security bugs is a thankless endeavor, even if you just randomly stumble over them. I couldn't imagine doing this as a job.

weard_beard 2y ago
This is the operating procedure at every conceivable level. You would not believe how difficult it is to convince young developers raised on Javascript that client side validation is not enough, much less the business owners setting out functional requirements and budgets.
- dboreham 2y ago
  [flagged]
  - weard_beard 2y ago
    They'd be fired if they spent a penny more fixing it.
  - UltraSane 2y ago
    Musk should be fired. He has irreversibly destroyed the Tesla brand.
    - woleium 2y ago
      But has he realized yet?
      - NewJazz 2y ago
        I wouldn't be surprised if the recent stock downturn is mostly him illegally offloading shares without disclosing it.
- cluckindan 2y ago
  ”You would not believe how difficult it is to convince young developers raised on Javascript that client side validation is not enough” At first read, I think you’re JSplaining, but I’m willing to give you the benefit of the doubt. How difficult is it exactly? Can you provide examples, perhaps even of the particular difficulties? Are the difficulties on the side of the convincer or the convincee, or both?
  - nextts 2y ago
    I think it is something they have to experience. Tell them if they are happy with it, give me a $10 bug bounty. Then go hack a deploy of their branch. Then tell em to keep the $10 but remember the lesson.
    - fn-mote 2y ago
      Wow. I would never guess it was so hard to convince someone of this. “The code I write doesn’t have XSS or SQL injection vulnerabilities,” sure. At least those are plausible things to believe. Client side validation?? How could anybody believe in that?
      - nine_k 2y ago
        I convinced fellow engineers who were adamant that the code they had written was OK by writing actual exploits against their code. Twice. Worked both times, without betting on money.
      - wglb 2y ago
        An axiom of secure programming is to never trust the client. You don't really know what the client is. Often it takes several penetrations via compromised/replaced clients to get the message through. Just look at all the discussions about why browser-based javascript encryption is problematic.
eCa 2y ago
> The idea is that you can bring your own account into an enterprise The issues goes beyond authorization. I’ve had Github randomly once in a blue use my personal email address as the default when merging a work PR. If anyone asks, I advice against mixing personal and professional stuff in the same Github account (or anywhere).
- PokestarFan 2y ago
  Why not just use the GitHub generated email address you get when you hide your email?
- l72 2y ago
  My company does not allow any employees to use their personal GitHub for work (or Facebook, instagram, or anything else) after running into issue when employees leave.
  - booi 2y ago
    Wouldn’t you just remove them from the org?
    - onionisafruit 2y ago
      They may decide to change their github login to <company_name>LIES, and suddenly that’s all over your old PRs and Issues. Including in public repos where customers go looking for help.
      - TheDong 2y ago
        That's even more true with a dedicated work github account than a mixed personal/work one; either way they can still login and edit the account name even if removed from the company org, and if it's not shared it doesn't burn their personal account too... right? Is this speaking from experience?
        deleted 2y ago
TZubiri 2y ago
the premise of comfort from shared credentials, and perhaps of increased security from sso; breaks down the moment you have a vulnerability like this. Any type of password store, even a physical one, or just reusing passwords, ends up being safer. Minimalism wins again
peterldowns 2y ago
To be fair to the vendors, Github makes it extremely difficult to do the right thing here. I built a repo/commit/pr-analysis tool (https://dev.log.xyz https://dev.log.xyz) and it took a lot of effort to make it so that "iff you can see it in Github you can see it in Devlog." The entire experience was beyond frustrating. Github also makes their OAuth permissions picker extremely confusing. When I "login with Github" I am never sure exactly what I'm sharing, from which organizations I'm a member of.
- _cenw 2y ago
  Yeah, it's a massive UX issue. The way to actually check if someone has a SAML session is to attempt to get their membership. If you get a 403, there isn't one. But good luck explaining to the user that they need to click "authorize" next to the organization in the OAuth flow. No way to send a hint that it may be required, and no way to do a step-up flow. I did a full writeup here: https://notes.acuteaura.net/posts/github-enterprise-security/ https://notes.acuteaura.net/posts/github-enterprise-security...
- wutwutwat 2y ago
  > Github makes it extremely difficult to do the right thing here ... it took a lot of effort to make it so that "iff you can see it in Github you can see it in Devlog." The entire experience was beyond frustrating. Do they? You don't have to mess with syncing teams, memberships, or assignment to repos if you don't want to. You can make one api call: > The authenticated user has explicit permission to access repositories they own, repositories where they are a collaborator, and repositories that they can access through an organization membership. https://docs.github.com/en/rest/repos/repos?apiVersion=2022-11-28#list-repositories-for-the-authenticated-user https://docs.github.com/en/rest/repos/repos?apiVersion=2022-...
  - _cenw 2y ago
    I should've tested this endpoint. GitHub's SAML implementation is done by a different team, always lags behind in quality and does some pretty unclean patching of the data - i.e. the notification filtering is done in the templating engine, so if all your notifications are SAML gated you get the header, no "all caught up" below it and (this is live from my account) "1-0 of 113". So I'd give it about a 50:50 chance of working. Edit: I just realized it eats your non-gated notifications too, if they're further down than position 25, and the "Next" button just leads to the same page with "?query=". Yay, another ticket about how glued on GitHub Enterprise Cloud is. The last one (GitHub eats API calls to accept invites to SAML organizations, deletes the invite, and sends a 200, writes success to the audit log... but ends up being a no-op) only has been 2 months or so ago. Thanks Microsoft.
deleted 2y ago
[deleted]
fulafel 2y ago
For anyone trying to connect the above to this vuln research, this seems unrelated ("GitHub doesn’t currently use ruby-saml for authentication, but began evaluating the use of the library with the intention of using an open source library for SAML authentication once more")

by _cenw 2y ago

weard_beard 2y ago

This is the operating procedure at every conceivable level. You would not believe how difficult it is to convince young developers raised on Javascript that client side validation is not enough, much less the business owners setting out functional requirements and budgets.

dboreham 2y ago
[flagged]
- weard_beard 2y ago
  They'd be fired if they spent a penny more fixing it.
- UltraSane 2y ago
  Musk should be fired. He has irreversibly destroyed the Tesla brand.
  - woleium 2y ago
    But has he realized yet?
    - NewJazz 2y ago
      I wouldn't be surprised if the recent stock downturn is mostly him illegally offloading shares without disclosing it.
cluckindan 2y ago
”You would not believe how difficult it is to convince young developers raised on Javascript that client side validation is not enough” At first read, I think you’re JSplaining, but I’m willing to give you the benefit of the doubt. How difficult is it exactly? Can you provide examples, perhaps even of the particular difficulties? Are the difficulties on the side of the convincer or the convincee, or both?
- nextts 2y ago
  I think it is something they have to experience. Tell them if they are happy with it, give me a $10 bug bounty. Then go hack a deploy of their branch. Then tell em to keep the $10 but remember the lesson.
  - fn-mote 2y ago
    Wow. I would never guess it was so hard to convince someone of this. “The code I write doesn’t have XSS or SQL injection vulnerabilities,” sure. At least those are plausible things to believe. Client side validation?? How could anybody believe in that?
    - nine_k 2y ago
      I convinced fellow engineers who were adamant that the code they had written was OK by writing actual exploits against their code. Twice. Worked both times, without betting on money.
    - wglb 2y ago
      An axiom of secure programming is to never trust the client. You don't really know what the client is. Often it takes several penetrations via compromised/replaced clients to get the message through. Just look at all the discussions about why browser-based javascript encryption is problematic.

eCa 2y ago

> The idea is that you can bring your own account into an enterprise The issues goes beyond authorization. I’ve had Github randomly once in a blue use my personal email address as the default when merging a work PR. If anyone asks, I advice against mixing personal and professional stuff in the same Github account (or anywhere).

PokestarFan 2y ago
Why not just use the GitHub generated email address you get when you hide your email?
l72 2y ago
My company does not allow any employees to use their personal GitHub for work (or Facebook, instagram, or anything else) after running into issue when employees leave.
- booi 2y ago
  Wouldn’t you just remove them from the org?
  - onionisafruit 2y ago
    They may decide to change their github login to <company_name>LIES, and suddenly that’s all over your old PRs and Issues. Including in public repos where customers go looking for help.
    - TheDong 2y ago
      That's even more true with a dedicated work github account than a mixed personal/work one; either way they can still login and edit the account name even if removed from the company org, and if it's not shared it doesn't burn their personal account too... right? Is this speaking from experience?
      - deleted 2y ago

TZubiri 2y ago

the premise of comfort from shared credentials, and perhaps of increased security from sso; breaks down the moment you have a vulnerability like this. Any type of password store, even a physical one, or just reusing passwords, ends up being safer. Minimalism wins again

peterldowns 2y ago

To be fair to the vendors, Github makes it extremely difficult to do the right thing here. I built a repo/commit/pr-analysis tool (https://dev.log.xyz https://dev.log.xyz) and it took a lot of effort to make it so that "iff you can see it in Github you can see it in Devlog." The entire experience was beyond frustrating. Github also makes their OAuth permissions picker extremely confusing. When I "login with Github" I am never sure exactly what I'm sharing, from which organizations I'm a member of.

_cenw 2y ago
Yeah, it's a massive UX issue. The way to actually check if someone has a SAML session is to attempt to get their membership. If you get a 403, there isn't one. But good luck explaining to the user that they need to click "authorize" next to the organization in the OAuth flow. No way to send a hint that it may be required, and no way to do a step-up flow. I did a full writeup here: https://notes.acuteaura.net/posts/github-enterprise-security/ https://notes.acuteaura.net/posts/github-enterprise-security...
wutwutwat 2y ago
> Github makes it extremely difficult to do the right thing here ... it took a lot of effort to make it so that "iff you can see it in Github you can see it in Devlog." The entire experience was beyond frustrating. Do they? You don't have to mess with syncing teams, memberships, or assignment to repos if you don't want to. You can make one api call: > The authenticated user has explicit permission to access repositories they own, repositories where they are a collaborator, and repositories that they can access through an organization membership. https://docs.github.com/en/rest/repos/repos?apiVersion=2022-11-28#list-repositories-for-the-authenticated-user https://docs.github.com/en/rest/repos/repos?apiVersion=2022-...
- _cenw 2y ago
  I should've tested this endpoint. GitHub's SAML implementation is done by a different team, always lags behind in quality and does some pretty unclean patching of the data - i.e. the notification filtering is done in the templating engine, so if all your notifications are SAML gated you get the header, no "all caught up" below it and (this is live from my account) "1-0 of 113". So I'd give it about a 50:50 chance of working. Edit: I just realized it eats your non-gated notifications too, if they're further down than position 25, and the "Next" button just leads to the same page with "?query=". Yay, another ticket about how glued on GitHub Enterprise Cloud is. The last one (GitHub eats API calls to accept invites to SAML organizations, deletes the invite, and sends a 200, writes success to the audit log... but ends up being a no-op) only has been 2 months or so ago. Thanks Microsoft.

deleted 2y ago

[deleted]

fulafel 2y ago

For anyone trying to connect the above to this vuln research, this seems unrelated ("GitHub doesn’t currently use ruby-saml for authentication, but began evaluating the use of the library with the intention of using an open source library for SAML authentication once more")