supabase / supabase-js

An isomorphic Javascript client for Supabase. Query your Supabase database, subscribe to realtime events, upload and download files, browse typescript examples, invoke postgres functions via rpc, invoke supabase edge functions, query pgvector.
https://supabase.com
MIT License
2.86k stars 220 forks source link

Strings Upserted Into An Encrypted Text Column Cannot Be Decrypted #795

Closed AlexIsMaking closed 8 months ago

AlexIsMaking commented 11 months ago

Bug report

Describe the bug

When a string is inserted into an encrypted column via the API, the decrypted copy of that column does not display the original value.

To Reproduce

Steps to reproduce the behavior, please provide code snippets or a repository:

  const { team, user, bot } = installation;
  const customerId = cusId;
  const customerData = {
    id: customerId,
    ref: team.name,
    bot_token: bot.token,
    installed_by_user: user.id,
    installed_by_team: team.id,
  };
  const conflictField = 'id';

  await upsertData('customers', customerData, conflictField, supabase);

async function upsertData(table, data, conflictField, supabase) {
  const { data: result, error } = await supabase
    .from(table)
    .upsert(data, { onConflict: conflictField });

  if (error) {
    throw new Error('Error upserting data: ', error);
  } else {
    return result;
  }
}

Expected behavior

The content that was originally inserted into the encrypted column will be returned when the column is decrypted.

System information

Note

I first asked about this on Discord here.

AlexIsMaking commented 11 months ago

Just tried inserting the data by calling Postgres function but the outcome was the same.

72L commented 10 months ago

I am seeing the same issue. It looks like strings are being encrypted twice when using supabase-js.

@supabase/supabase-js version 2.26.0
72L commented 10 months ago

It looks like update works, but upsert does not work. Might need to upsert first and then update.

steve-chavez commented 10 months ago

@AlexIsMaking @72L Can you share how you create your tables? Is this using TCE?

AlexIsMaking commented 10 months ago

@steve-chavez yes it's using TCE, I've created the table and columns from the dashboard.

AlexIsMaking commented 10 months ago

It looks like update works, but upsert does not work.

@72L just tested this and it looks like you might be right.

I'll update the title a little bit later, I'm just going to wait and see if the behaviour changes because I noticed that inserting and then updating data using upsert works when the row is new but stops working X hours later.

Might need to upsert first and then update.

Appreciate this suggestion too. Do you know of a way to do this without making 2 API requests by any chance?

AlexIsMaking commented 10 months ago

@72L just checked and looks like you were right about upserting being the issue - I was able to update my encrypted text column and retrieve the decrypted content as expected, several hours after the row was first created.

72L commented 10 months ago

Do you know of a way to do this without making 2 API requests by any chance?

No, I'm making two api requests now ha

@steve-chavez same here, I used the dashboard to create a new column on an existing table with TCE

steve-chavez commented 10 months ago

@michelp Any idea to what could be wrong here? PostgREST basically generates an INSERT .. ON CONFLICT for upsert.

72L commented 10 months ago

looks like the update workaround is no longer working

michelp commented 10 months ago

I can't reproduce this in straight SQL using an encrypted table as snow below, Can you provide the table definition and security label for the effected table?

postgres=# insert into "tce-example"."decrypted_test" (name, secret) values ('foo', 's3kr3t');
INSERT 0 1
postgres=# insert into "tce-example"."decrypted_test" (name, secret) values ('foo', 's3kr3t');
ERROR:  duplicate key value violates unique constraint "test_name_key"
DETAIL:  Key (name)=(foo) already exists.
postgres=# insert into "tce-example"."decrypted_test" (name, secret) values ('foo', 's3kr3t') on conflict (name) do update set secret = 'zing';
INSERT 0 1
postgres=# select * from "tce-example"."decrypted_test" ;
-[ RECORD 1 ]----+-------------------------------------------------
secret           | ow7MfsW8XNq4/cd/aRVbO6/SA08+k0YmlDA6gdDQZObyVX2w
decrypted_secret | zing
name             | foo

postgres=# 
AlexIsMaking commented 10 months ago

@michelp here's the table definition -

create table
  public.demo (
    id text not null,
    created_at timestamp with time zone null default now(),
    secret text null,
    constraint demo_pkey primary key (id)
  ) tablespace pg_default;

create trigger demo_encrypt_secret_trigger_secret before insert
or
update of secret on demo for each row
execute function demo_encrypt_secret_secret ();

Not sure where to look for the security label? I'm using 1 key for the entire column, like this.

michelp commented 10 months ago

It's in the pg_seclabel catalog

For example

postgres=# select * from pg_seclabel;
 objoid | classoid | objsubid | provider |                                                    label                                                     
--------+----------+----------+----------+--------------------------------------------------------------------------------------------------------------
  16530 |     1259 |       10 | pgsodium | ENCRYPT WITH KEY COLUMN parent_key ASSOCIATED (id, associated_data) NONCE raw_key_nonce
  16697 |     1259 |        1 | pgsodium | ENCRYPT WITH KEY ID 56f1a181-6ae9-4144-bae5-0e6d3225484e
  16705 |     1259 |        0 | pgsodium | DECRYPT WITH VIEW "tce-example"."other-test2"
  16705 |     1259 |        2 | pgsodium | ENCRYPT WITH KEY ID 13e57804-293d-4fde-86e4-691e6fc752a7 ASSOCIATED (associated) NONCE nonce
  16705 |     1259 |        5 | pgsodium | ENCRYPT WITH KEY COLUMN secret2_key_id ASSOCIATED (id, associated2) NONCE nonce2
  16786 |     1259 |        1 | pgsodium | ENCRYPT WITH KEY COLUMN secret2_key_id-test ASSOCIATED (associated2-test) NONCE nonce2-test SECURITY INVOKER
(6 rows)
AlexIsMaking commented 10 months ago

@michelp thanks, here you go -

objoid classoid objsubid provider label
16788 1259 10 pgsodium ENCRYPT WITH KEY COLUMN parent_key ASSOCIATED (id, associated_data) NONCE raw_key_nonce
16948 1259 4 pgsodium ENCRYPT WITH KEY COLUMN key_id ASSOCIATED (id, description, created_at, updated_at) NONCE nonce
28583 1259 11 pgsodium ENCRYPT WITH KEY ID c7d22cb9-4e1b-4a1b-a701-42f913477920
28583 1259 13 pgsodium ENCRYPT WITH KEY ID c7d22cb9-4e1b-4a1b-a701-42f913477920
28583 1259 14 pgsodium ENCRYPT WITH KEY ID c7d22cb9-4e1b-4a1b-a701-42f913477920
32556 1259 4 pgsodium ENCRYPT WITH KEY ID c7d22cb9-4e1b-4a1b-a701-42f913477920 SECURITY INVOKER
michelp commented 10 months ago

I don't know which table is related to which oid, try this:

postgres=# select relname, attname, label from pg_seclabel sl, pg_class c, pg_attribute a where sl.objoid = c.oid and c.oid = a.attrelid and a.attnum = sl.objsubid;
  relname  |   attname    |                                                    label                                                     
-----------+--------------+--------------------------------------------------------------------------------------------------------------
 test2     | secret2      | ENCRYPT WITH KEY COLUMN secret2_key_id ASSOCIATED (id, associated2) NONCE nonce2
 test2     | secret       | ENCRYPT WITH KEY ID 13e57804-293d-4fde-86e4-691e6fc752a7 ASSOCIATED (associated) NONCE nonce
 key       | raw_key      | ENCRYPT WITH KEY COLUMN parent_key ASSOCIATED (id, associated_data) NONCE raw_key_nonce
 bob-testt | secret2-test | ENCRYPT WITH KEY COLUMN secret2_key_id-test ASSOCIATED (associated2-test) NONCE nonce2-test SECURITY INVOKER
 test      | secret       | ENCRYPT WITH KEY ID 56f1a181-6ae9-4144-bae5-0e6d3225484e
(5 rows)
AlexIsMaking commented 10 months ago

@michelp

relname attname label
key raw_key ENCRYPT WITH KEY COLUMN parent_key ASSOCIATED (id, associated_data) NONCE raw_key_nonce
secrets secret ENCRYPT WITH KEY COLUMN key_id ASSOCIATED (id, description, created_at, updated_at) NONCE nonce
customers api_key ENCRYPT WITH KEY ID c7d22cb9-4e1b-4a1b-a701-42f913477920
customers user_token ENCRYPT WITH KEY ID c7d22cb9-4e1b-4a1b-a701-42f913477920
customers bot_token ENCRYPT WITH KEY ID c7d22cb9-4e1b-4a1b-a701-42f913477920
demo secret ENCRYPT WITH KEY ID c7d22cb9-4e1b-4a1b-a701-42f913477920 SECURITY INVOKER
michelp commented 10 months ago

Hmm it seems to be working fine in SQL so perhaps this is a postgrest issue? See below for insert with conflict update:

postgres=# create table
  public.demo (
    id text not null,
    created_at timestamp with time zone null default now(),
    secret text null,
    constraint demo_pkey primary key (id)
  ) tablespace pg_default;
