Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use ?sql=xxx:signature instead of signed values #45

Closed
simonw opened this issue Mar 21, 2021 · 8 comments
Closed

Use ?sql=xxx:signature instead of signed values #45

simonw opened this issue Mar 21, 2021 · 8 comments
Labels
enhancement New feature or request security small

Comments

@simonw
Copy link
Owner

simonw commented Mar 21, 2021

URLs to SQL queries currently look like this:

https://simonwillison.net/dashboard/?sql=InNlbGVjdCAlKG5hbWUpcyBhcyBuYW1lLCB0b19jaGFyKGRhdGVfdHJ1bmMoJ21vbnRoJywgY3JlYXRlZCksICdZWVlZLU1NJykgYXMgYmFyX2xhYmVsLFxyXG5jb3VudCgqKSBhcyBiYXJfcXVhbnRpdHkgZnJvbSBibG9nX2VudHJ5IGdyb3VwIGJ5IGJhcl9sYWJlbCBvcmRlciBieSBjb3VudCgqKSBkZXNjIg%3A1lLfRD%3AvFP_m0s3BxRS2qyiWtlMlE1KRa2qoKItofP1vvK7hdY&sql=InNlbGVjdCBib2R5IGFzIGh0bWwgZnJvbSBibG9nX2VudHJ5IGxpbWl0IDEi%3A1lLfRD%3AEK0KOXcGgYdgD4Yzglbodf806GnbmrdtPridp8m0hlY

The problem here is that if the Django secret is reset these become broken links - there's no easy way to recover the SQL.

Instead, if the signatures do not match, how about populating the forms but NOT executing the SQL queries, and showing a warning message at the top of the page?

The ?sql= parameters could then become ?sql=SELECT ...::oKItofP1vvK7hdY where oKItofP1vvK7hdY is a signature but the rest of the query is in plain text.

@simonw simonw added enhancement New feature or request security labels Mar 21, 2021
@simonw simonw added duplicate This issue or pull request already exists small and removed duplicate This issue or pull request already exists labels Apr 14, 2021
@simonw
Copy link
Owner Author

simonw commented Apr 14, 2021

Another reason this is useful: if you have a staging environment and a live environment with different secrets you can still modify the URL of one to the hostname of the other and still edit and re-submit the queries on the new host.

@simonw
Copy link
Owner Author

simonw commented Apr 14, 2021

I'm going to add code to detect the old style of ?sql= and redirect it to the new style, since there are already dashboard links out there in the wild that I'd like to continue working.

@simonw
Copy link
Owner Author

simonw commented Apr 14, 2021

I can still use most of Django's signing code for this. The only difference is that I'll be using sign and unsign directly, instead of sign_object and unsight_object (which are called for me by .dumps() and loads(): https://github.com/django/django/blob/59552bea5790c97be0da0a6f16ccd0189857c7a7/django/core/signing.py#L93-L119)

@simonw
Copy link
Owner Author

simonw commented Apr 14, 2021

I'm really uncomfortably about the idea of accepting a random weird looking string, unsigning it, finding the signature is invalid, then saying "it might be base64 encoded JSON", attempting to decrypt THAT (from an already untrusted source) and showing that end result back to the user.

I think it can be done securely, but there's enough weirdness going on there that I don't want to try.

So instead I'm going to say that the "decode old signed strings" part of this will only work if the signature DOES pass. I'm not going to attempt to decode a JSON/base64 string that wasn't protected by a signature.

@simonw
Copy link
Owner Author

simonw commented Apr 14, 2021

So there are three parts to this:

  • Switch to signing SQL directly rather than doing the JSON/base64 thing we're doing at the moment
  • Have an optional upgrade path (controlled by a setting, off by default) where if a signed ?sql= string appears to be a base64 encoded JSON object with a valid signature we decode that and redirect to the new-style raw SQL URLs
  • For raw SQL URLs only, if the signature fails we show the user the SQL and let them chose to execute it or not (so that upgraded secrets won't completely invalidate existing links, but instead will prevent them from being executed without user interaction)

@simonw
Copy link
Owner Author

simonw commented Apr 14, 2021

I might include clickjacking prevention here: if you load the dashboard with a ?sql=xxx which doesn't have a valid signature, set a 5 second countdown before the execute query buttons will work to prevent clickjacking.

Actually no. If I care about clickjacking I should use the Content-Security-Policy: frame-ancestors 'self' header.

@simonw
Copy link
Owner Author

simonw commented Apr 14, 2021

Here's how SQL that is missing the signature will be displayed:

Django_SQL_Dashboard

@simonw
Copy link
Owner Author

simonw commented Apr 14, 2021

I'm splitting out the handling of old signed values into a separate ticket.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request security small
Projects
None yet
Development

No branches or pull requests

1 participant