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

Expose a way to restrict openssl signature algorithms #24818

Closed
annabokhan opened this issue Dec 3, 2018 · 15 comments
Closed

Expose a way to restrict openssl signature algorithms #24818

annabokhan opened this issue Dec 3, 2018 · 15 comments
Labels
crypto Issues and PRs related to the crypto subsystem. feature request Issues that request new features to be added to Node.js. stale

Comments

@annabokhan
Copy link

Is your feature request related to a problem? Please describe.
We are attempting to restrict client signature algorithms in node.js tls client. Since tls.createSecureContext does not expose an option to specify neither signature algorithms nor client signature algorithms, we've attempted to do so via a custom openssl configuration file (see example below), but unfortunately it does not work. Upon some investigation it looks like node is not calling SSL_CTX_config after creating the context.

openssl_conf = default_conf

[default_conf]

ssl_conf = ssl_sect

[ssl_sect]

system_default = ssl_default_sect

[ssl_default_sect]

ClientSignatureAlgorithms = ECDSA+SHA256
SignatureAlgorithms = ECDSA+SHA256

Describe the solution you'd like
We would prefer for the above to work as described - all configuration from a configuration file supplied to node via --openssl-config option (or env variable) should be applicable.
Additionally, these options should be exposed in tls.createSecureContext

Describe alternatives you've considered
The only alternative solution as of now is to use custom built openssl

@sam-github
Copy link
Contributor

Configuring the TLS cipher suite doesn't work for you? https://nodejs.org/api/tls.html#tls_modifying_the_default_tls_cipher_suite

@bnoordhuis bnoordhuis added question Issues that look for answers. crypto Issues and PRs related to the crypto subsystem. labels Dec 4, 2018
@annabokhan
Copy link
Author

@sam-github unfortunately not, or it wouldn't be an issue. We need an ability to limit signature algorithms.

@sam-github sam-github added the feature request Issues that request new features to be added to Node.js. label Dec 4, 2018
@sam-github
Copy link
Contributor

We'd have to expose long SSL_CTX_set1_client_sigalgs_list(SSL_CTX *ctx, const char *str);
& long SSL_set1_client_sigalgs_list(SSL *ssl, const char *str); as we did ciphers:. I'll look at it, but its not likely to hit a stable release for months.

I wasn't there when this was built (@bnoordhuis was, though). Still, I'll take a stab at justifying why it doesn't load the config.

Node is built to be standalone, easily copied to a production server and run there, and to run robustly in the face of various system configurations. In the same way that it builds its dependencies into it, its https/tls behaviour doesn't depend on the vagaries of your systems configuration. That has its ups and downs, in this case, its a downer for you because you have a useful system configuration, but it would nightmarish for many of users to try to understand and configure the system ssl. Assuming they even had rights to.

@annabokhan
Copy link
Author

@sam-github I understand your argument, but since the node already exposes a way to supply a custom openssl configuration file, I would argue that users should be able to configure anything that openSSL allows to configure, including signature algorithms. This would also be quite useful for example when building docker containers for base images shared across the org - instead of relying on each developer to specify secure TLS settings (min protocol, etc), we could enforce these in the common configuration file instead.

@sam-github
Copy link
Contributor

node already exposes a way to supply a custom openssl configuration file

It does? How? I don't recall seeing that.

@annabokhan
Copy link
Author

@sam-github
Copy link
Contributor

Ah, that. Yes, its passed to CONF_modules_load_file(), but that's all its used for ATM.

@annabokhan
Copy link
Author

Indeed, and that's sufficient for some settings, but in order for SSL configuration module (ssl_conf) settings to be applied, there needs to be an extra SSL_CTX_config call for each created context. Additionally, node could also expose a way to supply a name of the configuration to be passed to SSL_CTX_config allowing even further SSL customization

@bnoordhuis
Copy link
Member

bnoordhuis commented Dec 6, 2018

We are attempting to restrict client signature algorithms in node.js tls client.

You mention what but not why. Can you explain your use case?

As Sam mentioned, there are reasons why things work the way they do w.r.t. to the config file. The only reason Node.js even supports one is for FIPS compliance, and in retrospect that was probably unnecessary - the PR that introduced it could have benefited from more reviews.

@annabokhan
Copy link
Author

In our specific scenario the client does not support SHA-512 signatures due to limitations of the hardware (TPM to be specific), thus we need to exclude them

@bnoordhuis
Copy link
Member

Not sure I follow, let's make sure we're on the same page. Node.js is the TLS client, right? How exactly does the TPM factor in? Are you using a custom openssl engine to provide certificates or verification?

@annabokhan
Copy link
Author

That is correct, we are using a custom openssl engine for client authentication, since private key never leaves TPM.
Irregardless of our specific use case, do you agree that node lacks an option to configure client signature algorithms (signature algorithms as well) and do you think it would be useful to expose that capability?

@bnoordhuis
Copy link
Member

do you agree that node lacks an option to configure client signature algorithms (signature algorithms as well) and do you think it would be useful to expose that capability?

Yes; Sam already said as much. I was just trying to understand what you really wanted to accomplish. Oftentimes people ask for X when what they really need is Y.

@github-actions
Copy link
Contributor

There has been no activity on this feature request for 5 months and it is unlikely to be implemented. It will be closed 6 months after the last non-automated comment.

For more information on how the project manages feature requests, please consult the feature request management document.

@github-actions github-actions bot added the stale label May 15, 2022
@targos targos moved this from Pending Triage to Stale in Node.js feature requests May 15, 2022
@github-actions
Copy link
Contributor

There has been no activity on this feature request and it is being closed. If you feel closing this issue is not the right thing to do, please leave a comment.

For more information on how the project manages feature requests, please consult the feature request management document.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
crypto Issues and PRs related to the crypto subsystem. feature request Issues that request new features to be added to Node.js. stale
Projects
None yet
Development

No branches or pull requests

4 participants