SpecterOps / BloodHound

Six Degrees of Domain Admin
https://bloodhoundenterprise.io/
Apache License 2.0
1.07k stars 108 forks source link

No documentation regarding the SP certificate for SAML configuration #83

Open XidRanReb opened 1 year ago

XidRanReb commented 1 year ago

Description: The documentation regarding the configuration of SSO seems to be incomplete. There is no mention of the SP certificate for SAML configuration which is needed to sign SAML request.

Component(s) Affected:

Steps to Reproduce:

  1. Lauch the infra via Docker
  2. Set a SSO connection
  3. Get the following error {"level":"error","time":"2023-09-07T16:36:07.628900496Z","message":"[SAML] Failed initializing SAML SP instance azuread: failed to parse service provider azuread's cert pair: unable to decode cert"} {"level":"warn","time":"2023-09-07T16:36:07.628935749Z","message":"Writing API Error. Status: 401. Message: [{ authentication is invalid}]"}

Environment Information:

BloodHound: 5.0.7

Docker: 24.0.2

Potential Solution:

I added a pub certificate and its key in the config file :

  "saml": {
    "sp_cert":"XXX"
    "sp_key": "-----BEGIN PRIVATE KEY-----\nXXXXXXXX\n-----END PRIVATE KEY-----
  }

Side note : inconsistency for loading the cert and the key here. The BEGIN PRIVATE KEY is not added automaticaly. https://github.com/SpecterOps/BloodHound/blob/02b091f14d30a60945bf0d792c22279049960036/packages/go/crypto/tls.go#L77

StephenHinck commented 1 year ago

Hi there - documentation on the configuration of SAML is available here: https://support.bloodhoundenterprise.io/hc/en-us/articles/9228122981275-SAML-in-BloodHound

Based on the SAML config name in the error message, I'd suggest checking out https://support.bloodhoundenterprise.io/hc/en-us/articles/14266827417371-SAML-Configuration-of-Azure-AD and following those instructions. Several of our customers (and SpecterOps' own BloodHound Enterprise environment) are utilizing AzureAD successfully for SAML authentication.

The first thing that comes to mind is whether your tenant can reach the SAML provider. Is any web proxy performing SSL inspection or requiring authentication, preventing access to read the certificates?

XidRanReb commented 1 year ago

Based on my knowledge, the Identity Provider (Azure in my case) don't have anything to do with the Service Provider (BH) cert.

From the code where the error comes from :

https://github.com/SpecterOps/BloodHound/blob/7b088554246cc89d4309665214b34ab933ea0411/cmd/api/src/auth/bhsaml/provider.go#L200

I did not find any reference to those SAML.ServiceProviderCertificate and s.cfg.SAML.ServiceProviderKey, except in the undocumented SAMLConfiguration at https://github.com/SpecterOps/BloodHound/blob/7b088554246cc89d4309665214b34ab933ea0411/cmd/api/src/config/config.go#L120C1-L120C1

The potential solution described in my first post is working for me, but it may be hasardous as I am not used to go language. Maybe I have missed something.

samwmarsh commented 1 year ago

Hi @StephenHinck ,

We are also seeing this issue in our instance, both trying our own metadata.xml and the one provided at https://github.com/SpecterOps/BloodHound/blob/main/cmd/api/src/test/fixtures/fixtures/saml/idp_metadata.xml are returning the following log from our pod:

{"level":"error","time":"2023-09-15T10:40:34.580089079Z","message":"[SAML] Failed initializing SAML SP instance signin: failed to parse service provider bloodhound-test's cert pair: unable to decode cert"}
9
{"level":"warn","time":"2023-09-15T10:40:34.580123599Z","message":"Writing API Error. Status: 401. Message: [{ authentication is invalid}]"}

Our kubernetes deployment is on-prem and doesn't have internet access, just in case this causes further issues?

@XidRanReb - which config file did you edit for your workaround? I'm keen to test if this works on our instance too?

Thanks, Sam

XidRanReb commented 1 year ago

Hello @samwmarsh

I'm talking about this file : https://github.com/SpecterOps/BloodHound/blob/main/examples/docker-compose/bloodhound.config.json

samwmarsh commented 1 year ago

Hi @XidRanReb - thanks for the response, the private key - is this the private key of your service? I wouldn't anticipate users (us) to have access to the private signing key for the saml as this would allow us to roll our own SAML.

On other applications we normally provide the Identity Provider, SSO and SLO endpoints and the certificate data, and have no need to provide the sp_key so I'm confused by this bit. Are you able to assist @StephenHinck ?

