Skip to content

Commit

Permalink
crypto: load PFX chain the same way as regular one
Browse files Browse the repository at this point in the history 
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 <[email protected]>
  • Loading branch information
@indutny
indutny committed Dec 18, 2015
1 parent 56985ca commit a2c1799
Show file tree
Hide file tree
Showing 4 changed files with 156 additions and 55 deletions.
170 changes: 120 additions & 50 deletions src/node_crypto.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -517,46 +517,35 @@ int SSL_CTX_get_issuer(SSL_CTX* ctx, X509* cert, X509** issuer) {
}


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

x = PEM_read_bio_X509_AUX(in, nullptr, CryptoPemCallback, nullptr);

if (x == nullptr) {
SSLerr(SSL_F_SSL_CTX_USE_CERTIFICATE_CHAIN_FILE, ERR_R_PEM_LIB);
goto end;
}

ret = SSL_CTX_use_certificate(ctx, x);
int ret = SSL_CTX_use_certificate(ctx, x);

if (ret) {
// If we could set up our certificate, now proceed to
// the CA certificates.
X509 *ca;
int r;
unsigned long err;

if (ctx->extra_certs != nullptr) {
sk_X509_pop_free(ctx->extra_certs, X509_free);
ctx->extra_certs = nullptr;
}

while ((ca = PEM_read_bio_X509(in, nullptr, CryptoPemCallback, nullptr))) {
for (int i = 0; i < sk_X509_num(extra_certs); i++) {
X509* ca = sk_X509_value(extra_certs, i);

// NOTE: Increments reference count on `ca`
r = SSL_CTX_add1_chain_cert(ctx, ca);

if (!r) {
X509_free(ca);
ret = 0;
*issuer = nullptr;
goto end;
}
// Note that we must not free r if it was successfully
Expand All @@ -567,17 +556,8 @@ int SSL_CTX_use_certificate_chain(SSL_CTX* ctx,
// Find issuer
if (*issuer != nullptr || X509_check_issued(ca, x) != X509_V_OK)
continue;
*issuer = ca;
}

// When the while loop ends, it's usually just EOF.
err = ERR_peek_last_error();
if (ERR_GET_LIB(err) == ERR_LIB_PEM &&
ERR_GET_REASON(err) == PEM_R_NO_START_LINE) {
ERR_clear_error();
} else {
// some real error
ret = 0;
*issuer = ca;
}
}

Expand All @@ -590,13 +570,88 @@ int SSL_CTX_use_certificate_chain(SSL_CTX* ctx,
// no need to free `store`
} else {
// Increment issuer reference count
CRYPTO_add(&(*issuer)->references, 1, CRYPTO_LOCK_X509);
*issuer = X509_dup(*issuer);
if (*issuer == nullptr) {
ret = 0;
goto end;
}
}
}

end:
if (ret && x != nullptr) {
*cert = X509_dup(x);
if (*cert == nullptr)
ret = 0;
}
return ret;
}


// Read a file that contains our certificate in "PEM" format,
// possibly followed by a sequence of CA certificates that should be
// sent to the peer in the Certificate message.
//
// Taken from OpenSSL - edited for style.
int SSL_CTX_use_certificate_chain(SSL_CTX* ctx,
BIO* in,
X509** cert,
X509** issuer) {
X509* x = nullptr;

// Just to ensure that `ERR_peek_last_error` below will return only errors
// that we are interested in
ERR_clear_error();

x = PEM_read_bio_X509_AUX(in, nullptr, CryptoPemCallback, nullptr);

if (x == nullptr) {
SSLerr(SSL_F_SSL_CTX_USE_CERTIFICATE_CHAIN_FILE, ERR_R_PEM_LIB);
return 0;
}

X509* extra = nullptr;
int ret = 0;
unsigned long err = 0;

// Read extra certs
STACK_OF(X509)* extra_certs = sk_X509_new_null();
if (extra_certs == nullptr) {
SSLerr(SSL_F_SSL_CTX_USE_CERTIFICATE_CHAIN_FILE, ERR_R_MALLOC_FAILURE);
goto done;
}

while ((extra = PEM_read_bio_X509(in, nullptr, CryptoPemCallback, nullptr))) {
if (sk_X509_push(extra_certs, extra))
continue;

// Failure, free all certs
goto done;
}
extra = nullptr;

// When the while loop ends, it's usually just EOF.
err = ERR_peek_last_error();
if (ERR_GET_LIB(err) == ERR_LIB_PEM &&
ERR_GET_REASON(err) == PEM_R_NO_START_LINE) {
ERR_clear_error();
} else {
// some real error
goto done;
}

ret = SSL_CTX_use_certificate_chain(ctx, x, extra_certs, cert, issuer);
if (!ret)
goto done;

done:
if (extra_certs != nullptr)
sk_X509_pop_free(extra_certs, X509_free);
if (extra != nullptr)
X509_free(extra);
if (x != nullptr)
*cert = x;
X509_free(x);

return ret;
}

Expand All @@ -614,6 +669,16 @@ void SecureContext::SetCert(const FunctionCallbackInfo<Value>& args) {
if (!bio)
return;

// Free previous certs
if (sc->issuer_ != nullptr) {
X509_free(sc->issuer_);
sc->issuer_ = nullptr;
}
if (sc->cert_ != nullptr) {
X509_free(sc->cert_);
sc->cert_ = nullptr;
}

int rv = SSL_CTX_use_certificate_chain(sc->ctx_,
bio,
&sc->cert_,
Expand Down Expand Up @@ -888,7 +953,7 @@ void SecureContext::LoadPKCS12(const FunctionCallbackInfo<Value>& args) {
PKCS12* p12 = nullptr;
EVP_PKEY* pkey = nullptr;
X509* cert = nullptr;
STACK_OF(X509)* extraCerts = nullptr;
STACK_OF(X509)* extra_certs = nullptr;
char* pass = nullptr;
bool ret = false;

Expand All @@ -913,28 +978,33 @@ void SecureContext::LoadPKCS12(const FunctionCallbackInfo<Value>& args) {
pass[passlen] = '\0';
}

// Free previous certs
if (sc->issuer_ != nullptr) {
X509_free(sc->issuer_);
sc->issuer_ = nullptr;
}
if (sc->cert_ != nullptr) {
X509_free(sc->cert_);
sc->cert_ = nullptr;
}

if (d2i_PKCS12_bio(in, &p12) &&
PKCS12_parse(p12, pass, &pkey, &cert, &extraCerts) &&
SSL_CTX_use_certificate(sc->ctx_, cert) &&
PKCS12_parse(p12, pass, &pkey, &cert, &extra_certs) &&
SSL_CTX_use_certificate_chain(sc->ctx_,
cert,
extra_certs,
&sc->cert_,
&sc->issuer_) &&
SSL_CTX_use_PrivateKey(sc->ctx_, pkey)) {
// set extra certs
while (X509* x509 = sk_X509_pop(extraCerts)) {
if (!sc->ca_store_) {
sc->ca_store_ = X509_STORE_new();
SSL_CTX_set_cert_store(sc->ctx_, sc->ca_store_);
}

X509_STORE_add_cert(sc->ca_store_, x509);
SSL_CTX_add_client_CA(sc->ctx_, x509);
X509_free(x509);
}
ret = true;
}

if (pkey != nullptr)
EVP_PKEY_free(pkey);
if (cert != nullptr)
X509_free(cert);
sk_X509_free(extraCerts);

ret = true;
}
if (extra_certs != nullptr)
sk_X509_free(extra_certs);

PKCS12_free(p12);
BIO_free_all(in);
Expand Down
8 changes: 8 additions & 0 deletions test/fixtures/keys/Makefile
View file
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,14 @@ agent1-cert.pem: agent1-csr.pem ca1-cert.pem ca1-key.pem
-CAcreateserial \
-out agent1-cert.pem

agent1-pfx.pem: agent1-cert.pem agent1-key.pem ca1-cert.pem
openssl pkcs12 -export \
-in agent1-cert.pem \
-inkey agent1-key.pem \
-certfile ca1-cert.pem \
-out agent1-pfx.pem \
-password pass:sample

agent1-verify: agent1-cert.pem ca1-cert.pem
openssl verify -CAfile ca1-cert.pem agent1-cert.pem

Expand Down
Binary file added test/fixtures/keys/agent1-pfx.pem
View file
Binary file not shown.
33 changes: 28 additions & 5 deletions test/parallel/test-tls-ocsp-callback.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,7 @@ var constants = require('constants');
var fs = require('fs');
var join = require('path').join;

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

function test(testOptions, cb) {

Expand All @@ -47,6 +43,13 @@ function test(testOptions, cb) {
var ocspResponse;
var session;

if (testOptions.pfx) {
delete options.key;
delete options.cert;
options.pfx = testOptions.pfx;
options.passphrase = testOptions.passphrase;
}

var server = tls.createServer(options, function(cleartext) {
cleartext.on('error', function(er) {
// We're ok with getting ECONNRESET in this test, but it's
Expand Down Expand Up @@ -106,3 +109,23 @@ function test(testOptions, cb) {
assert.equal(ocspCount, 1);
});
}

var tests = [
{ response: false },
{ response: 'hello world' },
{ ocsp: false }
];

if (!common.hasFipsCrypto) {
tests.push({ pfx: pfx, passphrase: 'sample', response: 'hello pfx' });
}

function runTests(i) {
if (i === tests.length) return;

test(tests[i], common.mustCall(function() {
runTests(i + 1);
}));
}

runTests(0);

0 comments on commit a2c1799

Please sign in to comment.