Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit d19da66

Browse filesBrowse files
indutnyMyles Borins
indutny
authored and
Myles Borins
committed
crypto: load PFX chain the same way as regular one
Load the certificate chain from the PFX file the same as we do it for a regular certificate chain. Fix: #4127 PR-URL: #4165 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
1 parent db0e906 commit d19da66
Copy full SHA for d19da66

File tree

Expand file treeCollapse file tree

4 files changed

+156
-55
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

4 files changed

+156
-55
lines changed
Open diff view settings
Collapse file

‎src/node_crypto.cc‎

Copy file name to clipboardExpand all lines: src/node_crypto.cc
+120-50Lines changed: 120 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -519,46 +519,35 @@ int SSL_CTX_get_issuer(SSL_CTX* ctx, X509* cert, X509** issuer) {
519519
}
520520

521521

522-
// Read a file that contains our certificate in "PEM" format,
523-
// possibly followed by a sequence of CA certificates that should be
524-
// sent to the peer in the Certificate message.
525-
//
526-
// Taken from OpenSSL - editted for style.
527522
int SSL_CTX_use_certificate_chain(SSL_CTX* ctx,
528-
BIO* in,
523+
X509* x,
524+
STACK_OF(X509)* extra_certs,
529525
X509** cert,
530526
X509** issuer) {
531-
int ret = 0;
532-
X509* x = nullptr;
527+
CHECK_EQ(*issuer, nullptr);
528+
CHECK_EQ(*cert, nullptr);
533529

534-
x = PEM_read_bio_X509_AUX(in, nullptr, CryptoPemCallback, nullptr);
535-
536-
if (x == nullptr) {
537-
SSLerr(SSL_F_SSL_CTX_USE_CERTIFICATE_CHAIN_FILE, ERR_R_PEM_LIB);
538-
goto end;
539-
}
540-
541-
ret = SSL_CTX_use_certificate(ctx, x);
530+
int ret = SSL_CTX_use_certificate(ctx, x);
542531

543532
if (ret) {
544533
// If we could set up our certificate, now proceed to
545534
// the CA certificates.
546-
X509 *ca;
547535
int r;
548-
unsigned long err;
549536

550537
if (ctx->extra_certs != nullptr) {
551538
sk_X509_pop_free(ctx->extra_certs, X509_free);
552539
ctx->extra_certs = nullptr;
553540
}
554541

555-
while ((ca = PEM_read_bio_X509(in, nullptr, CryptoPemCallback, nullptr))) {
542+
for (int i = 0; i < sk_X509_num(extra_certs); i++) {
543+
X509* ca = sk_X509_value(extra_certs, i);
544+
556545
// NOTE: Increments reference count on `ca`
557546
r = SSL_CTX_add1_chain_cert(ctx, ca);
558547

559548
if (!r) {
560-
X509_free(ca);
561549
ret = 0;
550+
*issuer = nullptr;
562551
goto end;
563552
}
564553
// Note that we must not free r if it was successfully
@@ -569,17 +558,8 @@ int SSL_CTX_use_certificate_chain(SSL_CTX* ctx,
569558
// Find issuer
570559
if (*issuer != nullptr || X509_check_issued(ca, x) != X509_V_OK)
571560
continue;
572-
*issuer = ca;
573-
}
574561

575-
// When the while loop ends, it's usually just EOF.
576-
err = ERR_peek_last_error();
577-
if (ERR_GET_LIB(err) == ERR_LIB_PEM &&
578-
ERR_GET_REASON(err) == PEM_R_NO_START_LINE) {
579-
ERR_clear_error();
580-
} else {
581-
// some real error
582-
ret = 0;
562+
*issuer = ca;
583563
}
584564
}
585565

@@ -592,13 +572,88 @@ int SSL_CTX_use_certificate_chain(SSL_CTX* ctx,
592572
// no need to free `store`
593573
} else {
594574
// Increment issuer reference count
595-
CRYPTO_add(&(*issuer)->references, 1, CRYPTO_LOCK_X509);
575+
*issuer = X509_dup(*issuer);
576+
if (*issuer == nullptr) {
577+
ret = 0;
578+
goto end;
579+
}
596580
}
597581
}
598582

599583
end:
584+
if (ret && x != nullptr) {
585+
*cert = X509_dup(x);
586+
if (*cert == nullptr)
587+
ret = 0;
588+
}
589+
return ret;
590+
}
591+
592+
593+
// Read a file that contains our certificate in "PEM" format,
594+
// possibly followed by a sequence of CA certificates that should be
595+
// sent to the peer in the Certificate message.
596+
//
597+
// Taken from OpenSSL - edited for style.
598+
int SSL_CTX_use_certificate_chain(SSL_CTX* ctx,
599+
BIO* in,
600+
X509** cert,
601+
X509** issuer) {
602+
X509* x = nullptr;
603+
604+
// Just to ensure that `ERR_peek_last_error` below will return only errors
605+
// that we are interested in
606+
ERR_clear_error();
607+
608+
x = PEM_read_bio_X509_AUX(in, nullptr, CryptoPemCallback, nullptr);
609+
610+
if (x == nullptr) {
611+
SSLerr(SSL_F_SSL_CTX_USE_CERTIFICATE_CHAIN_FILE, ERR_R_PEM_LIB);
612+
return 0;
613+
}
614+
615+
X509* extra = nullptr;
616+
int ret = 0;
617+
unsigned long err = 0;
618+
619+
// Read extra certs
620+
STACK_OF(X509)* extra_certs = sk_X509_new_null();
621+
if (extra_certs == nullptr) {
622+
SSLerr(SSL_F_SSL_CTX_USE_CERTIFICATE_CHAIN_FILE, ERR_R_MALLOC_FAILURE);
623+
goto done;
624+
}
625+
626+
while ((extra = PEM_read_bio_X509(in, nullptr, CryptoPemCallback, nullptr))) {
627+
if (sk_X509_push(extra_certs, extra))
628+
continue;
629+
630+
// Failure, free all certs
631+
goto done;
632+
}
633+
extra = nullptr;
634+
635+
// When the while loop ends, it's usually just EOF.
636+
err = ERR_peek_last_error();
637+
if (ERR_GET_LIB(err) == ERR_LIB_PEM &&
638+
ERR_GET_REASON(err) == PEM_R_NO_START_LINE) {
639+
ERR_clear_error();
640+
} else {
641+
// some real error
642+
goto done;
643+
}
644+
645+
ret = SSL_CTX_use_certificate_chain(ctx, x, extra_certs, cert, issuer);
646+
if (!ret)
647+
goto done;
648+
649+
done:
650+
if (extra_certs != nullptr)
651+
sk_X509_pop_free(extra_certs, X509_free);
652+
if (extra != nullptr)
653+
X509_free(extra);
600654
if (x != nullptr)
601-
*cert = x;
655+
X509_free(x);
656+
602657
return ret;
603658
}
604659

@@ -616,6 +671,16 @@ void SecureContext::SetCert(const FunctionCallbackInfo<Value>& args) {
616671
if (!bio)
617672
return;
618673

674+
// Free previous certs
675+
if (sc->issuer_ != nullptr) {
676+
X509_free(sc->issuer_);
677+
sc->issuer_ = nullptr;
678+
}
679+
if (sc->cert_ != nullptr) {
680+
X509_free(sc->cert_);
681+
sc->cert_ = nullptr;
682+
}
683+
619684
int rv = SSL_CTX_use_certificate_chain(sc->ctx_,
620685
bio,
621686
&sc->cert_,
@@ -887,7 +952,7 @@ void SecureContext::LoadPKCS12(const FunctionCallbackInfo<Value>& args) {
887952
PKCS12* p12 = nullptr;
888953
EVP_PKEY* pkey = nullptr;
889954
X509* cert = nullptr;
890-
STACK_OF(X509)* extraCerts = nullptr;
955+
STACK_OF(X509)* extra_certs = nullptr;
891956
char* pass = nullptr;
892957
bool ret = false;
893958

@@ -912,28 +977,33 @@ void SecureContext::LoadPKCS12(const FunctionCallbackInfo<Value>& args) {
912977
pass[passlen] = '\0';
913978
}
914979

980+
// Free previous certs
981+
if (sc->issuer_ != nullptr) {
982+
X509_free(sc->issuer_);
983+
sc->issuer_ = nullptr;
984+
}
985+
if (sc->cert_ != nullptr) {
986+
X509_free(sc->cert_);
987+
sc->cert_ = nullptr;
988+
}
989+
915990
if (d2i_PKCS12_bio(in, &p12) &&
916-
PKCS12_parse(p12, pass, &pkey, &cert, &extraCerts) &&
917-
SSL_CTX_use_certificate(sc->ctx_, cert) &&
991+
PKCS12_parse(p12, pass, &pkey, &cert, &extra_certs) &&
992+
SSL_CTX_use_certificate_chain(sc->ctx_,
993+
cert,
994+
extra_certs,
995+
&sc->cert_,
996+
&sc->issuer_) &&
918997
SSL_CTX_use_PrivateKey(sc->ctx_, pkey)) {
919-
// set extra certs
920-
while (X509* x509 = sk_X509_pop(extraCerts)) {
921-
if (!sc->ca_store_) {
922-
sc->ca_store_ = X509_STORE_new();
923-
SSL_CTX_set_cert_store(sc->ctx_, sc->ca_store_);
924-
}
925-
926-
X509_STORE_add_cert(sc->ca_store_, x509);
927-
SSL_CTX_add_client_CA(sc->ctx_, x509);
928-
X509_free(x509);
929-
}
998+
ret = true;
999+
}
9301000

1001+
if (pkey != nullptr)
9311002
EVP_PKEY_free(pkey);
1003+
if (cert != nullptr)
9321004
X509_free(cert);
933-
sk_X509_free(extraCerts);
934-
935-
ret = true;
936-
}
1005+
if (extra_certs != nullptr)
1006+
sk_X509_free(extra_certs);
9371007

9381008
PKCS12_free(p12);
9391009
BIO_free_all(in);
Collapse file

‎test/fixtures/keys/Makefile‎

Copy file name to clipboardExpand all lines: test/fixtures/keys/Makefile
+8Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,14 @@ agent1-cert.pem: agent1-csr.pem ca1-cert.pem ca1-key.pem
7979
-CAcreateserial \
8080
-out agent1-cert.pem
8181

82+
agent1-pfx.pem: agent1-cert.pem agent1-key.pem ca1-cert.pem
83+
openssl pkcs12 -export \
84+
-in agent1-cert.pem \
85+
-inkey agent1-key.pem \
86+
-certfile ca1-cert.pem \
87+
-out agent1-pfx.pem \
88+
-password pass:sample
89+
8290
agent1-verify: agent1-cert.pem ca1-cert.pem
8391
openssl verify -CAfile ca1-cert.pem agent1-cert.pem
8492

Collapse file

‎test/fixtures/keys/agent1-pfx.pem‎

Copy file name to clipboard
2.38 KB
Binary file not shown.
Collapse file

‎test/parallel/test-tls-ocsp-callback.js‎

Copy file name to clipboardExpand all lines: test/parallel/test-tls-ocsp-callback.js
+28-5Lines changed: 28 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -22,11 +22,7 @@ var constants = require('constants');
2222
var fs = require('fs');
2323
var join = require('path').join;
2424

25-
test({ response: false }, function() {
26-
test({ response: 'hello world' }, function() {
27-
test({ ocsp: false });
28-
});
29-
});
25+
var pfx = fs.readFileSync(join(common.fixturesDir, 'keys', 'agent1-pfx.pem'));
3026

3127
function test(testOptions, cb) {
3228

@@ -47,6 +43,13 @@ function test(testOptions, cb) {
4743
var ocspResponse;
4844
var session;
4945

46+
if (testOptions.pfx) {
47+
delete options.key;
48+
delete options.cert;
49+
options.pfx = testOptions.pfx;
50+
options.passphrase = testOptions.passphrase;
51+
}
52+
5053
var server = tls.createServer(options, function(cleartext) {
5154
cleartext.on('error', function(er) {
5255
// We're ok with getting ECONNRESET in this test, but it's
@@ -106,3 +109,23 @@ function test(testOptions, cb) {
106109
assert.equal(ocspCount, 1);
107110
});
108111
}
112+
113+
var tests = [
114+
{ response: false },
115+
{ response: 'hello world' },
116+
{ ocsp: false }
117+
];
118+
119+
if (!common.hasFipsCrypto) {
120+
tests.push({ pfx: pfx, passphrase: 'sample', response: 'hello pfx' });
121+
}
122+
123+
function runTests(i) {
124+
if (i === tests.length) return;
125+
126+
test(tests[i], common.mustCall(function() {
127+
runTests(i + 1);
128+
}));
129+
}
130+
131+
runTests(0);

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.