Skip to content

Commit

Permalink
crypto: prettify othername in PrintGeneralName
Browse files Browse the repository at this point in the history
Refs: nodejs@466e541

PR-URL: nodejs#42123
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
  • Loading branch information
tniessen authored and xtx1130 committed Apr 25, 2022
1 parent 1714c6a commit a9a0bcc
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 26 deletions.
28 changes: 13 additions & 15 deletions src/crypto/crypto_common.cc
Original file line number Diff line number Diff line change
Expand Up @@ -692,9 +692,8 @@ static inline void PrintUtf8AltName(const BIOPointer& out,
true, safe_prefix);
}

// This function currently emulates the behavior of i2v_GENERAL_NAME in a safer
// and less ambiguous way.
// TODO(tniessen): gradually improve the format in the next major version(s)
// This function emulates the behavior of i2v_GENERAL_NAME in a safer and less
// ambiguous way. "othername:" entries use the GENERAL_NAME_print format.
static bool PrintGeneralName(const BIOPointer& out, const GENERAL_NAME* gen) {
if (gen->type == GEN_DNS) {
ASN1_IA5STRING* name = gen->d.dNSName;
Expand Down Expand Up @@ -767,33 +766,32 @@ static bool PrintGeneralName(const BIOPointer& out, const GENERAL_NAME* gen) {
OBJ_obj2txt(oline, sizeof(oline), gen->d.rid, true);
BIO_printf(out.get(), "Registered ID:%s", oline);
} else if (gen->type == GEN_OTHERNAME) {
// TODO(tniessen): the format that is used here is based on OpenSSL's
// implementation of i2v_GENERAL_NAME (as of OpenSSL 3.0.1), mostly for
// backward compatibility. It is somewhat awkward, especially when passed to
// translatePeerCertificate, and should be changed in the future, probably
// to the format used by GENERAL_NAME_print (in a major release).
// The format that is used here is based on OpenSSL's implementation of
// GENERAL_NAME_print (as of OpenSSL 3.0.1). Earlier versions of Node.js
// instead produced the same format as i2v_GENERAL_NAME, which was somewhat
// awkward, especially when passed to translatePeerCertificate.
bool unicode = true;
const char* prefix = nullptr;
// OpenSSL 1.1.1 does not support othername in i2v_GENERAL_NAME and may not
// define these NIDs.
// OpenSSL 1.1.1 does not support othername in GENERAL_NAME_print and may
// not define these NIDs.
#if OPENSSL_VERSION_MAJOR >= 3
int nid = OBJ_obj2nid(gen->d.otherName->type_id);
switch (nid) {
case NID_id_on_SmtpUTF8Mailbox:
prefix = " SmtpUTF8Mailbox:";
prefix = "SmtpUTF8Mailbox";
break;
case NID_XmppAddr:
prefix = " XmppAddr:";
prefix = "XmppAddr";
break;
case NID_SRVName:
prefix = " SRVName:";
prefix = "SRVName";
unicode = false;
break;
case NID_ms_upn:
prefix = " UPN:";
prefix = "UPN";
break;
case NID_NAIRealm:
prefix = " NAIRealm:";
prefix = "NAIRealm";
break;
}
#endif // OPENSSL_VERSION_MAJOR >= 3
Expand Down
22 changes: 11 additions & 11 deletions test/parallel/test-x509-escaping.js
Original file line number Diff line number Diff line change
Expand Up @@ -88,22 +88,22 @@ const { hasOpenSSL3 } = common;
// OpenSSL should not know it.
'Registered ID:1.3.9999.12.34',
hasOpenSSL3 ?
'othername: XmppAddr::abc123' :
'othername:XmppAddr:abc123' :
'othername:<unsupported>',
hasOpenSSL3 ?
'othername:" XmppAddr::abc123\\u002c DNS:good.example.com"' :
'othername:"XmppAddr:abc123\\u002c DNS:good.example.com"' :
'othername:<unsupported>',
hasOpenSSL3 ?
'othername:" XmppAddr::good.example.com\\u0000abc123"' :
'othername:"XmppAddr:good.example.com\\u0000abc123"' :
'othername:<unsupported>',
// This is unsupported because the OID is not recognized.
'othername:<unsupported>',
hasOpenSSL3 ? 'othername: SRVName::abc123' : 'othername:<unsupported>',
hasOpenSSL3 ? 'othername:SRVName:abc123' : 'othername:<unsupported>',
// This is unsupported because it is an SRVName with a UTF8String value,
// which is not allowed for SRVName.
'othername:<unsupported>',
hasOpenSSL3 ?
'othername:" SRVName::abc\\u0000def"' :
'othername:"SRVName:abc\\u0000def"' :
'othername:<unsupported>',
];

Expand Down Expand Up @@ -173,14 +173,14 @@ const { hasOpenSSL3 } = common;
},
},
hasOpenSSL3 ? {
text: 'OCSP - othername: XmppAddr::good.example.com\n' +
text: 'OCSP - othername:XmppAddr:good.example.com\n' +
'OCSP - othername:<unsupported>\n' +
'OCSP - othername: SRVName::abc123',
'OCSP - othername:SRVName:abc123',
legacy: {
'OCSP - othername': [
' XmppAddr::good.example.com',
'XmppAddr:good.example.com',
'<unsupported>',
' SRVName::abc123',
'SRVName:abc123',
],
},
} : {
Expand All @@ -196,10 +196,10 @@ const { hasOpenSSL3 } = common;
},
},
hasOpenSSL3 ? {
text: 'OCSP - othername:" XmppAddr::good.example.com\\u0000abc123"',
text: 'OCSP - othername:"XmppAddr:good.example.com\\u0000abc123"',
legacy: {
'OCSP - othername': [
' XmppAddr::good.example.com\0abc123',
'XmppAddr:good.example.com\0abc123',
],
},
} : {
Expand Down

0 comments on commit a9a0bcc

Please sign in to comment.