SAML authentication backend #1017

ejholmes · 2016-10-20T08:46:37Z

This adds support for SAML authentication. Empire is Enterprise ready.

I highly recommend that you go through this commit by commit (and probably ignore the first commit):

First commit just adds the crewjam/go-xmlsec dependency, which is just a go wrapper around libxmlsec for doing xmldsig verifications.
Second commit adds pkg/saml, which provides a library for implementing a SAML provider. It's essentially a fork of crewjam/saml with the code for implementing an IdP removed.
Third commit adds the /saml/acs endpoint for handing SAML assertions from the IdP.

Consider this mostly a POC at this point. There's still a lot of polish needed, but wanted to open this up now to get some eyes on it. I'd love to have someone that's had more experience implementing a SAML service provider to take a look and see if there's anything obviously wrong with the implementation.

Caveats

Because SAML is inherently browser based, this currently does not work with emp login. We can follow up with a OneLogin extension that uses the Generate SAML Assertion endpoint to support emp login. To login, you start with an IdP initiated login (e.g. clicking the app button in OneLogin) and you get presented with some copy/paste instructions to setup ~/.netrc with an API token.
Because SAML is insane, this currently introduces a dependency on libxml2 and libxmlsec1. This means you have to brew install some dependencies for tests to pass. I don't see any realistic way around this with Go.
Currently, SLO (single logout) isn't supported, nor any authorization checks post token generation. It should probably respect some TTL in the SAML assertion and expire tokens after that amount of time.

Overall, I'm pretty excited about this, since it opens up Empire to a lot of existing authentication providers, makes it easier to control access to Empire, solves the canonical employee problem, and it'll make implementing the Duo integration very simple for us.

Closes #1016

TODO

Update dev instructions with brew install stuff.
Docs on configuring SAML.
Integration tests exercising SAML.
Expire tokens.
Ensure that fake auth backend isn't added to chain.
Show a better error message when request isn't authenticated (emp login doesn't work).
Sign relay state tokens.

phobologic

Totally a partial review - got pulled off on other things, I'll try to get back to this.

phobologic · 2016-10-20T16:57:07Z

cmd/empire/main.go

+				Name:   FlagSAMLMetadata,
+				Value:  "",
+				Usage:  "The location of the SAML metadata XML (e.g. https://app.onelogin.com/saml/metadata/1234)",
+				EnvVar: "EMPIRE_SAML_METADATA",


Maybe add _URL to the end of this, the env var would be a lot more clear at first glance that way.

Oh, it looks like this might not be a URL sometime?

Right. This can take the raw content, a url, or a file path to the content. I'll update the flag usage to reflect that.

ejholmes · 2016-10-21T07:38:00Z

Added a commit to this to address some of the todo's in the PR description:

emp login is disabled if the SAML authentication backend is used. The error message here could be more friendly, and include a link to SSO.
Access tokens created via SAML authentication expire before SessionNotOnOrAfter (for us, this is 24 hours). It's kinda lame that you would have to re-authenticate every 24 hours, but I don't see a way around this with SAML. If someone gets removed from the IdP, then we'd want to make sure that access tokens expire soon after. What do other SAML service providers do to handle long lived tokens? AWS only allows up to 12 hours, using STS.
emp login requests the UsernamePassword authentication strategy, so that you can't use it to continuously refresh tokens.

ejholmes · 2016-10-25T04:12:22Z

Added another commit to improve the UX around logging in. If SAML authentication is enabled, then the error returned includes a link to start an SP initiated login:

$ emp apps
error: Request not authenticated, API token is missing, invalid or expired. Login at https://example.org/saml/login

And the instructions include emp logout to ensure that the old invalid token gets removed from ~/.netrc.

I still think following up with a onelogin extension that uses the Generate SAML assertion API would be good, but running into various issues with their API and their support is slow...

I think this just needs some integration tests exercising SAML login, and docs on how to configure the SAML backend, then it should be good to go. I'll probably put this up in staging under a separate domain so we can test it.

ejholmes · 2016-10-25T07:26:06Z

Added a commit with docs on configuring the SAML authentication backend.

This adds support for expiring auth tokens, and improves security around `emp login`: 1. You can only obtain an access token from `emp login` if you're providing username/password. This prevents the possibility of creating an long lived access token from a token that would expire. 2. `emp login` is disabled when SAML authentication is used.

ejholmes · 2016-10-25T23:31:18Z

I just re-wrote the commits, so the first 3 are only changes to vendor/ and pkg/ and the rest are the changes to Empire to support SAML.

This diff might be the easiest way to review: 3871b12...saml

ejholmes · 2016-10-26T08:35:07Z

And, just added one more commit that integration tests a full SAML service provider initiated login flow.

taylorann · 2016-10-27T00:26:33Z

While this may seem like a good idea in theory, in practice I can't carry my phone around with me everywhere because lady pockets suck. Please refactor. 💁🏻

phobologic

Not a whole lot for me to add - there's a lot of new functionality here. Seems useful to give a go in staging at least and see how it works out.

phobologic · 2016-10-26T23:59:18Z

cmd/empire/main.go

@@ -82,6 +88,12 @@ var Commands = []cli.Command{
 		Usage:     "Run the empire HTTP api",
 		Flags: append([]cli.Flag{
 			cli.StringFlag{
+				Name:   FlagURL,
+				Value:  "",
+				Usage:  "That base URL where this Empire instance runs",


Is this required? If not always, when is it? Might be worth adding to the Usage.

It's only required if the saml authentication backend is used. I'll update this to error out if the saml backend is used, but this isn't provided.

phobologic · 2016-10-26T23:59:45Z

cmd/empire/main.go

@@ -98,6 +110,30 @@ var Commands = []cli.Command{
 				EnvVar: "EMPIRE_SCHEDULER",
 			},
 			cli.StringFlag{
+				Name:   FlagServerAuth,
+				Value:  "",
+				Usage:  "The authentication backend to use to authenticate requests to the API. Can be `fake`, `github`, or `saml`.",


Is there a default?

Previously, we've defaulted to fake, which was determined by the absence of EMPIRE_GITHUB_CLIENT_ID.

For backwards compatibility, I went with leaving this blank by default. If it's blank, then it'll use the existing behavior and look at EMPIRE_GITHUB_CLIENT_ID. We could break backwards compatibility here and error out if it's not explicitly provided, which I'd be ok with.

ejholmes · 2016-10-28T23:25:14Z

I'll go ahead and merge this into master, and will follow up with any bug fixes and/or UX improvements separately.

Add go-xmlsec dependency

36d8896

ejholmes force-pushed the saml branch from 205f962 to 3a0e4c4 Compare October 20, 2016 08:59

phobologic reviewed Oct 20, 2016

View reviewed changes

ejholmes force-pushed the saml branch 2 times, most recently from 7b2ffc1 to 3b241a2 Compare October 21, 2016 07:30

ejholmes force-pushed the saml branch 5 times, most recently from e5c9b62 to b157776 Compare October 25, 2016 04:06

ejholmes added 9 commits October 25, 2016 16:24

Add remind101/empire/pkg/jwt

db539af

Implement pkg/saml

3871b12

Add SAML support

74cb83d

Make configuration of authentication backed more explicit.

3924ed5

Add SessionNotOnOrAfter to AuthnStatement.

171636c

Support service provider initiated login.

9c74105

Docs on configuring the SAML authentication backend.

8e88496

Update CHANGELOG

f019b87

ejholmes force-pushed the saml branch from fb36ae4 to f019b87 Compare October 25, 2016 23:24

ejholmes force-pushed the saml branch 2 times, most recently from f786ca2 to 0e378cd Compare October 26, 2016 08:29

Integration test full SAML service provider initiated login.

1c639eb

ejholmes force-pushed the saml branch from 0e378cd to 1c639eb Compare October 26, 2016 23:55

Fix uriContentOrValue when maybeURI is not a URI.

c652589

phobologic approved these changes Oct 27, 2016

View reviewed changes

Update dev instructions to install libxmlsec

e33096a

ejholmes merged commit 3fadb7a into master Oct 28, 2016

ejholmes deleted the saml branch October 28, 2016 23:25

ejholmes mentioned this pull request Nov 1, 2016

Onelogin authentication backend #1018

Closed

ejholmes mentioned this pull request Dec 9, 2016

Error reporter defaults to /dev/null #1010

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

SAML authentication backend #1017

SAML authentication backend #1017

ejholmes commented Oct 20, 2016 •

edited

Loading

phobologic left a comment

phobologic Oct 20, 2016

phobologic Oct 20, 2016

ejholmes Oct 20, 2016

ejholmes commented Oct 21, 2016 •

edited

Loading

ejholmes commented Oct 25, 2016

ejholmes commented Oct 25, 2016

ejholmes commented Oct 25, 2016

ejholmes commented Oct 26, 2016

taylorann commented Oct 27, 2016

phobologic left a comment

phobologic Oct 26, 2016

ejholmes Oct 27, 2016

phobologic Oct 26, 2016

ejholmes Oct 27, 2016

ejholmes commented Oct 28, 2016

SAML authentication backend #1017

SAML authentication backend #1017

Conversation

ejholmes commented Oct 20, 2016 • edited Loading

phobologic left a comment

Choose a reason for hiding this comment

phobologic Oct 20, 2016

Choose a reason for hiding this comment

phobologic Oct 20, 2016

Choose a reason for hiding this comment

ejholmes Oct 20, 2016

Choose a reason for hiding this comment

ejholmes commented Oct 21, 2016 • edited Loading

ejholmes commented Oct 25, 2016

ejholmes commented Oct 25, 2016

ejholmes commented Oct 25, 2016

ejholmes commented Oct 26, 2016

taylorann commented Oct 27, 2016

phobologic left a comment

Choose a reason for hiding this comment

phobologic Oct 26, 2016

Choose a reason for hiding this comment

ejholmes Oct 27, 2016

Choose a reason for hiding this comment

phobologic Oct 26, 2016

Choose a reason for hiding this comment

ejholmes Oct 27, 2016

Choose a reason for hiding this comment

ejholmes commented Oct 28, 2016

ejholmes commented Oct 20, 2016 •

edited

Loading

ejholmes commented Oct 21, 2016 •

edited

Loading