Thanks

samwmarsh commented 12 months ago

Hi @XidRanReb / @StephenHinck - is it possible you can look at my previous comment please? Thanks

XidRanReb commented 12 months ago

Hi,

The service provider can sign his SAML request. It's up to the IDP to validate the signature and they usually don't. That's the purpose of the cert I added here. It has nothing to do with an IDP cert and the SAML answer.

samwmarsh commented 11 months ago

Hi @StephenHinck / @XidRanReb - we've followed the above guidance with the SAML details in the json, and it works! Thanks! We are verifying cert, but we've got that working on the IdP side. We're now seeing an sisue when we attempt to pass these as env vars rather than within the config.json file. When attempting to pass the values like

https://github.com/SpecterOps/BloodHound/blob/a4ea6f1aadee05df9df1272247a3a9b80a2e4399/examples/docker-compose/.env.example#L34-L35

so

-e bhe_saml_sp_cert=XXX -e bhe_saml_sp_key="-----BEGIN PRIVATE KEY-----\nXXXXXXXX\n-----END PRIVATE KEY-----", we get a kernel panic, which leads us to https://github.com/SpecterOps/BloodHound/blob/a4ea6f1aadee05df9df1272247a3a9b80a2e4399/packages/go/crypto/tls.go#L40

Have you attempted to do this using env vars, or just documented it without testing?

samwmarsh commented 11 months ago

Side note : inconsistency for loading the cert and the key here. The BEGIN PRIVATE KEY is not added automaticaly.

I've raised https://github.com/SpecterOps/BloodHound/pull/175 which should append/prepend key values where they don't currently exist. I believe this may also fix my issue, as I believe the env variables are being imported as "\n" instead of "\n" for newlines in the key.

xqrt commented 7 months ago

hi,

Did anyone manage to get this working with Keycloak?

Thanks, G

hkelley commented 3 months ago

@samwmarsh, were you ultimately able to set these vars (avoided touching the .json config)?

    bhe_saml_sp_cert
    bhe_saml_sp_key

If so, could you please summarize how you formatted each one with respect to BEGIN/END tags and newline escaping?

samwmarsh commented 3 months ago

@hkelley we were not able to set these vars without touching the .json, we ended up using a k8s configmap to write the saml cert/key into the json file (using argocd/helm) and these are stored as secrets within the k8s namespace.

apiVersion: v1
kind: ConfigMap
metadata:
  name: {{ .Values.bloodhound.appName }}
  namespace: {{ .Values.global.namespace }}
data:
  bloodhound.config.json: |
    {
...snip...
        "saml": {
            "sp_cert": "$saml_sp_cert",
            "sp_key": "-----BEGIN PRIVATE KEY-----\n$saml_sp_key\n-----END PRIVATE KEY-----"
        }
...snip...
    }
stianholsen commented 2 months ago

@hkelley Hi. Managed to get this working using the environment variables. Had to escape the newline characters for cert and the key. Didn't do any special formatting other than that. Also didn't have to add prefix or postfix to the key. Then I encountered the SAML assertion error set here: https://github.com/SpecterOps/BloodHound/blob/main/cmd/api/src/api/saml/saml.go#L274 This was solved by setting the nameidentifier (NameID) to user.userprincipalname and adding the eduPersonPrincipalName claim to user.mail: https://github.com/SpecterOps/BloodHound/blob/main/cmd/api/src/api/saml/saml.go#L233

hkelley commented 2 months ago

Thanks, @stianholsen . Are you saying you set it like so?

bhe_saml_sp_key = XX\\nXX\\nXXX
bhe_saml_sp_cert = MIIDGzCCAgMCFBucugfgWFBXhTT/si/\\ngycsbkgdYMA0GCSqGSIb3DQEBCwUAMEoxCzAJBgNVBAYTAlVTMQswCQYDVQQIDAJOWTERMA8GA1UEBwwIUHVyY2hhc2Ux\\nDjAMBgNVBAoMBVNvbXBvMQswCQYDVQQLDAJJVDAeFw0yNDA2MjQyMTAwNTJaFw0zNDA2MjIyMTAwNTJaMEoxCzAJBgNV
stianholsen commented 2 months ago

Hi. Was a bit unclear I see. But I did a cat of both the cert and the key files and replaced \n with \\n using sed and loaded the output into the environment variables. So, the prefix and postfix need to be part of the final string, but didn't have to manually type them in