FlowFuse / helm

A Helm chart to deploy FlowFuse on Kubernetes
Apache License 2.0
5 stars 14 forks source link

Include Sequelize Encryption option #151

Closed Elexy closed 11 months ago

Elexy commented 1 year ago

Description

It would be nice to be able to connect to a Postgres db using encryption using the SSL parameter: https://sequelize.org/docs/v6/other-topics/dialect-specific-things/#postgresql

Currently the options are limited to: db.host Hostname of the Postgres Database. Default: postgres. db.database Database name on Postgres Server. Default: flowforge. db.user Username used when connecting to Postgres Server. db.password Password used when connecting to Postgres Server.

hardillb commented 1 year ago

Hello,

Thanks for raising this, just to be clear are you wanting to pass client TLS certificates or a custom CA cert to authenticate the remote DB?

I think if I've understood the doc correctly the sequelize engine will connect using SSL if the DB is configured correctly and you only need to pass these options if using client certs or a custom Certificate Authority to provide the DBs certificate.

If so we would also need to update the forge app to make use of these options if included.

Elexy commented 1 year ago

Sorry, I could have been a bit more complete. I am trying to connect to cockroachdb cloud which just requires SSL.

The error I am getting is:

ConnectionError [SequelizeConnectionError]: server requires encryption
    at Client._connectionCallback (/usr/src/forge/app/node_modules/sequelize/lib/dialects/postgres/connection-manager.js:143:24)
    at Client._handleErrorWhileConnecting (/usr/src/forge/app/node_modules/pg/lib/client.js:327:19)
    at Client._handleErrorMessage (/usr/src/forge/app/node_modules/pg/lib/client.js:347:19)
    at Connection.emit (node:events:513:28)
    at /usr/src/forge/app/node_modules/pg/lib/connection.js:117:12
    at Parser.parse (/usr/src/forge/app/node_modules/pg-protocol/dist/parser.js:40:17)
    at Socket.<anonymous> (/usr/src/forge/app/node_modules/pg-protocol/dist/index.js:11:42)
    at Socket.emit (node:events:513:28)
    at addChunk (node:internal/streams/readable:315:12)
    at readableAddChunk (node:internal/streams/readable:289:9) {
  parent: error: server requires encryption
      at Parser.parseErrorMessage (/usr/src/forge/app/node_modules/pg-protocol/dist/parser.js:287:98)
      at Parser.handlePacket (/usr/src/forge/app/node_modules/pg-protocol/dist/parser.js:126:29)
      at Parser.parse (/usr/src/forge/app/node_modules/pg-protocol/dist/parser.js:39:38)
      at Socket.<anonymous> (/usr/src/forge/app/node_modules/pg-protocol/dist/index.js:11:42)
      at Socket.emit (node:events:513:28)
      at addChunk (node:internal/streams/readable:315:12)
      at readableAddChunk (node:internal/streams/readable:289:9)
      at Socket.Readable.push (node:internal/streams/readable:228:10)
      at TCP.onStreamRead (node:internal/stream_base_commons:190:23) {
    length: 47,
    severity: 'FATAL',
    code: '08C00',
    detail: undefined,
    hint: undefined,
    position: undefined,
    internalPosition: undefined,
    internalQuery: undefined,
    where: undefined,
    schema: undefined,
    table: undefined,
    column: undefined,
    dataType: undefined,
35
    constraint: undefined,
    file: undefined,
    line: undefined,
    routine: undefined
  },
  original: error: server requires encryption
      at Parser.parseErrorMessage (/usr/src/forge/app/node_modules/pg-protocol/dist/parser.js:287:98)
      at Parser.handlePacket (/usr/src/forge/app/node_modules/pg-protocol/dist/parser.js:126:29)
      at Parser.parse (/usr/src/forge/app/node_modules/pg-protocol/dist/parser.js:39:38)
      at Socket.<anonymous> (/usr/src/forge/app/node_modules/pg-protocol/dist/index.js:11:42)
      at Socket.emit (node:events:513:28)
      at addChunk (node:internal/streams/readable:315:12)
      at readableAddChunk (node:internal/streams/readable:289:9)
      at Socket.Readable.push (node:internal/streams/readable:228:10)
      at TCP.onStreamRead (node:internal/stream_base_commons:190:23) {
    length: 47,
    severity: 'FATAL',
    code: '08C00',
    detail: undefined,
    hint: undefined,
    position: undefined,
    internalPosition: undefined,
    internalQuery: undefined,
    where: undefined,
    schema: undefined,
    table: undefined,
    column: undefined,
    dataType: undefined,
    constraint: undefined,
    file: undefined,
    line: undefined,
    routine: undefined
  }
}

For that particular PG service no certificates are required, but I assume it would be useful to have that as well as some point.

Happy to contribute to the helm chart but I think this also requires a change in the app configuration and code.

hardillb commented 1 year ago

OK, thanks

That helps clarify what you are asking for, but also leads to some other questions.

The Postgres docs (https://www.postgresql.org/docs/11/libpq-connect.html#id-1.7.3.8.3.6) imply that sslmode should default to prefer which should try and connect with SSL first then fallback to plain. If this is the case then I'm not sure how we got to that error.

We might be able to set the flag in the forge app to force it into prefer mode, but I'd want to test that first.

If I ran up a really simple nodejs app that just tries to connect to the database would you be willing to test it with your credentials against the cockroaddb instance?

Elexy commented 1 year ago

yeah, no problem.

otoh, cockraochdb cloud is free to try and realy quick to setup.

hardillb commented 1 year ago

I'm just setting up a test cockroachdb test account and it has asked me to download a CA certificate in order to be able to use it, so it does look like they are using none standard CA to sign their database certificate.

This would imply that the forge app is trying to connect using SSL, but failing to verify the cert (and the error message is misleading). I will test and find out.

hardillb commented 1 year ago

~It connects if we force sslmode: prefer into the options even without passing a custom CA cert, I have opened a PR to apply that flowforge/flowforge#2500~

~It should ship as FlowForge 1.10 in a couple of weeks all being well and no need to update the helm chart for now.~

It appears that sequelize doesn't pass the sslmode option through properly and just assumes ssl is required if set which we can't do.

I will have to look at it again.

Elexy commented 1 year ago

I'm just setting up a test cockroachdb test account and it has asked me to download a CA certificate in order to be able to use it, so it does look like they are using none standard CA to sign their database certificate.

This would imply that the forge app is trying to connect using SSL, but failing to verify the cert (and the error message is misleading). I will test and find out.

just some additional info: I was asked to download the cert as well. I guess there is an option to use SSL without verifying the cert, because I am able to use other apps, like budibase (also js based) without uploading a cert.

hardillb commented 1 year ago

Hmm, looks like the underlying driver is not supporting sslmode=prefer

https://github.com/brianc/node-postgres/issues/2720

So we will need to make it an explicit option

hardillb commented 11 months ago

I'm closing this as delivered in #154

phuynh-cm commented 7 months ago

Hey @Elexy , were you able to use cockroach DB instead of PostgreSQL when you installed FlowFuse using their helm chart?