FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

crypto: use RFC2253 format in PrintGeneralName · nodejs/node@563b2ed · GitHub

/ node Public

Commit 563b2ed

Browse files
tniessen
authored
crypto: use RFC2253 format in PrintGeneralName
For backward compatibility, node uses X509_NAME_oneline to format X509_NAME entries in PrintGeneralName. However, the format produced by this function is non-standard and its use is discouraged. It also does not handle Unicode names correctly. This change switches to X509_NAME_print_ex with flags that produce an RFC2253-compatible format. Non-ASCII strings are converted to UTF-8 and preserved in the output. Control characters are not escaped by OpenSSL when producing the RFC2253 format because they will be escaped by node in a JSON-compatible manner afterwards. PR-URL: #42002 Refs: #42001 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
1 parent 16650ee commit 563b2ed

2 files changed

Lines changed: 42 additions & 26 deletions

File tree

‎src/crypto/crypto_common.cc‎

Lines changed: 27 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,11 @@ static constexpr int kX509NameFlagsMultiline =
4848
XN_FLAG_SEP_MULTILINE |
4949
XN_FLAG_FN_SN;
5050

51+
static constexpr int kX509NameFlagsRFC2253WithinUtf8JSON =
52+
XN_FLAG_RFC2253 &
53+
~ASN1_STRFLGS_ESC_MSB &
54+
~ASN1_STRFLGS_ESC_CTRL;
55+
5156
int SSL_CTX_get_issuer(SSL_CTX* ctx, X509* cert, X509** issuer) {
5257
X509_STORE* store = SSL_CTX_get_cert_store(ctx);
5358
DeleteFnPtr<X509_STORE_CTX, X509_STORE_CTX_free> store_ctx(
@@ -740,20 +745,31 @@ static bool PrintGeneralName(const BIOPointer& out, const GENERAL_NAME* gen) {
740745
// escaping.
741746
PrintLatin1AltName(out, name);
742747
} else if (gen->type == GEN_DIRNAME) {
743-
// For backward compatibility, use X509_NAME_oneline to print the
744-
// X509_NAME object. The format is non standard and should be avoided
745-
// elsewhere, but conveniently, the function produces ASCII and the output
746-
// is unlikely to contains commas or other characters that would require
747-
// escaping. With that in mind, note that it SHOULD NOT produce ASCII
748-
// output since an RFC5280 AttributeValue may be a UTF8String.
749-
// TODO(tniessen): switch to RFC2253 rules in a major release
748+
// Earlier versions of Node.js used X509_NAME_oneline to print the X509_NAME
749+
// object. The format was non standard and should be avoided. The use of
750+
// X509_NAME_oneline is discouraged by OpenSSL but was required for backward
751+
// compatibility. Conveniently, X509_NAME_oneline produced ASCII and the
752+
// output was unlikely to contains commas or other characters that would
753+
// require escaping. However, it SHOULD NOT produce ASCII output since an
754+
// RFC5280 AttributeValue may be a UTF8String.
755+
// Newer versions of Node.js have since switched to X509_NAME_print_ex to
756+
// produce a better format at the cost of backward compatibility. The new
757+
// format may contain Unicode characters and it is likely to contain commas,
758+
// which require escaping. Fortunately, the recently safeguarded function
759+
// PrintAltName handles all of that safely.
750760
BIO_printf(out.get(), "DirName:");
751-
char oline[256];
752-
if (X509_NAME_oneline(gen->d.dirn, oline, sizeof(oline)) != nullptr) {
753-
PrintAltName(out, oline, strlen(oline), false, nullptr);
754-
} else {
761+
BIOPointer tmp(BIO_new(BIO_s_mem()));
762+
CHECK(tmp);
763+
if (X509_NAME_print_ex(tmp.get(),
764+
gen->d.dirn,
765+
0,
766+
kX509NameFlagsRFC2253WithinUtf8JSON) < 0) {
755767
return false;
756768
}
769+
char* oline = nullptr;
770+
size_t n_bytes = BIO_get_mem_data(tmp.get(), &oline);
771+
CHECK_IMPLIES(n_bytes != 0, oline != nullptr);
772+
PrintAltName(out, oline, n_bytes, true, nullptr);
757773
} else if (gen->type == GEN_IPADD) {
758774
BIO_printf(out.get(), "IP Address:");
759775
const ASN1_OCTET_STRING* ip = gen->d.ip;

‎test/parallel/test-x509-escaping.js‎

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -66,21 +66,21 @@ const { hasOpenSSL3 } = common;
6666
'email:foo@example.com',
6767
// ... but should be escaped if they contain commas.
6868
'email:"foo@example.com\\u002c DNS:good.example.com"',
69-
'DirName:/C=DE/L=Hannover',
70-
// TODO(tniessen): support UTF8 in DirName
71-
'DirName:"/C=DE/L=M\\\\xC3\\\\xBCnchen"',
72-
'DirName:"/C=DE/L=Berlin\\u002c DNS:good.example.com"',
73-
'DirName:"/C=DE/L=Berlin\\u002c DNS:good.example.com\\\\x00' +
74-
'evil.example.com"',
75-
'DirName:"/C=DE/L=Berlin\\u002c DNS:good.example.com\\\\\\\\x00' +
76-
'evil.example.com"',
77-
// These next two tests might be surprising. OpenSSL applies its own rules
78-
// first, which introduce backslashes, which activate node's escaping.
79-
// Unfortunately, there are also differences between OpenSSL 1.1.1 and 3.0.
80-
'DirName:"/C=DE/L=Berlin\\\\x0D\\\\x0A"',
81-
hasOpenSSL3 ?
82-
'DirName:"/C=DE/L=Berlin\\\\/CN=good.example.com"' :
83-
'DirName:/C=DE/L=Berlin/CN=good.example.com',
69+
// New versions of Node.js use RFC2253 to print DirName entries, which
70+
// almost always results in commas, which should be escaped properly.
71+
'DirName:"L=Hannover\\u002cC=DE"',
72+
// Node.js unsets ASN1_STRFLGS_ESC_MSB to prevent unnecessarily escaping
73+
// Unicode characters, so Unicode characters should be preserved.
74+
'DirName:"L=München\\u002cC=DE"',
75+
'DirName:"L=Berlin\\\\\\u002c DNS:good.example.com\\u002cC=DE"',
76+
// Node.js also unsets ASN1_STRFLGS_ESC_CTRL and relies on JSON-compatible
77+
// escaping rules to safely escape control characters.
78+
'DirName:"L=Berlin\\\\\\u002c DNS:good.example.com\\u0000' +
79+
'evil.example.com\\u002cC=DE"',
80+
'DirName:"L=Berlin\\\\\\u002c DNS:good.example.com\\\\\\\\\\u0000' +
81+
'evil.example.com\\u002cC=DE"',
82+
'DirName:"L=Berlin\\u000d\\u000a\\u002cC=DE"',
83+
'DirName:"L=Berlin/CN=good.example.com\\u002cC=DE"',
8484
// Even OIDs that are well-known (such as the following, which is
8585
// sha256WithRSAEncryption) should be represented numerically only.
8686
'Registered ID:1.2.840.113549.1.1.11',

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL