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

crypto: use RFC2253 format in PrintGeneralName #42002

Merged
merged 1 commit into from Feb 22, 2022
Merged

crypto: use RFC2253 format in PrintGeneralName #42002

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
38 changes: 27 additions & 11 deletions src/crypto/crypto_common.cc
View file
Open in desktop
Expand Up @@ -48,6 +48,11 @@ static constexpr int X509_NAME_FLAGS =
XN_FLAG_SEP_MULTILINE |
XN_FLAG_FN_SN;

static constexpr int kX509NameFlagsRFC2253WithinUtf8JSON =
XN_FLAG_RFC2253 &
~ASN1_STRFLGS_ESC_MSB &
~ASN1_STRFLGS_ESC_CTRL;

int SSL_CTX_get_issuer(SSL_CTX* ctx, X509* cert, X509** issuer) {
X509_STORE* store = SSL_CTX_get_cert_store(ctx);
DeleteFnPtr<X509_STORE_CTX, X509_STORE_CTX_free> store_ctx(
Expand Down Expand Up @@ -740,20 +745,31 @@ static bool PrintGeneralName(const BIOPointer& out, const GENERAL_NAME* gen) {
// escaping.
PrintLatin1AltName(out, name);
} else if (gen->type == GEN_DIRNAME) {
// For backward compatibility, use X509_NAME_oneline to print the
// X509_NAME object. The format is non standard and should be avoided
// elsewhere, but conveniently, the function produces ASCII and the output
// is unlikely to contains commas or other characters that would require
// escaping. With that in mind, note that it SHOULD NOT produce ASCII
// output since an RFC5280 AttributeValue may be a UTF8String.
// TODO(tniessen): switch to RFC2253 rules in a major release
// Earlier versions of Node.js used X509_NAME_oneline to print the X509_NAME
// object. The format was non standard and should be avoided. The use of
// X509_NAME_oneline is discouraged by OpenSSL but was required for backward
// compatibility. Conveniently, X509_NAME_oneline produced ASCII and the
// output was unlikely to contains commas or other characters that would
// require escaping. However, it SHOULD NOT produce ASCII output since an
// RFC5280 AttributeValue may be a UTF8String.
// Newer versions of Node.js have since switched to X509_NAME_print_ex to
// produce a better format at the cost of backward compatibility. The new
// format may contain Unicode characters and it is likely to contain commas,
// which require escaping. Fortunately, the recently safeguarded function
// PrintAltName handles all of that safely.
BIO_printf(out.get(), "DirName:");
char oline[256];
if (X509_NAME_oneline(gen->d.dirn, oline, sizeof(oline)) != nullptr) {
PrintAltName(out, oline, strlen(oline), false, nullptr);
} else {
BIOPointer tmp(BIO_new(BIO_s_mem()));
CHECK(tmp);
if (X509_NAME_print_ex(tmp.get(),
gen->d.dirn,
0,
kX509NameFlagsRFC2253WithinUtf8JSON) < 0) {
return false;
}
char* oline = nullptr;
size_t n_bytes = BIO_get_mem_data(tmp.get(), &oline);
CHECK_IMPLIES(n_bytes != 0, oline != nullptr);
PrintAltName(out, oline, n_bytes, true, nullptr);
} else if (gen->type == GEN_IPADD) {
BIO_printf(out.get(), "IP Address:");
const ASN1_OCTET_STRING* ip = gen->d.ip;
Expand Down
30 changes: 15 additions & 15 deletions test/parallel/test-x509-escaping.js
View file
Open in desktop
Expand Up @@ -66,21 +66,21 @@ const { hasOpenSSL3 } = common;
'email:foo@example.com',
// ... but should be escaped if they contain commas.
'email:"foo@example.com\\u002c DNS:good.example.com"',
'DirName:/C=DE/L=Hannover',
// TODO(tniessen): support UTF8 in DirName
'DirName:"/C=DE/L=M\\\\xC3\\\\xBCnchen"',
'DirName:"/C=DE/L=Berlin\\u002c DNS:good.example.com"',
'DirName:"/C=DE/L=Berlin\\u002c DNS:good.example.com\\\\x00' +
'evil.example.com"',
'DirName:"/C=DE/L=Berlin\\u002c DNS:good.example.com\\\\\\\\x00' +
'evil.example.com"',
// These next two tests might be surprising. OpenSSL applies its own rules
// first, which introduce backslashes, which activate node's escaping.
// Unfortunately, there are also differences between OpenSSL 1.1.1 and 3.0.
'DirName:"/C=DE/L=Berlin\\\\x0D\\\\x0A"',
hasOpenSSL3 ?
'DirName:"/C=DE/L=Berlin\\\\/CN=good.example.com"' :
'DirName:/C=DE/L=Berlin/CN=good.example.com',
// New versions of Node.js use RFC2253 to print DirName entries, which
// almost always results in commas, which should be escaped properly.
'DirName:"L=Hannover\\u002cC=DE"',
// Node.js unsets ASN1_STRFLGS_ESC_MSB to prevent unnecessarily escaping
// Unicode characters, so Unicode characters should be preserved.
'DirName:"L=München\\u002cC=DE"',
'DirName:"L=Berlin\\\\\\u002c DNS:good.example.com\\u002cC=DE"',
// Node.js also unsets ASN1_STRFLGS_ESC_CTRL and relies on JSON-compatible
// escaping rules to safely escape control characters.
'DirName:"L=Berlin\\\\\\u002c DNS:good.example.com\\u0000' +
'evil.example.com\\u002cC=DE"',
'DirName:"L=Berlin\\\\\\u002c DNS:good.example.com\\\\\\\\\\u0000' +
'evil.example.com\\u002cC=DE"',
'DirName:"L=Berlin\\u000d\\u000a\\u002cC=DE"',
'DirName:"L=Berlin/CN=good.example.com\\u002cC=DE"',
// Even OIDs that are well-known (such as the following, which is
// sha256WithRSAEncryption) should be represented numerically only.
'Registered ID:1.2.840.113549.1.1.11',
Expand Down