CREATE TABLE
postgres=# SELECT format('ENCRYPT WITH KEY ID %s', (pgsodium.create_key('aead-det')).id)
    AS seclabel \gset
postgres=# SECURITY LABEL FOR pgsodium  ON COLUMN public.demo.secret IS :'seclabel';
NOTICE:  view "decrypted_demo" does not exist, skipping
NOTICE:  function public.demo_encrypt_secret_secret() does not exist, skipping
NOTICE:  trigger "demo_encrypt_secret_trigger_secret" for relation "public.demo" does not exist, skipping
NOTICE:  about to masking role public.demo public.decrypted_demo
SECURITY LABEL
postgres=# insert into public.demo (id, secret) values ('foo', 's3kret');
INSERT 0 1
postgres=# insert into public.demo (id, secret) values ('foo', 's3kret');
ERROR:  duplicate key value violates unique constraint "demo_pkey"
DETAIL:  Key (id)=(foo) already exists.
postgres=# insert into public.demo (id, secret) values ('foo', 's3kret') on conflict (id) do update set secret = 'bizz';
INSERT 0 1
postgres=# select * from public.demo;
 id  |          created_at           |                      secret                      
-----+-------------------------------+--------------------------------------------------
 foo | 2023-07-14 18:12:24.762347+00 | 7Qem3XpVhD1VyKUSx78BonHtIzcdl9vj/yw65UWfSjTVzXig
(1 row)

postgres=# select * from public.decrypted_demo ;
 id  |          created_at           |                      secret                      | decrypted_secret 
-----+-------------------------------+--------------------------------------------------+------------------
 foo | 2023-07-14 18:12:24.762347+00 | 7Qem3XpVhD1VyKUSx78BonHtIzcdl9vj/yw65UWfSjTVzXig | bizz
(1 row)
michelp commented 10 months ago

Ah I think I get it, the issue is that you're upserting into the table and not the decrypted view, postgrest is correctly returning you the upserted row into the table, if you want to get the secret back out, you have to access it through the view:

postgres=# insert into public.decrypted_demo (id, secret) values ('foo', 's3kret') on conflict (id) do update set secret = 'zing';
INSERT 0 1
postgres=# select * from public.decrypted_demo ;
 id  |          created_at           |                      secret                      | decrypted_secret 
-----+-------------------------------+--------------------------------------------------+------------------
 foo | 2023-07-14 18:12:24.762347+00 | /A7jJo3nwa4l0NSdeRUs8ZwnC6ap/xmt3tJ2kVpEUPrJvXXT | zing
(1 row)

Note I'm inserting into the decryptor view, not the table itself (you can insert/update views when postgres considers them "updatable", see the section "Updatable Views" https://www.postgresql.org/docs/current/sql-createview.html)

AlexIsMaking commented 10 months ago

@michelp got it, that worked for me too ๐ŸŽ‰

Note I'm inserting into the decryptor view, not the table itself (you can insert/update views when postgres considers them "updatable", see the section "Updatable Views" https://www.postgresql.org/docs/current/sql-createview.html)

Would definitely help if the docs mentioned that. This makes it sound like you can just update the table, rather than the view -

Once you've created an encrypted column, you can insert data into the table like you would any other table. For example if you were to insert an email address into an encrypted column, you will see that the address is transparently converted into an encrypted value on the new row.

michelp commented 10 months ago

Ah good point that is confusingly worded, I'll update the docs, thanks!

AlexIsMaking commented 10 months ago

Thanks! Guess I can close this now then.

michelp commented 10 months ago

I'll also add a helper function that does that security label join mess so it's easier to diagnose.

AlexIsMaking commented 10 months ago

@michelp sorry, looks like I spoke too soon. If I run exactly the same script that was working before and upsert the row that I created earlier, in the decrypted view, I run into the same issue now - the decrypted view displays an encrypted key. I saw this before too - it seems like there's something time related that causes the upsert to stop working.

If you try upserting the row that you created earlier, rather than creating a new one, hopefully you can reproduce the issue.

michelp commented 10 months ago

I can run that insert over and over again with no problems or time issues, are you sure you're not mixing up the secret and decrypted_secret columns? There's a specific reason they are separate, the secret column can be written to, and is stored encrypted, but reading it isn't useful (it is after all encrypted), the decrypted_secret column of the view is for reading the value, it cannot be written to (postgres wont let you, because it's a "computed column" of the view). If you write the encrypted secret column back into itself, it will double encrypt the value, which you don't want.

AlexIsMaking commented 10 months ago

@michelp I'm definitely not mixing up the columns, here's the complete code that I'm using for testing -

import { createClient } from '@supabase/supabase-js';
import dotenv from 'dotenv';

dotenv.config();

const supabaseUrl = process.env.SUPABASE_URL;
const supabaseServiceKey = process.env.SUPABASE_SERVICE_KEY;
const options = {
  auth: {
    persistSession: false,
  },
};

const supabase = createClient(
  supabaseUrl,
  supabaseServiceKey,
  options,
);

const tableName = 'decrypted_demo';
const conflict = 'id';
const apiKey = 's3kr3t';
const content = {
  id: 'test',
  secret: apiKey,
};

async function upsertRowInTable(table, rowData, conflictField) {
  try {
    const { data: result, error } = await supabase
      .from(tableName)
      .upsert(rowData, { onConflict: conflictField })
      .select();

    if (error) {
      console.error(`Error inserting row into ${table}:`, error);
      return null;
    }

    return result;
  } catch (err) {
    throw new Error('Unexpected error in upsertRowInTable:', err);
  }
}

const result = await upsertRowInTable(tableName, content, conflict, supabase);
console.log('result:', result);
result: [
  {
    id: 'test',
    created_at: '2023-07-14T18:27:29.382944+00:00',
    secret: 'ReVoKjIXeaize08bvcotOvsjdE9pnvpiOcZosi/T3JWvugtOMpmn48wFYXBFsGmooAP7rO7SS6Ju\n' +
      'cRAAaQCDI8Gf086gGwQn5G9YBufzSTA2Qikx',
    decrypted_secret: '1GtUuxbDD++TYcqxBVt1znjKiRaGaosyIVxVSEFU9yxAhorf028='
  }
]

Are you updating the same row that you created earlier, via an upsert? If it's a new row, you (probably), won't see the error.

michelp commented 10 months ago

Yep I'm inserting the "same" row, of course in SQL the "sameness" of a row in this case is whether or not it satisfies a uniqueness constraint or not, so I'm inserting the same id value, and thus violating the constraint, and consequently the ON CONFLICT clause is evaluated.

This might be something to do with PostgREST after all then, since I can't reproduce what you're describing in straight SQL. I'll investigate further.

AlexIsMaking commented 9 months ago

@michelp have you had a chance to look further into this at all? It'd be great to be able to use the encrypted columns to store sensitive information.

michelp commented 9 months ago

Hi @AlexIsMaking,

We've found the issue is an intersection of two problems, the first is that PostgREST (correctly IMO) generates the ON CONFLICT query using the EXCLUDED.* pseudo-row to set columns from the row that conflicted (ie the data to be updated). In this case, the excluded column contains the encrypted data, not the plaintext (that plaintext isn't included in any column of the excluded row, that comes from the view) so it ends up being double encrypted.

The view does have both columns, but the second issue is that you cannot usefully INSERT ... ON CONFLICT to a view, because a conflict clause requires a unique constraint violation, and views do not have constraints. So even if problem 1 didn't exist problem 2 would prevent it.

The only workaround we can think of for the moment is to use and RPC function to do the upsert, joining the table and the view and doing INSERT ... ON CONFLICT (id) DO UPDATE SET ..., t.secret = v.decrypted_secret and this will decrypt the column using the view, then reencrypt it using the table trigger. I'll try and come up with a working example for you and drop it here in a comment.

AlexIsMaking commented 9 months ago

@michelp really appreciate the update. An example of workaround would be a big help, that sounds like it might be a little bit complicated to set up ๐Ÿ˜…

michelp commented 8 months ago

Hi Alex,

Sorry for the delay I forgot to circle back on this one, here's a gist of a working example for a function that does "upsert" into a TCE table:

https://gist.github.com/michelp/b77b29089855f8bdaf8852cd115781ac

You'll have to adapt this to your specific case. In order to call an RPC function with supabase js, see this documentation:

https://supabase.com/docs/reference/javascript/rpc

AlexIsMaking commented 8 months ago

@michelp thanks a lot for sharing that example. I'm a little bit wary about saying this definitely works for me, after we had the issue with the previous solution seemingly working but only temporarily ๐Ÿ˜… but it's working so far ๐Ÿ™Œ

For the benefit of anyone who's setting this up - unfortunately this SQL can't be run in Supabase's built-in SQL editor because it uses \gset. If you don't have psql set up (which is needed in order to use \gset), like me, you can run this SQL using Postgres.js, like this:

https://gist.github.com/AlexIsMaking/e58c3c4996755b1e96b306a670479ecc

AlexIsMaking commented 8 months ago

Just confirmed that, that the function is working. Thanks again @michelp!

JungeWerther commented 1 month ago

Hi! Ran into the same problem. After searching and trying things for 3h finally found this thread. RPC function works like a charm, thank you !!!