SCRAM key signatures are derived incorrectly

[SomeoneToIgnore]

I have been trying to connect from a cloudflare worker to postgres, using SCRAM (which is the new default since PG14).

For that, a modified version of https://github.com/cloudflare/worker-template-postgres was used, which showed that PG with SCRAM auth method is never able to get connected to from the worker, while MD5 and passwordless methods worked just fine.

When similar example was run from Deno, it worked for all auth methods, including SCRAM with the same postgres.

I've narrowed down the case to the following snippet:

const text_encoder = new TextEncoder();

// interface KeySignatures {
//   client: Uint8Array;
//   server: Uint8Array;
//   stored: Uint8Array;
// }

// See https://github.com/denodrivers/postgres/blob/8a07131efa17f4a6bcab86fd81407f149de93449/connection/scram.ts#L93
// async function deriveKeySignatures(
//   password: string,
//   salt: Uint8Array,
//   iterations: number,
// ): Promise<KeySignatures> {
async function deriveKeySignatures(
  password,
  salt,
  iterations,
) {
  console.log(`Input: \n${JSON.stringify({
    password,
    salt: arrayToPrettyString(salt),
    iterations
  }, false, 2)}\n================\n`);

  const pbkdf2_password = await crypto.subtle.importKey(
    "raw",
    text_encoder.encode(password),
    "PBKDF2",
    false,
    ["deriveBits", "deriveKey"],
  );
  const key = await crypto.subtle.deriveKey(
    {
      hash: "SHA-256",
      iterations,
      name: "PBKDF2",
      salt,
    },
    pbkdf2_password,
    { name: "HMAC", hash: "SHA-256" },
    false,
    ["sign"],
  );

  const client = new Uint8Array(
    await crypto.subtle.sign("HMAC", key, text_encoder.encode("Client Key")),
  );
  const server = new Uint8Array(
    await crypto.subtle.sign("HMAC", key, text_encoder.encode("Server Key")),
  );
  const stored = new Uint8Array(await crypto.subtle.digest("SHA-256", client));
  return { client, server, stored };
}

////////////////////////

// See https://www.rfc-editor.org/rfc/rfc7677
// See https://github.com/denodrivers/postgres/blob/8a07131efa17f4a6bcab86fd81407f149de93449/tests/auth_test.ts#L6
function decode(b64) {
  const binString = atob(b64);
  const size = binString.length;
  const bytes = new Uint8Array(size);
  for (let i = 0; i < size; i++) {
    bytes[i] = binString.charCodeAt(i);
  }
  return bytes;
}
const password = 'pencil'
const salt = decode('W22ZaJ0SNY7soEsUEjb6gQ==');
const iterations = 4096

function signaturesToString(signatures) {
  let { client, server, stored } = signatures;
  return JSON.stringify({
    client: arrayToPrettyString(client),
    server: arrayToPrettyString(server),
    stored: arrayToPrettyString(stored),
  }, false, 2);
}

function arrayToPrettyString(array) {
  return `[${Array.apply([], array).join(",")}]`
}

////////////////////////   DENO
(async () => {
  const signatures = await deriveKeySignatures(password, salt, iterations);
  console.log(`Output: \n${signaturesToString(signatures)}`);
})();

////////////////////////   workerd
addEventListener("fetch", async event => {
  const signatures = await deriveKeySignatures(password, salt, iterations);
  const signaturesString = signaturesToString(signatures);
  console.log(`Output: \n${signaturesString}`);
  event.respondWith(new Response(signaturesString));
});

The snippet reuses slightly modified code from postgres deno driver:
https://github.com/denodrivers/postgres/blob/8a07131efa17f4a6bcab86fd81407f149de93449/connection/scram.ts#L93
and a test for that: https://github.com/denodrivers/postgres/blob/8a07131efa17f4a6bcab86fd81407f149de93449/tests/auth_test.ts#L6
based on https://www.rfc-editor.org/rfc/rfc7677

---

When launched from deno, it outputs the following:

~/work/workerd_configs master*​​ ❯ deno run ./crypto/crypto.ts
Input:
{
  "password": "pencil",
  "salt": "[91,109,153,104,157,18,53,142,236,160,75,20,18,54,250,129]",
  "iterations": 4096
}
================

Output:
{
  "client": "[166,15,201,35,214,126,134,68,169,45,22,185,110,218,94,244,101,107,12,114,92,72,67,116,190,37,83,85,118,153,110,139]",
  "server": "[193,243,203,193,193,58,157,53,161,76,9,144,238,217,118,41,234,34,88,99,229,102,164,49,74,185,159,63,0,229,217,213]",
  "stored": "[88,110,93,242,131,230,220,235,92,62,121,29,139,133,40,236,25,30,102,64,69,206,151,23,146,226,230,181,187,19,226,166]"
}

When deployed to the cloudflare worker, it outputs the following (sometimes, twice):

Input: 
{
  "password": "pencil",
  "salt": "[91,109,153,104,157,18,53,142,236,160,75,20,18,54,250,129]",
  "iterations": 4096
}
================

worker.js:96 Output: 
{
  "client": "[39,109,21,199,34,123,195,118,110,149,158,193,245,83,213,158,108,10,144,29,179,78,203,90,42,67,86,166,43,73,194,174]",
  "server": "[191,207,32,144,249,160,146,168,90,192,19,74,137,113,122,251,66,75,42,5,24,220,131,66,45,238,213,39,31,56,245,224]",
  "stored": "[177,174,171,65,56,228,4,105,247,107,133,55,230,13,215,114,1,133,143,68,20,119,43,239,251,225,142,245,250,194,40,71]"
}

When run with workerd (built from f7bd5f4) locally, it outputs the following:

work/workerd_configs/crypto master*​​ ❯ cat crypto.capnp
using Workerd = import "/workerd/workerd.capnp";

const config :Workerd.Config = (
  services = [
    (name = "main", worker = .mainWorker),
  ],

  sockets = [
    # Serve HTTP on port 8080.
    ( name = "http",
      address = "*:8080",
      http = (),
      service = "main"
    ),
  ]
);

const mainWorker :Workerd.Worker = (
  serviceWorkerScript = embed "crypto.ts",
  compatibilityDate = "2022-09-16",
);

work/workerd_configs/crypto master*​​ ❯ ../../workerd/bazel-bin/src/workerd/server/workerd serve "crypto.capnp"
workerd/io/worker-entrypoint.c++:209: error: e = kj/async-io-unix.c++:1652: failed: connect() blocked by restrictPeers()
stack: 104c30cf4 104c32160 104c1d5b8 104c1da5c 1049fbb6c 104a0c764 104a2f6c0 104a37308 104a540ac 104c0f8cc 104a540ac 102b59fa0; sentryErrorContext = workerEntrypoint
workerd/server/server.c++:1834: error: Uncaught exception: kj/async-io-unix.c++:1652: failed: worker_do_not_log; Request failed due to internal error
stack: 104c30cf4 104c32160 104c1d5b8 104c1da5c 1049fbb6c 104a0c764 104a2f6c0 104a37308 104a540ac 104c0f8cc 104a540ac 102b59fa0 104a066cc 104a097f0
workerd/io/worker-entrypoint.c++:209: error: e = kj/async-io-unix.c++:1652: failed: connect() blocked by restrictPeers()
stack: 104c30cf4 104c32160 104c1d5b8 104c1da5c 1049fbb6c 104a0c764 104a2f6c0 104a37308 104a540ac 104c0f8cc 104a540ac 102b59fa0; sentryErrorContext = workerEntrypoint
workerd/server/server.c++:1834: error: Uncaught exception: kj/async-io-unix.c++:1652: failed: worker_do_not_log; Request failed due to internal error
stack: 104c30cf4 104c32160 104c1d5b8 104c1da5c 1049fbb6c 104a0c764 104a2f6c0 104a37308 104a540ac 104c0f8cc 104a540ac 102b59fa0 104a066cc 104a097f0

and returns internal server error.
This is a bug, according to https://github.com/cloudflare/workerd/blame/f7bd5f40adb3909cefcc63b7b06a2d6c1baafbd5/README.md#L40

