Commit graph

22 commits

Author SHA1 Message Date
Stephan Renatus
d9487e553b
*: fix some lint issues
Mostly gathered these using golangci-lint's deadcode and ineffassign
linters.

Signed-off-by: Stephan Renatus <srenatus@chef.io>
2019-07-30 11:29:08 +02:00
Mark Sagi-Kazar
06521ffa49
Remove the logrus logger wrapper 2019-02-22 21:31:46 +01:00
Mark Sagi-Kazar
be581fa7ff
Add logger interface and stop relying on Logrus directly 2019-02-22 13:38:57 +01:00
Stephan Renatus
26c0206627
connector/saml: make unparsable (trailing, non-space/newline) data an error
Fixes #1304, if we want to be harsh.

However, I think if it was the user's intention to pass two certs, and
the second one couldn't be read, that shouldn't just disappear. After
all, when attempting to login later, that might fail because the
expected IdP cert data isn't there.

Signed-off-by: Stephan Renatus <srenatus@chef.io>
2018-09-29 11:09:33 +02:00
Stephan Renatus
b9f6594bf0 *: github.com/coreos/dex -> github.com/dexidp/dex
Signed-off-by: Stephan Renatus <srenatus@chef.io>
2018-09-05 17:57:08 +02:00
Stephan Renatus
608260d0f1 saml: add tests case covering tampered NameID field (comment)
As sketched here:

https://developer.okta.com/blog/2018/02/27/a-breakdown-of-the-new-saml-authentication-bypass-vulnerability

Thought it was interesting to see how our SAML connector behaved. And
it seems to be behaving well. :)

Signed-off-by: Stephan Renatus <srenatus@chef.io>
2018-02-28 08:42:17 +01:00
Michael Stapelberg
a41d93db4a Implement the “authproxy” connector (for Apache2 mod_auth etc.) 2017-10-25 21:53:51 +02:00
Eric Chiang
aad328bb35 *: add log events for login, LDAP queries, and SAML responses 2017-08-11 12:00:06 -07:00
Eric Stroczynski
4a88d0641a : update {S->s}irupsen/logrus 2017-07-25 13:46:44 -07:00
Eric Chiang
00b5c99ffc connector/saml/testdata: fix bad status test case
Notice this when inspecting the code coverage results. For some
reason this test wasn't triggering the bad status code path, maybe
due to signature validation. Removing the comment fixed the code
coverage.
2017-04-11 17:20:29 -07:00
Eric Chiang
362e0798a4 connector/saml: clean up SAML verification logic and comments 2017-04-07 14:13:05 -07:00
Phu Kieu
bd754e2b2d Fix entityIssuer -> ssoIssuer typo 2017-04-06 14:50:44 -07:00
Phu Kieu
47897f73fa Validate audience with entityIssuer if present, use redirectURI otherwise 2017-04-06 14:40:56 -07:00
Phu Kieu
217b5ca2c7 Add ssoIssuer to fix Response issuer checking
Rename issuer to entityIssuer
2017-04-06 11:05:49 -07:00
Eric Chiang
a97cffcd52 connector/saml: refactor tests and add self-signed responses
Introduces SAML tests which execute full response processing and
compare user attributes. tesdata now includes a full, self-signed
CA and documents signed using xmlsec1.

Adds deprication notices to existing tests, but don't remove them
since they still provide coverage.
2017-04-04 11:11:35 -07:00
Eric Chiang
e0709dc2ac connector/saml: fix validation bug with multiple Assertion elements
When a SAML response provided multiple Assertion elements, only the
first one is checked for a valid signature. If the Assertion is
verified, the original Assertion is removed and the canonicalized
version is prepended to the Response. However, if there were
multiple assertions, the second assertion could end up first in the
list of Assertions, even if it was unsigned.

For example this:

    <Response>
      <!--
         Response unsigned. According to SAML spec must check
         assertion signature.
      -->
      <Assertion>
        <Signature>
          <!-- Correrctly signed assertion -->
        </Signature>
      </Assertion>

      <Assertion>
        <!-- Unsigned assertion inserted by attacker-->
      </Assertion>
    </Response>

could be verified then re-ordered to the following:

    <Response>
      <!--
         Response unsigned. According to SAML spec must check
         assertion signature.
      -->
      <Assertion>
        <!-- Unsigned assertion inserted by attacker-->
      </Assertion>

      <Assertion>
        <!-- Canonicalized, correrctly signed assertion -->
      </Assertion>
    </Response>

Fix this by removing all unverified child elements of the Response,
not just the original assertion.
2017-04-04 11:11:35 -07:00
Phu Kieu
6f9ef961bb Use etreeutils.NSSelectOne to select Assertion element 2017-03-24 11:20:53 -07:00
rithu john
59502850f0 connector: Connectors without a RefreshConnector should not return a refresh token instead of erroring 2017-03-23 14:56:34 -07:00
Eric Chiang
50b223a9db *: validate InResponseTo SAML response field and make issuer optional 2017-03-22 13:02:44 -07:00
Eric Chiang
33f0199077 *: fix spelling using github.com/client9/misspell 2017-03-20 09:16:56 -07:00
Holger Koser
e46f2ebe40 Improve SAML Signature and Response Validation
* Improve Order of Namespace Declarations and Attributes in Canonical XML. This is related to an issue in goxmldsig for which I created an [pull request](https://github.com/russellhaering/goxmldsig/pull/17).
* Do not compress the AuthnRequest if `HTTP-POST` binding is used.
* SAML Response is valid if the Message and/or the Assertion is signed.
* Add `AssertionConsumerServiceURL` to `AuthnRequest`
* Validate Status on the Response
* Validate Conditions on the Assertion
* Validation SubjectConfirmation on the Subject
2017-01-26 19:05:40 +01:00
Eric Chiang
31dfb54b6f connector: add a SAML connector 2017-01-09 18:30:58 -08:00