Security flaws in an SSO plugin for Caddy (2023)

(blog.trailofbits.com)

123 points | by mooreds 165 days ago

10 comments

  • jrpelkonen 165 days ago
    Using math/rand seeded with a timestamp to generate crypto materials does not instill high confidence in the skills of the team. But their lack of interest in addressing the issues raised here is perhaps even more worrying.

    If I was a user of this plugin, I’d start looking for alternatives pronto.

    • mholt 164 days ago
      To be clear, "the team" you refer to is one burnt-out developer (and not the Caddy maintainer team). Having been through burnout myself several times, I can understand the lack of response.

      I'm sure the project would benefit greatly if the community and companies who use it could bolster it up.

      • lifthrasiir 164 days ago
        I do feel the pain as well, but the OP specifically says "confirmed" (i.e. there indeed was a response) without no further qualification. If the plugin author did say the underlying reason, I believe the OP should have reproduced the reason as well because it's an even bigger security risk in my opinion.
    • devwastaken 164 days ago
      Ripe for a PR.
  • kalev 164 days ago
    Huge credits to the Caddy team, I’ve been using Caddy for all of my production projects over 4+ years and it’s absolutely rock solid. Never had an issue with it. Unfortunate a 3rd party plugin here is causing fallout by other HNers dumping their negative remarks here. Still, a security plugin which seems to have holes in it is a bit awkward. I’m sure someone will pick up what’s needed to be done.
    • ThePhysicist 164 days ago
      Yeah, occasional security issues are part of the game and nothing to be ashamed of, on the other hand Caddy's headline is "makes your sites more secure, more reliable, and more scalable than any other solution". Maybe they should cut back one the hyperbole a bit.

      Seems a bit like the situation with Wordpress, the core is pretty solid security-wise but the third-party ecosystem isn't as well-tested or maintained.

      • allendoerfer 164 days ago
        The WordPress core is definitely not rock solid. It is shipping abandoned versions of third-party plugins with it (like TinyMCE), has horrible coding standards and to this day does not use prepared statements.

        It is focused on backwards compatibility and simply does not prioritize security very much. All claims about “okay security in the core” come from having even worse plugins, it making a good story and simply an enormous user base.

      • hhh 164 days ago
        Why should they cut back on their offerings because of a third party plugin?
        • ThePhysicist 164 days ago
          They can do what they want of course, for me it just sounds like hyperbole. People seem to be unable to give a realistic description of their software, everything always has to be the fastest, most scalable, most secure. Personally it's a red flag especially in regards to security as it shows a cavalier attitude. Using e.g. math/rand instead of crypto/rand is a really basic mistake and something that any decent security reviewer would flag immediately. Trusting headers that can be spoofed is also one of the most basic attack vectors that e.g. a penetration tester will try out when seeing that a server makes use of header information for a security-critical code path. I mean people use this stuff in production to secure personal and other sensitive data, and they do so because the website literally tells them "it's the most secure solution out there". I don't think most people get that the plugins are not part of the official distribution or have a different, much lower standard in terms of engineering security. That's the core issue for me, it's fine if your contributors are burnt out and don't have time to fix security issues, it's not day their job, but then stop telling people your solution is the best, most secure solution out there.

          And if they want to support people then pay them, Caddy is owned by a large company so they should be able to pay maintainers for their most security-critical plugins. I really don't want to be too harsh, it's a great piece of software, but I'm tired of this marketing tendency to wildly exaggerate capabilities and properties of software systems.

          • mholt 164 days ago
            Caddy isn’t owned by a large company. They can barely pay me to work on it full time. I’m striving to up our sponsorships to be able to pay others but right now our team is volunteers.

            And this plugin isn’t built or maintained by our core team. It’s third party.

            I stand by our marketing.

            • hhh 164 days ago
              Thank you for the context! I should have verified this information before aligning.

              I went and sponsored a small monthly pledge as a happy Caddy user, and submitted it to my work's sponsorship team.

              • mholt 164 days ago
                Thank you for your support! We'll make sure it gets put to good use.
          • hhh 164 days ago
            I appreciate the context.

            > Caddy is owned by a large company so they should be able to pay maintainers for their most security-critical plugins.

            this is the most critical part to me, onboarding your highest used plugins is probably a good idea.

            • francislavoie 164 days ago
              We definitely don't have the resources to do that. Matt receives some funding from ZeroSSL which helps cover his expenses, and the rest is from sponsorships https://github.com/sponsors/mholt. It's not yet enough to hire any help. That's something Matt would like to do though, as soon as enough companies sponsor.

              In fact, while I'm a top contributor, I don't receive anything for my work. I do it as a volunteer. Matt has told me repeatedly he'd like to pay me for my efforts, but I have a full time job already and Caddy is a hobby for me, so I'm not looking to get paid.

              But either way, we wouldn't onboard a plugin with such a wide scope as caddy-security. It would take way too much effort for us, and it would bloat the standard distribution with features only a small part of the userbase would need. We're already spread thin as it is, our time is best spent improving and maintaining the existing core feature set.

  • rrdharan 165 days ago
    > August 23, 2023: The caddy-security plugin maintainers confirmed that there were no near-term plans to act on the reported vulnerabilities.

    Ouch. I was considering switching from oauth2_proxy to Caddy + SSO but this is pretty discouraging..

    • mholt 164 days ago
      My understanding is Paul has been burnt out from his FAANG job. I'm sure the project could benefit from a little bolstering from the community or companies who rely on it.
    • mdeeks 165 days ago
      Discouraging? Huge understatement. An auth plugin that refuses to fix multiple high severity vulnerabilities is wild. No one should use this plugin for anything. If I were the Caddy authors I would remove all references to this project and ask that they stop use of the Caddy name.
      • tadfisher 164 days ago
        There is no "refusal" as far as I can tell. The issues were reported [1] in September 2023 (as was this blog post) and the simplest one has been fixed (insecure random seed). I'm not aware of any public statements from the plugin maintainers, and there is no hostility in the issue comments.

        [1]: https://github.com/greenpau/caddy-security/issues?q=is%3Aiss...

        • bbarnett 164 days ago
          If there isn't a refusal, fair enough. Yet leaving high impact, publicly disclosed security issues unresolved for 6 months means the code is abandoned.

          I'll consoder stable code fine without new releases for years even, but security issues? Responses and fixes need to be measured in days, not months.

          (Some may say "but what about"..., and note all rhe conditions I listed above. Publicly disclosed + timefame. Doesn't matter who it is, big or small, 6 months means the software maintainers show no serious commitment to security, and one should run to other solutions.)

      • Aeolun 165 days ago
        I mean, do you feel you have a right to get those fixes for free? I'm sure you are welcome to contribute them.
        • nixgeek 164 days ago
          No, but I'd also hope that new and existing users of what's probably a popular middleware for Caddy aren't expected to read 100's of open GitHub Issues or luck upon the Trail of Bits report to know there are significant unresolved security issues and the maintainers don't have the time or resources to actually fix them.

          Better know that up front, than get popped and think "What happened?".

        • tgsovlerkhgsel 164 days ago
          At the very least, there should be a clear warning not to use the current version of the software. And I think it's reasonable to suggest that the main project authors should neither advertise software that's dangerously insecure nor allow it to use the main project's name (which could mislead people into assuming that it's part of the project and similarly well maintained).
          • Aeolun 164 days ago
            > which could mislead people into assuming that it's part of the project and similarly well maintained

            That’s entirely on the idiot that assumes a similarity in name means the project is maintained by the same owner.

        • lopkeny12ko 164 days ago
          I don't understand this attitude.

          If you put your work in the open, advertise it as an officially supported solution, and advertise it as secure, it had better be supported and secure. You can't just put out broken software and hide behind "but you didn't pay for it so who cares." Even worse when the attitude is "it's not my fault, go fix it yourself and submit a PR."

          If that is the intent, why even open source it in the first place and tell people to use it. Just keep the code private and use it for yourself.

          Over the last decade I have written hundreds of thousands of lines of code for personal projects that will never see the light of day, precisely for this reason. It's not production-quality software, and it would be horrendously irresponsible for me to put it out in the open, advertise it, and tell people to use it, compromising the security of their homelabs (or worse, enterprise deployments).

          • selckin 164 days ago
            you're describing proprietary software with a full support contract.

            the point of free software(tm) is gaining benefit from cooperation and community, and nobody is obligated to do anything.

  • yogorenapan 165 days ago
    I took a look around and the plug-in doesn’t seem to be actively maintained. Most commits within the last year has been simple dependency version bumps.

    Wouldn’t use it even if not for these security issues.

    • mholt 164 days ago
      The project could definitely use some help from the community, or financial support. I believe Paul may be open to opportunities there. I know his current job has kind of burnt him out.
  • positr0n 165 days ago
    It's this the most popular/common Caddy plugin for SSO?
  • mholt 164 days ago
    Mods: This should probably should have a [2023] appended to the title
  • yonixwm 164 days ago
    Why is it the plugin job to mitigate IP Spoofing via X-Forwarded-For Header? Isn’t it the first gateway job? Just like I wouldn’t blame a container server for cdn not making sure X-Forwarded-For is not passed from the client…

    Meaning how will the plugin know how levels deep is it in the infrastructure… and if you really care about warning devs then it should be a caddy level security bug, not this plugin specifically.

    So all those spoofing items is really out of scope for this plugin…

    • francislavoie 164 days ago
      Caddy itself has built-in support for correctly & safely parsing XFF via the trusted_proxies global option https://caddyserver.com/docs/caddyfile/options#trusted-proxi... The plugin should use the parsed client IP produced by this, but the plugin was written before trusted_proxies existed in Caddy. (Not excusing the mistake, but there's an "easy" solution if someone wants to resolve it.)
      • yonixwm 164 days ago
        Again why mention the plugin? Just use the main caddy option… plugins are ment to expand caddy not replace existing directives

        unless i am missing something and the plugin overrides default behavior then of course it’s to blame

    • buro9 164 days ago
      I think a security plugin author should have the awareness that X-Forwarded-For is just a string of untrusted user input.

      That definitely is the job of a security plugin author who uses the value.

      Whether it's lesson number one or number two... Don't trust your inputs has got to be near the top of things you learn when doing anything with security.

      Edit: now I've read the article... Shocked. The vast majority of this is trusting untrusted user input.

  • HumanOstrich 164 days ago
    [flagged]
    • shp0ngle 164 days ago
      Enforcing gofmt style is standard in basically all go projects. And it's for "free", you can fix it with one command, I don't see a problem here.
      • HumanOstrich 164 days ago
        Caddyfiles are not Go files and my IaC project is not a Go project. You know which projects don't force their personal preferences for tabs vs spaces on my codebase? TypeScript, nginx, Docker, MySQL, Grafana, Terraform, Bash, firewalld, ssh... Dang it's like all of them. Oh except Makefiles, you got me there.
        • shp0ngle 164 days ago
          ahh I misunderstood you there. I don't know enough about Caddy to elaborate here.
    • francislavoie 164 days ago
      To be clear, this is about a third party plugin of Caddy, not about Caddy itself. And the author of the plugin is burnt out by their FAANG day job.

      I wouldn't call tabs vs spaces "personal style preferences". Caddy is written in Go, and we adopted the idea of "caddy fmt" from "gofmt". And warnings are warnings, not errors. They're soft reminders that you can safely ignore.

      • HumanOstrich 164 days ago
        [flagged]
        • francislavoie 164 days ago
          For the purposes of preservation, since you deleted two of your comments and made a new reply: https://i.imgur.com/n7ckJnT.png

          I'm not sure why you're taking an aggressive tone. Of course the Caddyfile is not Go, but I'm saying it's inspired by it, and we made a design decision in alignment with the project's relationship with Go.

          • rat9988 164 days ago
            Why did you post a picture of a deleted post. Op doesn't wish to share it anymore. You could have respected his wish.
            • francislavoie 164 days ago
              Because they were trying to gaslight me. They edited their original post with simply "." and then deleted their reply to mine, and made a new reply. Then after I posted the screenshot, they reverted their original post and added a rude comment as an edit.
              • rat9988 164 days ago
                Thank you for the context. Admittedly I can see now why you did it.
  • Danielben 164 days ago
    [flagged]