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.

Sign up for GitHub

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

Jump to bottom

getPeerX509Certificate() supports getting chain of certificates #44905

Closed
vinayak-kukreja opened this issue Oct 6, 2022 · 5 comments · Fixed by #44935
Closed

getPeerX509Certificate() supports getting chain of certificates #44905

vinayak-kukreja opened this issue Oct 6, 2022 · 5 comments · Fixed by #44935
Labels
feature request Issues that request new features to be added to Node.js. tls Issues and PRs related to the tls subsystem.

Comments

@vinayak-kukreja
Copy link

What is the problem this feature will solve?

Trying to verify if a certificate is a root certificate involves having the subject and the issuer as same in PeerCertificate and knowing the the certificate was signed by CA.
I see we can get a chain of certificates using getPeerCertificate(true) but it does not have a property to verify if ca: true like the X509Certificate.

What is the feature you are proposing to solve the problem?

Wondering if it would be possible to either include this property in PeerCertificate or allowing chaining in getPeerX509Certificate().

What alternatives have you considered?

I did not find a way to verify ca: true in PeerCertificate. Let me know if you believe this can be done with current chaining that is available.
@vinayak-kukreja vinayak-kukreja added the feature request Issues that request new features to be added to Node.js. label Oct 6, 2022
@bnoordhuis bnoordhuis mentioned this issue Oct 9, 2022
Merged
bnoordhuis added a commit to bnoordhuis/io.js that referenced this issue Oct 9, 2022
@bnoordhuis
tls: add "ca" property to certificate object
13c5d8e 
The objects returned by getPeerCertificate() now have an additional "ca"
boolean property that indicates whether the certificate is a Certificate
Authority certificate or not.

Fixes: nodejs#44905
bnoordhuis added a commit to bnoordhuis/io.js that referenced this issue Oct 9, 2022
@bnoordhuis
tls: add "ca" property to certificate object
62f5d07 
The objects returned by getPeerCertificate() now have an additional "ca"
boolean property that indicates whether the certificate is a Certificate
Authority certificate or not.

Fixes: nodejs#44905
@bnoordhuis
Copy link
Member

#44935

@bnoordhuis bnoordhuis added the tls Issues and PRs related to the tls subsystem. label Oct 9, 2022
@vinayak-kukreja
Copy link
Author

Thank you for working on this, really appreciate it. :)
I see the PR has multiple approvals, is there a timeline followed for merging and deploying the change?

@jasnell
Copy link
Member

jasnell commented Oct 10, 2022

PRs stay open a minimum of 48 hours in general and must pass CI before they can land.

@vinayak-kukreja
Copy link
Author

Hey, following up here. When could it be possible to get this merged?

@targos targos moved this to Pending Triage in Node.js feature requests Oct 22, 2022
@vinayak-kukreja
Copy link
Author

Hey, any updates about when the fix can be merged?

nodejs-github-bot pushed a commit that referenced this issue Nov 9, 2022
@bnoordhuis
tls: add "ca" property to certificate object
9fe6dcf 
The objects returned by getPeerCertificate() now have an additional "ca"
boolean property that indicates whether the certificate is a Certificate
Authority certificate or not.

Fixes: #44905
PR-URL: #44935
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Michael Dawson <midawson@redhat.com>
lucshi pushed a commit to lucshi/node that referenced this issue Nov 9, 2022
@bnoordhuis
tls: add "ca" property to certificate object
abbdb22 
The objects returned by getPeerCertificate() now have an additional "ca"
boolean property that indicates whether the certificate is a Certificate
Authority certificate or not.

Fixes: nodejs#44905
PR-URL: nodejs#44935
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Michael Dawson <midawson@redhat.com>
RafaelGSS pushed a commit that referenced this issue Nov 10, 2022
@bnoordhuis @RafaelGSS
tls: add "ca" property to certificate object
bf868fd 
The objects returned by getPeerCertificate() now have an additional "ca"
boolean property that indicates whether the certificate is a Certificate
Authority certificate or not.

Fixes: #44905
PR-URL: #44935
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Michael Dawson <midawson@redhat.com>
danielleadams pushed a commit that referenced this issue Dec 30, 2022
@bnoordhuis @danielleadams
tls: add "ca" property to certificate object
5aca24b 
The objects returned by getPeerCertificate() now have an additional "ca"
boolean property that indicates whether the certificate is a Certificate
Authority certificate or not.

Fixes: #44905
PR-URL: #44935
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Michael Dawson <midawson@redhat.com>
danielleadams pushed a commit that referenced this issue Dec 30, 2022
@bnoordhuis @danielleadams
tls: add "ca" property to certificate object
8debd79 
The objects returned by getPeerCertificate() now have an additional "ca"
boolean property that indicates whether the certificate is a Certificate
Authority certificate or not.

Fixes: #44905
PR-URL: #44935
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Michael Dawson <midawson@redhat.com>
danielleadams pushed a commit that referenced this issue Jan 3, 2023
@bnoordhuis @danielleadams
tls: add "ca" property to certificate object
10cc827 
The objects returned by getPeerCertificate() now have an additional "ca"
boolean property that indicates whether the certificate is a Certificate
Authority certificate or not.

Fixes: #44905
PR-URL: #44935
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Michael Dawson <midawson@redhat.com>
danielleadams pushed a commit that referenced this issue Jan 4, 2023
@bnoordhuis @danielleadams
tls: add "ca" property to certificate object
9b231be 
The objects returned by getPeerCertificate() now have an additional "ca"
boolean property that indicates whether the certificate is a Certificate
Authority certificate or not.

Fixes: #44905
PR-URL: #44935
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Michael Dawson <midawson@redhat.com>
@avivkeller avivkeller moved this from Awaiting Triage to Done in Node.js feature requests Aug 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
feature request Issues that request new features to be added to Node.js. tls Issues and PRs related to the tls subsystem.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

tls: add "ca" property to certificate object bnoordhuis/io.js
3 participants
@bnoordhuis @jasnell @vinayak-kukreja