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.

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

tls: get the local certificate after tls handshake #24261

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions doc/api/tls.md
Original file line number Diff line number Diff line change
Expand Up @@ -566,6 +566,22 @@ added: v0.11.4
Always returns `true`. This may be used to distinguish TLS sockets from regular
`net.Socket` instances.

### tlsSocket.getCertificate()
<!-- YAML
added: REPLACEME
-->

* Returns: {Object}

Returns an object representing the local certificate. The returned object has
some properties corresponding to the fields of the certificate.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you be more specific than "some"?

Copy link
Contributor Author

@sam-github sam-github Nov 9, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pretty vague, isn't it! ;-) This is the extent of the current documentation for parsed cert objects, unchanged since 0c42fac51596a68a6be02ea537e5ec03f228844b. Its on my list to fix, but for now I'd like to leave as-is. The wording of these docs are close to identical as possible to the docs for https://nodejs.org/api/tls.html#tls_tlssocket_getpeercertificate_detailed. I'm working on a PR to add EC and DH key info to X509ToObject, and I promise to doc the cert format then, and reorganize the docs, since the cert can show up in a handful of APIs.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/to @bnoordhuis as promised, #24358


See [`tls.TLSSocket.getPeerCertificate()`][] for an example of the certificate
structure.

If there is no local certificate, an empty object will be returned. If the
socket has been destroyed, `null` will be returned.

### tlsSocket.getCipher()
<!-- YAML
added: v0.11.4
Expand Down Expand Up @@ -658,6 +674,7 @@ certificate.
```

If the peer does not provide a certificate, an empty object will be returned.
If the socket has been destroyed, `null` will be returned.

### tlsSocket.getPeerFinished()
<!-- YAML
Expand Down
3 changes: 3 additions & 0 deletions lib/_tls_common.js
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,9 @@ exports.createSecureContext = function createSecureContext(options) {
return c;
};

// Translate some fields from the handle's C-friendly format into more idiomatic
// javascript object representations before passing them back to the user. Can
// be used on any cert object, but changing the name would be semver-major.
exports.translatePeerCertificate = function translatePeerCertificate(c) {
if (!c)
return null;
Expand Down
12 changes: 11 additions & 1 deletion lib/_tls_wrap.js
Original file line number Diff line number Diff line change
Expand Up @@ -660,7 +660,17 @@ TLSSocket.prototype.setSession = function(session) {
TLSSocket.prototype.getPeerCertificate = function(detailed) {
if (this._handle) {
return common.translatePeerCertificate(
this._handle.getPeerCertificate(detailed));
this._handle.getPeerCertificate(detailed)) || {};
}

return null;
};

TLSSocket.prototype.getCertificate = function() {
if (this._handle) {
// It's not a peer cert, but the formatting is identical.
return common.translatePeerCertificate(
this._handle.getCertificate()) || {};
}

return null;
Expand Down
23 changes: 21 additions & 2 deletions src/node_crypto.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1409,6 +1409,7 @@ void SSLWrap<Base>::AddMethods(Environment* env, Local<FunctionTemplate> t) {
HandleScope scope(env->isolate());

env->SetProtoMethodNoSideEffect(t, "getPeerCertificate", GetPeerCertificate);
env->SetProtoMethodNoSideEffect(t, "getCertificate", GetCertificate);
env->SetProtoMethodNoSideEffect(t, "getFinished", GetFinished);
env->SetProtoMethodNoSideEffect(t, "getPeerFinished", GetPeerFinished);
env->SetProtoMethodNoSideEffect(t, "getSession", GetSession);
Expand Down Expand Up @@ -1871,8 +1872,26 @@ void SSLWrap<Base>::GetPeerCertificate(
}

done:
if (result.IsEmpty())
result = Object::New(env->isolate());
args.GetReturnValue().Set(result);
}


template <class Base>
void SSLWrap<Base>::GetCertificate(
const FunctionCallbackInfo<Value>& args) {
Base* w;
ASSIGN_OR_RETURN_UNWRAP(&w, args.Holder());
Environment* env = w->ssl_env();

ClearErrorOnReturn clear_error_on_return;

Local<Object> result;

X509Pointer cert(SSL_get_certificate(w->ssl_.get()));

if (cert)
result = X509ToObject(env, cert.get());

args.GetReturnValue().Set(result);
}

Expand Down
1 change: 1 addition & 0 deletions src/node_crypto.h
Original file line number Diff line number Diff line change
Expand Up @@ -272,6 +272,7 @@ class SSLWrap {

static void GetPeerCertificate(
const v8::FunctionCallbackInfo<v8::Value>& args);
static void GetCertificate(const v8::FunctionCallbackInfo<v8::Value>& args);
static void GetFinished(const v8::FunctionCallbackInfo<v8::Value>& args);
static void GetPeerFinished(const v8::FunctionCallbackInfo<v8::Value>& args);
static void GetSession(const v8::FunctionCallbackInfo<v8::Value>& args);
Expand Down
12 changes: 10 additions & 2 deletions test/parallel/test-tls-peer-certificate-multi-keys.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ if (!common.hasCrypto)

const assert = require('assert');
const tls = require('tls');
const util = require('util');
const fixtures = require('../common/fixtures');

const options = {
Expand All @@ -37,13 +36,22 @@ const options = {
const server = tls.createServer(options, function(cleartext) {
cleartext.end('World');
});

server.once('secureConnection', common.mustCall(function(socket) {
const cert = socket.getCertificate();
// The server's local cert is the client's peer cert.
assert.deepStrictEqual(
cert.subject.OU,
['Information Technology', 'Engineering', 'Marketing']
);
}));

server.listen(0, common.mustCall(function() {
const socket = tls.connect({
port: this.address().port,
rejectUnauthorized: false
}, common.mustCall(function() {
const peerCert = socket.getPeerCertificate();
console.error(util.inspect(peerCert));
assert.deepStrictEqual(
peerCert.subject.OU,
['Information Technology', 'Engineering', 'Marketing']
Expand Down
2 changes: 2 additions & 0 deletions test/parallel/test-tls-peer-certificate.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@ connect({
}, function(err, pair, cleanup) {
assert.ifError(err);
const socket = pair.client.conn;
const localCert = socket.getCertificate();
assert.deepStrictEqual(localCert, {});
let peerCert = socket.getPeerCertificate();
assert.ok(!peerCert.issuerCertificate);

Expand Down
4 changes: 4 additions & 0 deletions test/parallel/test-tls-pfx-authorizationerror.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ const server = tls
rejectUnauthorized: false
},
common.mustCall(function(c) {
assert.strictEqual(c.getPeerCertificate().serialNumber,
'FAD50CC6A07F516C');
assert.strictEqual(c.authorizationError, null);
c.end();
})
Expand All @@ -35,6 +37,8 @@ const server = tls
rejectUnauthorized: false
},
function() {
assert.strictEqual(client.getCertificate().serialNumber,
'FAD50CC6A07F516C');
client.end();
server.close();
}
Expand Down