-
-
Notifications
You must be signed in to change notification settings - Fork 6.1k
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
chore: replace node-forge
with @peculiar/x509
#8584
Conversation
ba76491
to
49f495a
Compare
const signingAlgorithm = { | ||
name: 'ECDSA', | ||
namedCurve: 'P-256', | ||
hash: 'SHA-256' | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ECDSA produces much shorter keys than RSA while achieving the same security level. Also, ECDSA certificates are supported by all the modern browsers (source).
extensions: [ | ||
new SubjectAlternativeNameExtension({ | ||
dns: ['localhost', 'localhost.localdomain', 'lvh.me', '*.lvh.me'], | ||
ip: ['127.0.0.1', '::1'] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fe80::1
has been replaced with ::1
here, because the former is a link-local address rather than a loopback address.
signingKey: privateKey, | ||
extensions: [ | ||
new SubjectAlternativeNameExtension({ | ||
dns: ['localhost', 'localhost.localdomain', 'lvh.me', '*.lvh.me'], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[::1]
is removed since it isn't a valid hostname.
Is webcrypto polyfill required by the library or used only to generate private keys? |
It is required by the library, which needs a crypto provider that is compatible with WebCrypto API. Fortunately, Node.js >= 15 has native support for WebCrypto, so we can remove the polyfill once Node 14 reaches its EOL. |
We've removed |
Description
This PR replaces
node-forge
with@peculiar/x509
as discussed in #8532 in order to reduce the bundle size.Additional context
I don't have a Windows development environment, but it seems that the reason why this CI task fails is irrelevant to this PR.
What is the purpose of this pull request?
Before submitting the PR, please make sure you do the following
fixes #123
).