"internal errors" (bugs in the implementation which the Workers team should address)

---

I would expect both locally built workerd and the one from cloudflare to compute the signatures with their values identical to what Deno script produces for the same input.

[evanderkoogh]

Hey @SomeoneToIgnore, thanks for the report. There are two different issues here though. One is that something isn't supported in workerd that works on Cloudflare Workers, and the one where something that is working in Deno isn't working the same in Cloudflare Workers.

Now I am sure the team will fix the workerd to Cloudflare Workers problem, but the difference between Cloudflare Workers and Deno when it comes to the rest of the code is a bit more mysterious than that.

Now obviously it could be a problem in the implementation of either SubtleCrypto, but before we go that route, could you please verify that the output of the text_encode and decode functions are functions is the same?

I have spent way too much time in my life wasted trying to find bugs in crypto software that turned out to be text encoding/decoding issues :D

[SomeoneToIgnore]

Hey @evanderkoogh , thank you for the good advice.
I've used this snippet to check things:

const password = 'pencil'
const text_encoder = new TextEncoder();

function encodingExamples() {
  return `
  Password: ${password},
  Password encoded: ${text_encoder.encode(password)},
  'Client Key' encoded: ${text_encoder.encode("Client Key")},
  'Server Key' encoded: ${text_encoder.encode("Server Key")},
    `;
}

////////////////////////   DENO
(async () => {
  console.log(encodingExamples());
})();

////////////////////////   workerd
addEventListener("fetch", async event => {
  const response = encodingExamples();
  console.log(response);
  event.respondWith(new Response(response));
});

That covers all 3 places in the original snippet, where the encoder is used:

  const pbkdf2_password = await crypto.subtle.importKey(
    "raw",
    text_encoder.encode(password),
    "PBKDF2",
    false,
    ["deriveBits", "deriveKey"],
  );

  const client = new Uint8Array(
    await crypto.subtle.sign("HMAC", key, text_encoder.encode("Client Key")),
  );
  const server = new Uint8Array(
    await crypto.subtle.sign("HMAC", key, text_encoder.encode("Server Key")),
  );

---

Remote workerd:

Password: pencil,
  Password encoded: 112,101,110,99,105,108,
  'Client Key' encoded: 67,108,105,101,110,116,32,75,101,121,
  'Server Key' encoded: 83,101,114,118,101,114,32,75,101,121,

Local workerd (same version as in the issue):

  Password: pencil,
  Password encoded: 112,101,110,99,105,108,
  'Client Key' encoded: 67,108,105,101,110,116,32,75,101,121,
  'Server Key' encoded: 83,101,114,118,101,114,32,75,101,121,
    

Local deno:

  Password: pencil,
  Password encoded: 112,101,110,99,105,108,
  'Client Key' encoded: 67,108,105,101,110,116,32,75,101,121,
  'Server Key' encoded: 83,101,114,118,101,114,32,75,101,121,

---

Now obviously it could be a problem in the implementation of either SubtleCrypto, but before we go that route, could you please verify that the output of the text_encode and decode functions are functions is the same?

I'm not entirely sure what to check on the "decode" side, there's no such thing in the API: https://developer.mozilla.org/en-US/docs/Web/API/TextEncoder and the decode function from the snippet is not related to the issue, as was created to decode base64 salt from RFC the same way Deno Postgres driver does it with atob.

The decode is done in depth of crypto.subtle which then outputs keys that are different, as you can see in the initial example.
So I would not agree that Deno has to play a significant role in the case, but the underlying crypto library used.

Please feel free to ask me for more experiments/tests in case needed!

[kentonv]

Hi @SomeoneToIgnore,

In your latest comment it looks like you are saying that cloudflareworkers.com, local workerd, and local Deno all produced the same result? Is that right? So what is the issue at this point?

[SomeoneToIgnore]

@kentonv

I've double checked it with my latest comment what @evanderkoogh asked in his comment

could you please verify that the output of the text_encode and decode functions are functions is the same?

The output of these functions is the same, so whatever crypto.subtle accepts is identical in all 3 cases (including the text_encode transformation that happens).

---

Still, there are two major problems that exist:

• whatever's behind crypto.subtle in remote workerd, fails to produce a correct set of keys in deriveKeySignatures function. This is what I've tried to explain with the original snippet and the original text
• whatever's in local workerd (built from f7bd5f4) is unable to execute deriveKeySignatures at all, so I cannot debug the crypto stack deeper and understand what's exactly wrong there

Yet, I have provided you with the way to run the same thing with deno (just because it's also web-related, you can use anything else really), that produces completely different key output for the same inputs, using a combination of crypto.subtle.

This is definitely a bug, the same input (password, salt, iterations) should produce the same cryptographic keys everywhere, deno, remote workerd, local workerd, whatever else.

[kentonv]

Oh sorry, I misunderstood your second comment.

So the code you posted has a subtle bug unrelated to crypto:

addEventListener("fetch", async event => {
  const signatures = await deriveKeySignatures(password, salt, iterations);
  const signaturesString = signaturesToString(signatures);
  console.log(`Output: \n${signaturesString}`);
  event.respondWith(new Response(signaturesString));
});

Here, you are trying to use an async function as your event handler. Unfortunately, per the Service Workers standard, this doesn't work. What will happen here is the code will execute up to the first await, and then it will return a promise. The event handler system doesn't pay attention to that promise. Instead, it decides the event handler is done. But event.respondWith() hasn't been called yet. So, as specified in the Service Workers standard, the system falls back to "default handling". Default handling in Cloudflare Workers means passing the request on to the global fetch() function -- which, in hosted Cloudflare Workers, would mean going to origin, though in workerd it is kind of nonsensical.

When you run this in workerd, you are getting an error connect() blocked by restrictPeers(). This is because the request URL is presumably a local network (or localhost) address, and by default the global fetch() will refuse to fetch from the local network in order to defend against SSRF.

When you run this in Cloudflare Workers, the request will fall back to going to your origin... do you actually see a useful result in this case?

This weirdness is why we are trying to move away from Service Workers syntax and recommend ES modules syntax instead. If you use service workers syntax we highly recommend your event handler is always a single line: event.respondWith(handleEvent(event)). Then you can define handleEvent as a normal async function returning a Response.

Anyway, would you mind fixing this issue and trying again? You should then at least get identical results on workerd and cloudflareworkers.com, as they run the same code. I'm not sure if any of this could explain the difference from Deno, though.

[SomeoneToIgnore]

Thank you @kentonv for a great elaboration, I've managed to fix local workerd and eventually find the culprit: yet another subtle language difference and, indeed, a Deno bug: denoland/deno#16180

The most interesting, is that I rewrite the same snippet for node dialect, it fails to run with

node:internal/errors:464
    ErrorCaptureStackTrace(err);
    ^

TypeError [ERR_INVALID_ARG_TYPE]: The "length" argument must be of type number. Received undefined
    at Object.pbkdf2DeriveBits (node:internal/crypto/pbkdf2:101:3)
    at SubtleCrypto.deriveKey (node:internal/crypto/webcrypto:189:10)
    at deriveKeySignatures (/Users/someonetoignore/work/workerd_configs/crypto/crypto_node.ts:37:28)
    at processTicksAndRejections (node:internal/process/task_queues:96:5)
    at async /Users/someonetoignore/work/workerd_configs/crypto/crypto_node.ts:93:22 {
  code: 'ERR_INVALID_ARG_TYPE'

pointing to this line:

  const key = await subtle.deriveKey(
    {
      hash: "SHA-256",
      iterations,
      name: "PBKDF2",
      salt,
    },
    pbkdf2_password,
    { name: "HMAC", hash: "SHA-256" }, // <---------
    false,
    ["sign"],
  );

and looks like Deno, indeed, uses wrong block size here and Node forces everybody to fill this in despite the parameter being optional.

On the bright side, both of my original issues (wrong cryptographic numbers and example that failed to run in local workerd) seem to be solved. I can only note that the docs everywhere could use some love 🙂
Thank you again for your patience.

[kentonv]

Glad we figured it out!