feat(backend): provision private key #638

sabineschaller · 2022-09-26T10:49:17Z

Changes proposed in this pull request

load private key or create one
part of backend config

Context

fixes Self-provisioned keys #606

Checklist

Related issues linked using fixes #number
Tests added/updated
Documentation added
Make sure that all checks pass

packages/backend/src/config/app.ts

sabineschaller · 2022-09-27T08:52:57Z

Should this PR also add the key to the internal key registry?

packages/backend/src/config/app.ts

wilsonianb · 2022-09-27T14:37:19Z

packages/backend/src/config/app.ts

+  if (!fs.existsSync('./tmp')) {
+    fs.mkdirSync('./tmp')
+  }
+  fs.writeFileSync(


This will overwrite an existing private.pem if the provided PRIVATE_KEY_PATH couldn't be loaded, which is probably ok 🤔

I was wondering that, too. But I don't see a reason to keep the old ones as long as the new one is also associated with this RS. And by associating I mean adding it to the internal key registry. Should this PR also do that?

I'm leaning towards not overwriting.
I don't like the idea of potentially overwriting a file not intended for for Rafiki, however unlikely such a misconfiguration may be.

And by associating I mean adding it to the internal key registry. Should this PR also do that?

I think we should hold off until we decide if we're going to switch the key registry to using JWKS

feat: add JWKS endpoint open-payments#190

I'm leaning towards not overwriting.
I don't like the idea of potentially overwriting a file not intended for for Rafiki, however unlikely such a misconfiguration may be.

Should we use unique tmp dirs?

I think we should hold off until we decide if we're going to switch the key registry to using JWKS

I'll create an issue to also create a payment pointer for the Rafiki instance itself that will then be linked to the key, in whatever way and form we end up doing that (probably JWKS endpoint).

packages/backend/src/config/app.ts

packages/backend/src/config/app.test.ts

packages/backend/src/config/app.ts

packages/backend/.gitignore

packages/backend/src/config/app.test.ts

wilsonianb · 2022-10-11T13:20:04Z

packages/backend/src/config/app.test.ts

+      expect(key).toBeInstanceOf(crypto.KeyObject)
+      expect(jwk).toMatchObject({
+        crv: 'Ed25519',
+        kty: 'OKP',
+        d: expect.any(String),
+        x: expect.any(String)
+      })
+      expect(key.export({ format: 'pem', type: 'pkcs8' })).toEqual(
+        keypair.privateKey.export({ format: 'pem', type: 'pkcs8' })
+      )


https://nodejs.org/api/crypto.html#keyobjectequalsotherkeyobject

Suggested change

expect(key).toBeInstanceOf(crypto.KeyObject)

expect(jwk).toMatchObject({

crv: 'Ed25519',

kty: 'OKP',

d: expect.any(String),

x: expect.any(String)

})

expect(key.export({ format: 'pem', type: 'pkcs8' })).toEqual(

keypair.privateKey.export({ format: 'pem', type: 'pkcs8' })

)

assert.ok(key.equals(keypair.privateKey))

expect(jwk).toEqual(key.export({ format: 'jwk' }))

expect(jwk).toEqual({

crv: 'Ed25519',

kty: 'OKP',

d: expect.any(String),

x: expect.any(String)

})

Property 'equals' does not exist on type 'KeyObject'.

😕
https://nodejs.org/dist/latest-v16.x/docs/api/crypto.html#keyobjectequalsotherkeyobject
Is that a typescript build error?

yes

src/config/app.test.ts:59:21 - error TS2339: Property 'equals' does not exist on type 'KeyObject'. 59 assert.ok(key.equals(keypair.privateKey)) ~~~~~~ Found 1 error.

I even made sure to use node v16.18.0.

packages/backend/src/config/app.test.ts

wilsonianb · 2022-10-11T13:36:22Z

packages/backend/src/config/app.ts

+  if (!fs.existsSync(TMP_DIR)) {
+    fs.mkdirSync(TMP_DIR)
+  }
+  fs.writeFileSync(
+    PRIVATE_KEY_FILE,


Now I'm wondering if parseOrProvisionKey should only operate with keyPath as opposed to TMP_DIR/PRIVATE_KEY_FILE, to prevent an operator from not realizing that the default PRIVATE_KEY_PATH is being used and not their configured one (even though that is being logged).

I'm not sure I understand. PRIVATE_KEY_FILE does include the path.

As in parseOrProvisionKey would only read or write (maybe not overwrite) the file at keyPath.
As is, it reads from keyPath but writes to PRIVATE_KEY_FILE.

But what if it only has read access and not write access at keyPath?

I'm imagining an operator running multiple instances of rafiki backend in a k8s cluster and configuring PRIVATE_KEY_FILE as a path to a secret.
In which case, I think we should throw if we can't use (or write to) the configured file (keyPath).
Otherwise you end up with individual instances provisioning their own private keys.

wilsonianb · 2022-10-11T13:43:44Z

packages/backend/src/config/app.ts

+  if (!fs.existsSync('./tmp')) {
+    fs.mkdirSync('./tmp')
+  }
+  fs.writeFileSync(


I'm leaning towards not overwriting.
I don't like the idea of potentially overwriting a file not intended for for Rafiki, however unlikely such a misconfiguration may be.

packages/backend/src/config/app.ts

mkurapov

I was able to test successfully, both with existing and new, generated key.

As you two are discussing in the comment, similarly, I do see the benefit of maybe having the parseOrProvisionKey take undefined as a possible argument, and doing the keyPath generation inside the method itself. This way if a defined keyPath is given, we know its's coming from a user-set config, not a default value, in which case we will throw as @wilsonianb mentions

mkurapov · 2022-10-25T11:07:12Z

packages/backend/src/config/app.ts

+    try {
+      const key = crypto.createPrivateKey(fs.readFileSync(keyFile))
+      const jwk = key.export({ format: 'jwk' })
+      if (jwk.crv === 'Ed25519') {


It would be nice to have 'Ed25519' coming from a type declaration, but it looks like we are limited to just a string here.

Maybe we could eventually do schema validation using:
https://github.com/interledger/open-payments/blob/b125873edb6d33acd40eb304e026e5111ec1dba9/openapi/shared/schemas.yaml#L201-L234

wilsonianb · 2022-10-25T14:42:31Z

Merging/rebasing main should resolve the failing Docker checks

* feat(backend): provision private key * fix(backend): check key curve * fix(backend): key loading error handling * feat(backend): store private key in tmp folder * fix(backend): check if dir exists, move tmp folder into config folder * refactor(backend): use const string for tmp dir and key file * test(backend): key provisioning * test(backend): include further checks * feat(backend): provision key returns key object and jwk * test(backend): add Brandon's suggestions * revert(backend): only return key * feat(backend): don't overwrite keyfiles * chore(backend): re-add line in gitignore * feat(backend): throw if key file cannot be read

feat(backend): provision private key

0ed473a

sabineschaller requested a review from wilsonianb September 26, 2022 10:49

github-actions bot added pkg: backend Changes in the backend package. type: source Changes business logic labels Sep 26, 2022

sabineschaller added 2 commits September 26, 2022 14:23

fix(backend): check key curve

f929569

fix(backend): key loading error handling

85ecb93

wilsonianb previously approved these changes Sep 26, 2022

View reviewed changes

wilsonianb reviewed Sep 26, 2022

View reviewed changes

packages/backend/src/config/app.ts Show resolved Hide resolved

packages/backend/src/config/app.ts Outdated Show resolved Hide resolved

feat(backend): store private key in tmp folder

01cc65c

sabineschaller dismissed wilsonianb’s stale review via 01cc65c September 27, 2022 08:12

sabineschaller requested a review from wilsonianb September 27, 2022 08:12

sabineschaller mentioned this pull request Sep 27, 2022

Potentially allow encrypted private key files #642

Closed

wilsonianb reviewed Sep 27, 2022

View reviewed changes

packages/backend/src/config/app.ts Outdated Show resolved Hide resolved

packages/backend/src/config/app.ts Outdated Show resolved Hide resolved

packages/backend/src/config/app.ts Outdated Show resolved Hide resolved

fix(backend): check if dir exists, move tmp folder into config folder

990768c

sabineschaller requested a review from wilsonianb September 27, 2022 14:10

wilsonianb reviewed Sep 27, 2022

View reviewed changes

sabineschaller added 2 commits September 29, 2022 10:47

refactor(backend): use const string for tmp dir and key file

28980ee

test(backend): key provisioning

41a2976

github-actions bot added the type: tests Testing related label Sep 29, 2022

sabineschaller requested a review from wilsonianb September 29, 2022 09:49

wilsonianb reviewed Sep 29, 2022

View reviewed changes

packages/backend/src/config/app.test.ts Outdated Show resolved Hide resolved

packages/backend/src/config/app.test.ts Outdated Show resolved Hide resolved

wilsonianb reviewed Oct 5, 2022

View reviewed changes

packages/backend/src/config/app.ts Outdated Show resolved Hide resolved

sabineschaller added 2 commits October 10, 2022 11:50

test(backend): include further checks

f79b15a

feat(backend): provision key returns key object and jwk

69c5744

sabineschaller requested a review from wilsonianb October 10, 2022 10:52

wilsonianb reviewed Oct 11, 2022

View reviewed changes

packages/backend/src/config/app.ts Outdated Show resolved Hide resolved

sabineschaller added 2 commits October 12, 2022 13:06

test(backend): add Brandon's suggestions

cfa602f

revert(backend): only return key

36f0864

feat(backend): don't overwrite keyfiles

e137013

sabineschaller requested a review from wilsonianb October 14, 2022 09:07

wilsonianb mentioned this pull request Oct 14, 2022

Docker build workflows timing out #673

Closed

mkurapov previously approved these changes Oct 21, 2022

View reviewed changes

chore(backend): re-add line in gitignore

b5cbf5a

sabineschaller dismissed mkurapov’s stale review via b5cbf5a October 25, 2022 01:26

feat(backend): throw if key file cannot be read

fd718dc

sabineschaller requested a review from mkurapov October 25, 2022 01:51

mkurapov approved these changes Oct 25, 2022

View reviewed changes

sabineschaller merged commit 92c89a3 into main Oct 26, 2022

sabineschaller deleted the s2-key-provisioning branch October 26, 2022 03:28

wilsonianb mentioned this pull request Nov 28, 2022

Create Payment Pointer for Rafiki instance itself #668

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(backend): provision private key #638

feat(backend): provision private key #638

sabineschaller commented Sep 26, 2022

sabineschaller commented Sep 27, 2022

wilsonianb Sep 27, 2022

sabineschaller Sep 27, 2022

wilsonianb Oct 11, 2022

wilsonianb Oct 11, 2022

sabineschaller Oct 12, 2022

sabineschaller Oct 12, 2022 •

edited

Loading

wilsonianb Oct 11, 2022

sabineschaller Oct 12, 2022

wilsonianb Oct 12, 2022

sabineschaller Oct 14, 2022

wilsonianb Oct 11, 2022

sabineschaller Oct 12, 2022

wilsonianb Oct 12, 2022

sabineschaller Oct 14, 2022

wilsonianb Oct 14, 2022 •

edited

Loading

wilsonianb Oct 11, 2022

mkurapov left a comment

mkurapov Oct 25, 2022

wilsonianb Oct 25, 2022

wilsonianb commented Oct 25, 2022

feat(backend): provision private key #638

feat(backend): provision private key #638

Conversation

sabineschaller commented Sep 26, 2022

Changes proposed in this pull request

Context

Checklist

sabineschaller commented Sep 27, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sabineschaller Oct 12, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wilsonianb Oct 14, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mkurapov left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wilsonianb commented Oct 25, 2022

sabineschaller Oct 12, 2022 •

edited

Loading

wilsonianb Oct 14, 2022 •

edited

Loading