Skip to content
This repository has been archived by the owner on Apr 22, 2023. It is now read-only.

added additional properties to getPeerCertificate #1286

Closed
wants to merge 1 commit into from
Closed

added additional properties to getPeerCertificate #1286

wants to merge 1 commit into from

Conversation

niclashoyer
Copy link

Added additional properties to getPeerCertificate, now includes
subjectAltName, Exponent and Modulus (FOAF+SSL friendly).

Patch written by Nathan,
http://groups.google.com/group/nodejs/browse_thread/thread/1d42da4cb2e51536

Added additional properties to getPeerCertificate, now includes
subjectAltName, Exponent and Modulus (FOAF+SSL friendly).

Patch written by Nathan,
http://groups.google.com/group/nodejs/browse_thread/thread/1d42da4cb2e51536
@bnoordhuis
Copy link
Member

If Nathan == Nathan Rixham, we need him to sign the CLA first. My two points from the ML thread also still stand.

  1. A comment should be added that OID 2.5.29.17 is Subject AltName
  2. The memset() / BIO_read() combos should be replaced with calls to BIO_gets() - handling of the nul byte is now more implicit than it needs to be (it took me a few seconds to figure it out). BIO_gets() takes care of it automatically and the memory BIO supports it so why not use it?

@niclashoyer
Copy link
Author

Oh okay. Is there any contact to Nathan? I just saw the patch on google groups and wondered why it isn't merged yet.

@bnoordhuis
Copy link
Member

Maybe revive the ML thread?

BIO* bio = BIO_new(BIO_s_mem());
BIO* bio = NULL;
ASN1_OBJECT *oid;
oid = OBJ_txt2obj("2.5.29.17", 1); // OID 2.5.29.17 is Subject AltName
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just use the NID_subject_alt_name instead of an OID?

https://github.com/pquerna/selene/blob/master/lib/core/certs.c#L427-458

@niclashoyer
Copy link
Author

I wrote to Nathan, but still no reply from him :(

There is no chance to get this into node if Nathan doesn't sign the CLA right?

I'll open an issue for that, maybe someone can look into this, so we don't need him to sign the CLA.

@bnoordhuis
Copy link
Member

Closing. We can't take the patch without a signed CLA.

@bnoordhuis bnoordhuis closed this Aug 28, 2011
@niclashoyer
Copy link
Author

Nathan has now signed the CLA, any chance to reopen this issue?

@bnoordhuis bnoordhuis reopened this Aug 30, 2011
@bnoordhuis
Copy link
Member

Reopened. I'll merge it when I have time to write a few tests for it (hint, hint).

@niclashoyer
Copy link
Author

There were merge conflicts after the last commit by koichik 6f60683

I merged manually, recommited the changes and added a test case.

I opened a new pull request, as I didn't manage to edit this pull request :)
Please see here: #1612

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants