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

Adds ENV boolean flag (DATABASE_SSL) for postgres ssl support #1433

Merged
merged 2 commits into from
Dec 30, 2023

Conversation

Jaredude
Copy link
Contributor

This is needed if hosting flowise data on a postgres server that requires ssl. In PostgreSQL v15, the default rds.force_ssl is 1 (on)

This is needed if hosting flowise data on a postgres server that requires ssl.
In PostgreSQL v15, the default rds.force_ssl is 1 (on)
@HenryHengZJ
Copy link
Contributor

thanks @Jaredude ! can you do a search on DATABASE_NAME, there are several places needed to be changed as well:

  • docker-compose.yml
  • .env.example
  • start.ts

@Jaredude
Copy link
Contributor Author

Jaredude commented Dec 28, 2023

Will update shortly... @HenryHengZJ I think I got everything.

@dkindlund
Copy link
Contributor

Hey @Jaredude and @HenryHengZJ , is there any chance you could also add support to allow connections to Postgres databases that use self-signed certificates? It seems passing in the rejectedUnauthorized option set to false doesn't work correctly -- as the pg client doesn't bother attempting any sort of TLS connection at all.

@HenryHengZJ
Copy link
Contributor

Hey @Jaredude and @HenryHengZJ , is there any chance you could also add support to allow connections to Postgres databases that use self-signed certificates? It seems passing in the rejectedUnauthorized option set to false doesn't work correctly -- as the pg client doesn't bother attempting any sort of TLS connection at all.

Do you have an example of how the database TypeORM configuration should looks like with self signed certificate?

@dkindlund
Copy link
Contributor

@HenryHengZJ , yes. From what I've read, these options are needed to force pg to use SSL/TLS but not attempt to validate self-signed certificates:

{  
  "ssl": {  
    "rejectUnauthorized": false,  
    "checkServerIdentity": () => undefined
  }  
}  

OR

{  
  "ssl": {  
    "rejectUnauthorized": false,  
    "checkServerIdentity": () => { return null; }
  }  
}  

So we're effectively disabling the checkServerIdentity function, which allows self-signed certificates to be accepted.

The problem right now, is that I can't actually specify the "checkServerIdentity": () => undefined line in the Flowise UI as part of Additional Parameters in the Postgres module, because the UI treats the () => undefined part as a literal string, unfortunately:

image

@Jaredude
Copy link
Contributor Author

@HenryHengZJ , yes. From what I've read, these options are needed to force pg to use SSL/TLS but not attempt to validate self-signed certificates:

{  
  "ssl": {  
    "rejectUnauthorized": false,  
    "checkServerIdentity": () => undefined
  }  
}  

OR

{  
  "ssl": {  
    "rejectUnauthorized": false,  
    "checkServerIdentity": () => { return null; }
  }  
}  

So we're effectively disabling the checkServerIdentity function, which allows self-signed certificates to be accepted.

The problem right now, is that I can't actually specify the "checkServerIdentity": () => undefined line in the Flowise UI as part of Additional Parameters in the Postgres module, because the UI treats the () => undefined part as a literal string, unfortunately:

image

We'd still want to validate a certificate
TypeORM postgres docs
reference node-postgres ssl document

To validate a self signed certificate, we'd need to do something along the following when we call new DataSource

{
  ...existing_setup,
  ssl: {
    rejectUnauthorized: false,
    ca: fs.readFileSync('/path/to/server-certificates/root.crt').toString(),
    key: fs.readFileSync('/path/to/client-key/postgresql.key').toString(),
    cert: fs.readFileSync('/path/to/client-certificates/postgresql.crt').toString(),
  },
}

@Jaredude
Copy link
Contributor Author

I looked into storing the cert values an ENV variables, since I think it would be one of the main purposes to using postgres is to not have to have a file system.

It can be done by converting it to base64 first and then we could set the values ssl using the following:
Buffer.from(process.env.SSL_CERT_BASE64, 'base64').toString('utf-8');

It would look like the following:

  ssl: {
    rejectUnauthorized: false,
    ca: Buffer.from(process.env.SSL_CA_BASE64, 'base64').toString(),
    key: Buffer.from(process.env.SSL_KEY_BASE64, 'base64').toString(),
    cert: Buffer.from(process.env.SSL_CERT_BASE64, 'base64').toString(),
  },

I don't have a Postgres setup with a self signed cert, but I can certainly modify this existing PR to make a change that allows simple ssl (already done) as well as self-signed ssl support via base64 ENV values.

@HenryHengZJ Any preference on whether to keep this PR as is with simple ssl support and make a separate PR for supporting self signed later?

@HenryHengZJ
Copy link
Contributor

@Jaredude I think we can keep this PR as simple ssl, and make another separate PR for self signed cert. The self signed cert should take higher priority if both DATABASE_SSL and SSL_KEY_BASE64 are present.

@dkindlund I think this is another issue/PR as you were mentioning about the PG vector integration in Flowise. Currently UI only takes in certain data types like string, number, array, not function, we'll need to figure something out there

@HenryHengZJ HenryHengZJ merged commit 0f03fe5 into FlowiseAI:main Dec 30, 2023
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants