SpecterOps / BloodHound

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

Saved queries have null userID when authenticating via API token #171

Closed lawndoc closed 11 months ago

lawndoc commented 1 year ago

Description:

I have been testing the new saved queries API, and I am using an API token because I have MFA on my account. I'm using the official Python client to HMAC sign and authenticate my requests to the API.

When I create saved queries while using the official Python client, all of the queries are saved with a userID of '00000000-0000-0000-0000-000000000000'

Component(s) Affected:

Steps to Reproduce:

  1. Call POST /api/v2/saved-queries while using bhesignature for authorization
  2. Call GET /api/v2/saved-queries and inspect the userID field of the newly created query

Expected Behavior:

Saved query is created with the userId of the user the API token belongs to.

Actual Behavior:

Saved query is created with a null userId field.

Screenshots/Code Snippets/Sample Files:

image

Environment Information:

BloodHound: v5.1.0

Collector: N/A

OS: N/A

Browser (if UI related): N/A

Node.js (if UI related) N/A

Go (if API related): not sure, using official bloodhound Docker container

Database (if persistence related): Neo4j 4.4 / Postgres 13.2

Docker (if using Docker): 24.0.2

Additional Information:

N/A

Potential Solution (Optional):

N/A

Related Issues:

N/A

Contributor Checklist:

lawndoc commented 1 year ago

It looks like this is the offending section in the API handler:

https://github.com/SpecterOps/BloodHound/blob/a211dc68169d03ecb2050b6a9e10cb3d4d1691e9/cmd/api/src/api/v2/saved_queries.go#L104-L108

Which calls the builtin-to-bloodhound context converter:

https://github.com/SpecterOps/BloodHound/blob/a211dc68169d03ecb2050b6a9e10cb3d4d1691e9/cmd/api/src/ctx/ctx.go#L71-L73

I'm not a Go dev, but I suspect the issue is somewhere in the context conversion or the HMAC signature being incompatible with the builtin context structure. I'm trying to understand the builtin and custom context structures.

https://github.com/SpecterOps/BloodHound/blob/a211dc68169d03ecb2050b6a9e10cb3d4d1691e9/cmd/api/src/ctx/ctx.go#L75-L86

lawndoc commented 1 year ago

A simple solution would be to allow admins or a custom "saved queries" role to provide the userID themselves in the POST request to /api/v2/saved-queries. That would also be a nice feature for my use case where I'm updating saved queries from git in a CI/CD pipeline because then I could deploy custom queries to all of my users.

That's not a true fix for the original bug though.

lawndoc commented 1 year ago

How does ctx.bhe get set in the request context?

elikmiller commented 1 year ago

Hey @lawndoc - we're picking up this work internally and working on a fix now. Thank you for the report!

StephenHinck commented 11 months ago

This will be fixed with https://github.com/SpecterOps/BloodHound/pull/245, once it releases!

lawndoc commented 11 months ago

Thanks for all your work!