Skip to content

Commit

Permalink
feat(saml): vulnerabilities check
Browse files Browse the repository at this point in the history 
+ update saml tests
Signed-off-by: ThibaultHerard <[email protected]>

Co-authored-by: sebferrer <[email protected]>
  • Loading branch information
@ThibHrrd @sebferrer
ThibHrrd and sebferrer committed Feb 14, 2023
1 parent 9cfed21 commit cd8689f
Show file tree
Hide file tree
Showing 30 changed files with 2,053 additions and 132 deletions.
13 changes: 7 additions & 6 deletions go.mod
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ require (
github.com/cenkalti/backoff v2.2.1+incompatible
github.com/coreos/go-oidc v2.2.1+incompatible
github.com/cortesi/modd v0.0.0-20210323234521-b35eddab86cc
github.com/crewjam/saml v0.4.6
github.com/crewjam/saml v0.4.10
github.com/davecgh/go-spew v1.1.1
github.com/davidrjonas/semver-cli v0.0.0-20190116233701-ee19a9a0dda6
github.com/dgraph-io/ristretto v0.1.1
Expand All @@ -44,9 +44,9 @@ require (
github.com/go-swagger/go-swagger v0.30.3
github.com/gobuffalo/fizz v1.14.4
github.com/gobuffalo/httptest v1.5.2
github.com/gobuffalo/pop/v6 v6.1.2-0.20230124165254-ec9229dbf7d7
github.com/gofrs/uuid v4.3.1+incompatible
github.com/golang-jwt/jwt/v4 v4.1.0
github.com/gobuffalo/pop/v6 v6.0.8
github.com/gofrs/uuid v4.3.0+incompatible
github.com/golang-jwt/jwt/v4 v4.4.2
github.com/golang/gddo v0.0.0-20190904175337-72a348e765d2
github.com/golang/mock v1.6.0
github.com/google/go-github/v27 v27.0.1
Expand All @@ -59,6 +59,7 @@ require (
github.com/hashicorp/golang-lru v0.5.4
github.com/imdario/mergo v0.3.13
github.com/inhies/go-bytesize v0.0.0-20220417184213-4913239db9cf
github.com/instana/testify v1.6.2-0.20200721153833-94b1851f4d65
github.com/jarcoal/httpmock v1.0.5
github.com/jteeuwen/go-bindata v3.0.7+incompatible
github.com/julienschmidt/httprouter v1.3.0
Expand Down Expand Up @@ -102,6 +103,7 @@ require (
golang.org/x/oauth2 v0.4.0
golang.org/x/sync v0.1.0
golang.org/x/tools v0.2.0
google.golang.org/grpc v1.50.1
gotest.tools v2.2.0+incompatible
)

Expand Down Expand Up @@ -326,8 +328,7 @@ require (
golang.org/x/time v0.1.0 // indirect
golang.org/x/xerrors v0.0.0-20220907171357-04be3eba64a2 // indirect
google.golang.org/appengine v1.6.7 // indirect
google.golang.org/genproto v0.0.0-20221227171554-f9683d7f8bef // indirect
google.golang.org/grpc v1.52.0 // indirect
google.golang.org/genproto v0.0.0-20221025140454-527a21cfbd71 // indirect
google.golang.org/protobuf v1.28.1 // indirect
gopkg.in/alecthomas/kingpin.v2 v2.2.6 // indirect
gopkg.in/alexcesaro/quotedprintable.v3 v3.0.0-20150716171945-2caba252f4dc // indirect
Expand Down
16 changes: 9 additions & 7 deletions go.sum
View file
Original file line number Diff line number Diff line change
Expand Up @@ -280,8 +280,8 @@ github.com/creack/pty v1.1.11 h1:07n33Z8lZxZ2qwegKbObQohDhXDQxiMMz1NOUGYlesw=
github.com/creack/pty v1.1.11/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ33E=
github.com/crewjam/httperr v0.2.0 h1:b2BfXR8U3AlIHwNeFFvZ+BV1LFvKLlzMjzaTnZMybNo=
github.com/crewjam/httperr v0.2.0/go.mod h1:Jlz+Sg/XqBQhyMjdDiC+GNNRzZTD7x39Gu3pglZ5oH4=
github.com/crewjam/saml v0.4.6 h1:XCUFPkQSJLvzyl4cW9OvpWUbRf0gE7VUpU8ZnilbeM4=
github.com/crewjam/saml v0.4.6/go.mod h1:ZBOXnNPFzB3CgOkRm7Nd6IVdkG+l/wF+0ZXLqD96t1A=
github.com/crewjam/saml v0.4.10 h1:Rjs6x4s/aQFXiaPjw3uhB4VdxRqoxHXOJrrj4BsMn9o=
github.com/crewjam/saml v0.4.10/go.mod h1:9Zh6dWPtB3MSzTRt8fIFH60Z351QQ+s7hCU3J/tTlA4=
github.com/cyphar/filepath-securejoin v0.2.3/go.mod h1:aPGpWjXOXUn2NCNjFvBE6aRxGGx79pTxQpKOJNYHHl4=
github.com/daaku/go.zipexe v1.0.0/go.mod h1:z8IiR6TsVLEYKwXAoE/I+8ys/sDkgTzSL0CLnGVd57E=
github.com/daaku/go.zipexe v1.0.1/go.mod h1:5xWogtqlYnfBXkSB1o9xysukNP9GTvaNkqzUZbt3Bw8=
Expand All @@ -291,7 +291,6 @@ github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c
github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
github.com/davidrjonas/semver-cli v0.0.0-20190116233701-ee19a9a0dda6 h1:VzPvKOw28XJ77PYwOq5gAqvFB4gk6gst0HxxiW8kfZQ=
github.com/davidrjonas/semver-cli v0.0.0-20190116233701-ee19a9a0dda6/go.mod h1:+6FzxsSbK4oEuvdN06Jco8zKB2mQqIB6UduZdd0Zesk=
github.com/dchest/uniuri v0.0.0-20200228104902-7aecb25e1fe5/go.mod h1:GgB8SF9nRG+GqaDtLcwJZsQFhcogVCJ79j4EdT0c2V4=
github.com/devigned/tab v0.1.1/go.mod h1:XG9mPq0dFghrYvoBF3xdRrJzSTX1b7IQrvaL9mzjeJY=
github.com/dgraph-io/ristretto v0.0.1/go.mod h1:T40EBc7CJke8TkpiYfGGKAeFjSaxuFXhuXRyumBd6RE=
github.com/dgraph-io/ristretto v0.0.2/go.mod h1:KPxhHT9ZxKefz+PCeOGsrHpl1qZ7i70dGTu2u+Ahh6E=
Expand Down Expand Up @@ -532,8 +531,9 @@ github.com/gogo/protobuf v1.3.0/go.mod h1:SlYgWuQ5SjCEi6WLHjHCa1yvBfUnHcTbrrZtXP
github.com/gogo/protobuf v1.3.1/go.mod h1:SlYgWuQ5SjCEi6WLHjHCa1yvBfUnHcTbrrZtXPKa29o=
github.com/gogo/protobuf v1.3.2 h1:Ov1cvc58UF3b5XjBnZv7+opcTcQFZebYjWzi34vdm4Q=
github.com/gogo/protobuf v1.3.2/go.mod h1:P1XiOD3dCwIKUDQYPy72D8LYyHL2YPYrpS2s69NZV8Q=
github.com/golang-jwt/jwt/v4 v4.1.0 h1:XUgk2Ex5veyVFVeLm0xhusUTQybEbexJXrvPNOKkSY0=
github.com/golang-jwt/jwt/v4 v4.1.0/go.mod h1:/xlHOz8bRuivTWchD4jCa+NbatV+wEUSzwAxVc6locg=
github.com/golang-jwt/jwt/v4 v4.4.2 h1:rcc4lwaZgFMCZ5jxF9ABolDcIHdBytAFgqFPbSJQAYs=
github.com/golang-jwt/jwt/v4 v4.4.2/go.mod h1:m21LjoU+eqJr34lmDMbreY2eSTRJ1cv77w39/MY0Ch0=
github.com/golang/gddo v0.0.0-20190904175337-72a348e765d2 h1:xisWqjiKEff2B0KfFYGpCqc3M3zdTz+OHQHRc09FeYk=
github.com/golang/gddo v0.0.0-20190904175337-72a348e765d2/go.mod h1:xEhNfoBDX1hzLm2Nf80qUvZ2sVwoMZ8d6IE2SrsQfh4=
github.com/golang/glog v0.0.0-20160126235308-23def4e6c14b/go.mod h1:SBH7ygxi8pfUlaOkMMuAQtPIUF8ecWP5IEl/CR7VP2Q=
Expand Down Expand Up @@ -785,6 +785,8 @@ github.com/inconshreveable/mousetrap v1.1.0/go.mod h1:vpF70FUmC8bwa3OWnCshd2FqLf
github.com/influxdata/influxdb1-client v0.0.0-20191209144304-8bf82d3c094d/go.mod h1:qj24IKcXYK6Iy9ceXlo3Tc+vtHo9lIhSX5JddghvEPo=
github.com/inhies/go-bytesize v0.0.0-20220417184213-4913239db9cf h1:FtEj8sfIcaaBfAKrE1Cwb61YDtYq9JxChK1c7AKce7s=
github.com/inhies/go-bytesize v0.0.0-20220417184213-4913239db9cf/go.mod h1:yrqSXGoD/4EKfF26AOGzscPOgTTJcyAwM2rpixWT+t4=
github.com/instana/testify v1.6.2-0.20200721153833-94b1851f4d65 h1:T25FL3WEzgmKB0m6XCJNZ65nw09/QIp3T1yXr487D+A=
github.com/instana/testify v1.6.2-0.20200721153833-94b1851f4d65/go.mod h1:nYhEREG/B7HUY7P+LKOrqy53TpIqmJ9JyUShcaEKtGw=
github.com/jackc/chunkreader v1.0.0/go.mod h1:RT6O25fNZIuasFJRyZ4R/Y2BbhasbmZXF9QQ7T3kePo=
github.com/jackc/chunkreader/v2 v2.0.0/go.mod h1:odVSm741yZoC3dpHEUXIqA9tQRhFrgOHwnPIn9lDKlk=
github.com/jackc/chunkreader/v2 v2.0.1 h1:i+RDz65UE+mmpjTfyz0MoVTnzeYxroil2G82ki7MGG8=
Expand Down Expand Up @@ -1117,8 +1119,8 @@ github.com/ory/nosurf v1.2.7/go.mod h1:d4L3ZBa7Amv55bqxCBtCs63wSlyaiCkWVl4vKf3OU
github.com/ory/sessions v1.2.2-0.20220110165800-b09c17334dc2 h1:zm6sDvHy/U9XrGpixwHiuAwpp0Ock6khSVHkrv6lQQU=
github.com/ory/sessions v1.2.2-0.20220110165800-b09c17334dc2/go.mod h1:dk2InVEVJ0sfLlnXv9EAgkf6ecYs/i80K/zI+bUmuGM=
github.com/ory/viper v1.7.5/go.mod h1:ypOuyJmEUb3oENywQZRgeAMwqgOyDqwboO1tj3DjTaM=
github.com/ory/x v0.0.534 h1:hc49pmcOuHdJ6rbHVGtJJ4/LU88dzDCtEQKfgeo/ecU=
github.com/ory/x v0.0.534/go.mod h1:CQopDsCC9t0tQsddE9UlyRFVEFd2xjKBVcw4nLMMMS0=
github.com/ory/x v0.0.519 h1:T8/LbbQQqm+3P7bfI838T7eECv6+laXlvIyCp0QB+R8=
github.com/ory/x v0.0.519/go.mod h1:xUtRpoiRARyJNPVk/fcCNKzyp25Foxt9GPlj8pd7egY=
github.com/otiai10/copy v1.2.0/go.mod h1:rrF5dJ5F0t/EWSYODDu4j9/vEeYHMkc8jt0zJChqQWw=
github.com/otiai10/curr v0.0.0-20150429015615-9b4961190c95/go.mod h1:9qAhocn7zKJG+0mI8eUu6xqkFDYS2kb2saOteoSB3cE=
github.com/otiai10/curr v1.0.0/go.mod h1:LskTG5wDwr8Rs+nNQ+1LlxRjAtTZZjtJW4rMXl6j4vs=
Expand Down Expand Up @@ -1435,7 +1437,6 @@ github.com/yuin/goldmark v1.3.5/go.mod h1:mwnBkeHKe2W/ZEtQ+71ViKU8L12m81fl3OWwC1
github.com/yuin/goldmark v1.4.0/go.mod h1:mwnBkeHKe2W/ZEtQ+71ViKU8L12m81fl3OWwC1Zlc8k=
github.com/yuin/goldmark v1.4.13/go.mod h1:6yULJ656Px+3vBD8DxQVa3kxgyrAnzto9xy5taEt/CY=
github.com/zenazn/goji v0.9.0/go.mod h1:7S9M489iMyHBNxwZnk9/EHS098H4/F6TATF2mIxtB1Q=
github.com/zenazn/goji v1.0.1/go.mod h1:7S9M489iMyHBNxwZnk9/EHS098H4/F6TATF2mIxtB1Q=
github.com/ziutek/mymysql v1.5.4/go.mod h1:LMSpPZ6DbqWFxNCHW77HeMg9I646SAhApZ/wKdgO/C0=
github.com/zmap/rc2 v0.0.0-20131011165748-24b9757f5521/go.mod h1:3YZ9o3WnatTIZhuOtot4IcUfzoKVjUHqu6WALIyI0nE=
github.com/zmap/zcertificate v0.0.0-20180516150559-0e3d58b1bac4/go.mod h1:5iU54tB79AMBcySS0R2XIyZBAVmeHranShAFELYx7is=
Expand Down Expand Up @@ -1578,6 +1579,7 @@ golang.org/x/crypto v0.0.0-20210616213533-5ff15b29337e/go.mod h1:GvvjBRRGRdwPK5y
golang.org/x/crypto v0.0.0-20210711020723-a769d52b0f97/go.mod h1:GvvjBRRGRdwPK5ydBHafDWAxML/pGHZbMvKqRZ5+Abc=
golang.org/x/crypto v0.0.0-20210921155107-089bfa567519/go.mod h1:GvvjBRRGRdwPK5ydBHafDWAxML/pGHZbMvKqRZ5+Abc=
golang.org/x/crypto v0.0.0-20211108221036-ceb1ce70b4fa/go.mod h1:GvvjBRRGRdwPK5ydBHafDWAxML/pGHZbMvKqRZ5+Abc=
golang.org/x/crypto v0.0.0-20220128200615-198e4374d7ed/go.mod h1:IxCIyHEi3zRg3s0A5j5BB6A9Jmi73HwBIUl50j+osU4=
golang.org/x/crypto v0.0.0-20220517005047-85d78b3ac167/go.mod h1:IxCIyHEi3zRg3s0A5j5BB6A9Jmi73HwBIUl50j+osU4=
golang.org/x/crypto v0.0.0-20220622213112-05595931fe9d/go.mod h1:IxCIyHEi3zRg3s0A5j5BB6A9Jmi73HwBIUl50j+osU4=
golang.org/x/crypto v0.0.0-20220722155217-630584e8d5aa/go.mod h1:IxCIyHEi3zRg3s0A5j5BB6A9Jmi73HwBIUl50j+osU4=
Expand Down
6 changes: 3 additions & 3 deletions selfservice/strategy/saml/config_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,7 @@ func TestAttributesMapWithAnExtraField(t *testing.T) {
idpInformation := make(map[string]string)
idpInformation["idp_sso_url"] = "https://samltest.id/idp/profile/SAML2/Redirect/SSO"
idpInformation["idp_entity_id"] = "https://samltest.id/saml/idp"
idpInformation["idp_certificate_path"] = "file://testdata/samlkratos.crt"
idpInformation["idp_certificate_path"] = "file://testdata/idp_cert.pem"
idpInformation["idp_logout_url"] = "https://samltest.id/idp/profile/SAML2/Redirect/SSO"

// Initiates the service provider
Expand All @@ -235,8 +235,8 @@ func TestAttributesMapWithAnExtraField(t *testing.T) {
saml.Configuration{
ID: "samlProvider",
Label: "samlProviderLabel",
PublicCertPath: "file://testdata/myservice.cert",
PrivateKeyPath: "file://testdata/myservice.key",
PublicCertPath: "file://testdata/sp_cert.pem",
PrivateKeyPath: "file://testdata/sp_key.pem",
Mapper: "file://testdata/saml.jsonnet",
AttributesMap: attributesMap,
IDPInformation: idpInformation,
Expand Down
21 changes: 5 additions & 16 deletions selfservice/strategy/saml/handler.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -93,18 +93,7 @@ func (h *Handler) serveMetadata(w http.ResponseWriter, r *http.Request, ps httpr
w.Write(buf)
}

// swagger:route GET /self-service/methods/saml/auth v0alpha2 initializeSelfServiceSamlFlowForBrowsers
//
// Initialize Authentication Flow for SAML (Either the login or the register)
//
// If you already have a session, it will redirect you to the main page.
//
// Schemes: http, https
//
// Responses:
// 200: selfServiceRegistrationFlow
// 400: jsonError
// 500: jsonError
// TODO Swagger
func (h *Handler) loginWithIdp(w http.ResponseWriter, r *http.Request, ps httprouter.Params) {
// Middleware is a singleton so we have to verify that it exists
config := h.d.Config()
Expand Down Expand Up @@ -153,9 +142,9 @@ func (h *Handler) instantiateMiddleware(ctx context.Context, config config.Confi
}

// Key pair to encrypt and sign SAML requests
keyPair, err := tls.LoadX509KeyPair(strings.Replace(providerConfig.PublicCertPath, "file://", "", 1), strings.Replace(providerConfig.PrivateKeyPath, "file://", "", 1))
keyPair, err := tls.LoadX509KeyPair(strings.Replace(providerConfig.PublicCertPath, "file://", "", 1), strings.Replace(providerConfig.PrivateKeyPath, "file://", "", 1)) // TODO : Fetcher
if err != nil {
return herodot.ErrNotFound.WithTrace(err)
return herodot.ErrNotFound.WithTrace(err) // TODO : Replace with File not found error
}
keyPair.Leaf, err = x509.ParseCertificate(keyPair.Certificate[0])
if err != nil {
Expand Down Expand Up @@ -313,15 +302,15 @@ func (h *Handler) instantiateMiddleware(ctx context.Context, config config.Confi
// Return the singleton MiddleWare
func GetMiddleware(pid string) (*samlsp.Middleware, error) {
if samlMiddlewares[pid] == nil {
return nil, errors.Errorf("An error occurred while retrieving the middeware, it is null")
return nil, errors.Errorf("An error occurred while retrieving the middeware, it is null") // TODO : Improve error message
}
return samlMiddlewares[pid], nil
}

func MustParseCertificate(pemStr []byte) (*x509.Certificate, error) {
b, _ := pem.Decode(pemStr)
if b == nil {
return nil, errors.Errorf("Cannot find the next PEM formatted block")
return nil, errors.Errorf("Cannot find the next PEM formatted block while parsing the certificate")
}
cert, err := x509.ParseCertificate(b.Bytes)
if err != nil {
Expand Down
22 changes: 11 additions & 11 deletions selfservice/strategy/saml/handler_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ func TestInitMiddleWareWithoutMetadata(t *testing.T) {
middleWare, _, _, err := InitTestMiddlewareWithoutMetadata(t,
"https://samltest.id/idp/profile/SAML2/Redirect/SSO",
"https://samltest.id/saml/idp",
"file://testdata/samlkratos.crt",
"file://testdata/idp_cert.pem",
"https://samltest.id/idp/profile/SAML2/Redirect/SSO")

require.NoError(t, err)
Expand Down Expand Up @@ -74,7 +74,7 @@ func TestMustParseCertificate(t *testing.T) {

saml.DestroyMiddlewareIfExists("samlProvider")

certificateBuffer, err := fetcher.NewFetcher().Fetch("file://testdata/samlkratos.crt")
certificateBuffer, err := fetcher.NewFetcher().Fetch("file://testdata/sp_cert.pem")
require.NoError(t, err)

certificate, err := ioutil.ReadAll(certificateBuffer)
Expand All @@ -83,13 +83,13 @@ func TestMustParseCertificate(t *testing.T) {
cert, err := saml.MustParseCertificate(certificate)

require.NoError(t, err)
assert.Check(t, cert.Issuer.Country[0] == "AU")
assert.Check(t, cert.Issuer.Organization[0] == "Internet Widgits Pty Ltd")
assert.Check(t, cert.Issuer.Province[0] == "Some-State")
assert.Check(t, cert.Subject.Country[0] == "AU")
assert.Check(t, cert.Subject.Organization[0] == "Internet Widgits Pty Ltd")
assert.Check(t, cert.Subject.Province[0] == "Some-State")
assert.Check(t, cert.NotBefore.String() == "2022-02-21 11:08:20 +0000 UTC")
assert.Check(t, cert.NotAfter.String() == "2023-02-21 11:08:20 +0000 UTC")
assert.Check(t, cert.SerialNumber.String() == "485646075402096403898806020771481121115125312047")
assert.Check(t, cert.Issuer.Country[0] == "US")
assert.Check(t, cert.Issuer.Organization[0] == "foo")
assert.Check(t, cert.Issuer.Province[0] == "GA")
assert.Check(t, cert.Subject.Country[0] == "US")
assert.Check(t, cert.Subject.Organization[0] == "foo")
assert.Check(t, cert.Subject.Province[0] == "GA")
assert.Check(t, cert.NotBefore.String() == "2013-10-02 00:08:51 +0000 UTC")
assert.Check(t, cert.NotAfter.String() == "2014-10-02 00:08:51 +0000 UTC")
assert.Check(t, cert.SerialNumber.String() == "14253244695696570161")
}
3 changes: 0 additions & 3 deletions selfservice/strategy/saml/metadata_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package saml_test

import (
"encoding/xml"
"fmt"
"io"
"io/ioutil"
"net/http"
Expand Down Expand Up @@ -75,8 +74,6 @@ func TestXmlMetadataExist(t *testing.T) {
assert.NilError(t, err)
res, err := NewTestClient(t, nil).Get(ts.URL + "/self-service/methods/saml/metadata/samlProvider")
assert.NilError(t, err)
body, _ := ioutil.ReadAll(res.Body)
fmt.Println(body)
assert.Check(t, is.Equal(http.StatusOK, res.StatusCode))
assert.Check(t, is.Equal("text/xml", res.Header.Get("Content-Type")))
}
Expand Down
2 changes: 1 addition & 1 deletion selfservice/strategy/saml/provider_saml.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ func NewProviderSAML(
}
}

// Translate attributes from saml asseryion into kratos claims
// Translate attributes from saml assertion into kratos claims
func (d *ProviderSAML) Claims(ctx context.Context, config *config.Config, attributeSAML samlsp.Attributes, pid string) (*Claims, error) {

var c ConfigurationCollection
Expand Down
Loading

0 comments on commit cd8689f

Please sign in to comment.