Skip to content

Conversation

@mbwhite
Copy link
Contributor

@mbwhite mbwhite commented Dec 19, 2018

This is the PR71 but with corrected formatting as per the discussion in PR71
Thank you to those who worked on fix; aim here is to get the fix merged as it is needed by
many projects

Signed-off-by: Matthew B. White [email protected]

This is the PR71 but with corrected formatting as per the discussion in PR71
Thank you to those who worked on fix; aim here is to get the fix merged as it is needed by
many projects

Signed-off-by: Matthew B. White <[email protected]>
@sstone1
Copy link

sstone1 commented Dec 19, 2018

@yorkie could you look at this PR please?

@yorkie
Copy link
Collaborator

yorkie commented Dec 19, 2018

This looks good to me, thank you @mbwhite @sstone1 :)

@yorkie yorkie merged commit 6033d4b into Southern:master Dec 19, 2018
@yorkie
Copy link
Collaborator

yorkie commented Dec 19, 2018

Released as [email protected], either.

@mbwhite
Copy link
Contributor Author

mbwhite commented Dec 19, 2018

Thank you @yorkie much appreciated!!

@mbwhite
Copy link
Contributor Author

mbwhite commented Dec 19, 2018

@yorkie we've observed in large scale testing that some extra unicode characters are appearing...

{ keyUsage: 'Certificate Sign',
  basicConstraints: 'CA:FALSE',
  subjectKeyIdentifier: 'D8:28:B4:C0:BC:92:4A:D3:C3:8C:54:6C:08:86:33:10:A6:8D:83:AE',
  authorityKeyIdentifier: 'keyid:C4:B3:FE:76:0D:E2:DE:3C:FC:75:FB:AE:55:86:04:F0:BB:7F:F6:01',
  subjectAlternativeName: 'DNS:Anils-MacBook-Pro.local',
  '1.2.3.4.5.6.7.8.1': '\u0004\u001a{"attrs":{"attr1":"val1"}}' }

This is the 'cert.extensions'. To your knowledge is this a mistake in the certificate we're using or somehow related to the changes?

@kohend
Copy link

kohend commented Jan 29, 2019

There's a bug in this PR: https://github.com/Southern/node-x509/blame/master/src/x509.cc#L280 should give the subject key algorithm as before and it gives the signature algorithm, same as https://github.com/Southern/node-x509/blame/master/src/x509.cc#L241

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.

4 participants