Supporting multiple SSO secrets with Discourse as provider

Feature
,
1

It would be nice to allow the Discourse SSO provider to allow multiple secrets.

Use cases (yeah, I know the first two are kind of the same):

  • allow use of a testing setup of an auth-proxy against a production Discourse install without risking the exposure of the production secret
  • allow the authentication of a service managed by an external entity without leaking the production secret
  • allow the Discourse SSO Consumer secret to differ from the Provider secret (would anyone do both at once?)

Two options immediately come to mind:

  • have Discourse sign the SSO payload both with key k and hash(k+consumer.hostname)
    • backwards compatible, allows a SSO consumer that expects such a thing to check both signatures
    • presumably does not leak key k to the consumer since the consumer need only know hash(k+consumer.hostname)
    • probably easy to implement
  • build into Discourse explicit support for differing keys k1, k2, etc. per (set/glob of hostnames)
    • would be easier for multiple services all using a single key
    • more work to implement
    • probably the “better” way to do it :smiley:
4 Likes
2

I am quite a fan of key agility, and this provides a mechanism to do that (at least the “do it properly” approach). I think the quick hack method is a really bad idea, for several reasons:

  • You’d presumably have to be sending out both signatures all the time, which is wasteful for all those clients not requiring the extra signature;
  • k+consumer.hostname is equivalent to k in any circumstance in which the attacker can influence the value of consumer.hostname.
  • Conversely, even getting agreement between consumer and provider on what consumer.hostname even is can be… an adventure, and the ways that are typically easier to do are also more amenable to attacker influence.
  • The construction hash(k+consumer.hostname) would have to be carefully evaluated against possible attacks, which is a non-trivial exercise. There are extremely good reasons why HMAC(K, m) is defined as H((K' xor opad) || H((K' xor ipad) || m)), and not just H(K || m). Cryptography Is Hard.
4 Likes
3

I think it is a bit overdue to make sso_secret single use vs “multi use” where it works both for incoming and outgoing SSO.

If we are cleaning this up I think we should probably clean it up all the way. Add a site setting for sso_provider_secrets which contains a list of pairs:

www.somewhere.com|SOME_LONG_SECRET
*.discourse.cloud|SOME_OTHER_LONG_SECRET

Etc… @joffreyjaffeux new control makes creating settings like this super easy :heart: but we would need a special mode where it hides the secrets in the UX like we do for other passwordish fields in site settings.

Then we also get extra security here cause even if a key leaks out it can only be used on the “domain” you allowed it so it will simply (post login) redirect to a non-hijacked URL which would be confused about the whole thing cause a NONCE would not check out.

Overall there is not too much work in this change and I totally support it cause it makes SSO provider support way cleaner and will force people using this feature to do a nice audit.

6 Likes
6

pretty easy to do now that I introduced “list_type” in yml setting

6 Likes
7

FTR, this is being worked on in

https://github.com/discourse/discourse/pull/6431

4 Likes
8

This has been merged, thanks @maja!

3 Likes
9

This topic was automatically closed after 32 hours. New replies are no longer allowed.

10

@maja was looking to it on mobile and it could use some love. Nothing urgent and critical as we don’t aim for perfection on mobile admin. But it should be fairly easy to fix.

4 Likes