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

pki.verifyCertificateChain fails to accept intermediate certificates in caStore #188

Open
bantu opened this issue Nov 16, 2014 · 11 comments

Comments

@bantu
Copy link

bantu commented Nov 16, 2014

Assume the following certificate chain:

hostname / common name
intermediate CA
root CA

Scenario 1

Put (only) root CA into caStore.
Then, pki.verifyCertificateChain correctly verifies that hostname chains up to a trust anchor.

Scenario 2

Put (only) intermediate CA into caStore.
Then, pki.verifyCertificateChain fails to verify that hostname chains up to a trust anchor.

Problem

This behavior is suboptimal, because pinning an intermediate CA usually provides higher/better security.

E.g.

  • Someone might have higher trust into C=US/O=Google Inc/CN=Google Internet Authority G2 than C=US/O=GeoTrust Inc./CN=GeoTrust Global CA or C=US/O=Equifax/OU=Equifax Secure Certificate Authority
  • The signature of intermediate onto hostname (say 4096 bit RSA) might be stronger than root CA onto intermediate CA (say 1024 bit RSA).
  • In case your company/instituation is the intermediate CA, you have higher trust into your security department than into an external root CA.

Furthermore it might be useful to also be able to pinn non-CA certificates directly. This has been previously discussed in #96 (comment) though.

@dlongley
Copy link
Member

Hi @bantu,

Can you submit a simplified code example w/certificate PEMs demonstrating the issue?

@bantu
Copy link
Author

bantu commented Nov 17, 2014

@dlongley There you go.

#!/usr/bin/env node
"use strict";

var root = " \
-----BEGIN CERTIFICATE----- \
MIIDIDCCAomgAwIBAgIENd70zzANBgkqhkiG9w0BAQUFADBOMQswCQYDVQQGEwJV \
UzEQMA4GA1UEChMHRXF1aWZheDEtMCsGA1UECxMkRXF1aWZheCBTZWN1cmUgQ2Vy \
dGlmaWNhdGUgQXV0aG9yaXR5MB4XDTk4MDgyMjE2NDE1MVoXDTE4MDgyMjE2NDE1 \
MVowTjELMAkGA1UEBhMCVVMxEDAOBgNVBAoTB0VxdWlmYXgxLTArBgNVBAsTJEVx \
dWlmYXggU2VjdXJlIENlcnRpZmljYXRlIEF1dGhvcml0eTCBnzANBgkqhkiG9w0B \
AQEFAAOBjQAwgYkCgYEAwV2xWGcIYu6gmi0fCG2RFGiYCh7+2gRvE4RiIcPRfM6f \
BeC4AfBONOziipUEZKzxa1NfBbPLZ4C/QgKO/t0BCezhABRP/PvwDN1Dulsr4R+A \
cJkVV5MW8Q+XarfCaCMczE1ZMKxRHjuvK9buY0V7xdlfUNLjUA86iOe/FP3gx7kC \
AwEAAaOCAQkwggEFMHAGA1UdHwRpMGcwZaBjoGGkXzBdMQswCQYDVQQGEwJVUzEQ \
MA4GA1UEChMHRXF1aWZheDEtMCsGA1UECxMkRXF1aWZheCBTZWN1cmUgQ2VydGlm \
aWNhdGUgQXV0aG9yaXR5MQ0wCwYDVQQDEwRDUkwxMBoGA1UdEAQTMBGBDzIwMTgw \
ODIyMTY0MTUxWjALBgNVHQ8EBAMCAQYwHwYDVR0jBBgwFoAUSOZo+SvSspXXR9gj \
IBBPM5iQn9QwHQYDVR0OBBYEFEjmaPkr0rKV10fYIyAQTzOYkJ/UMAwGA1UdEwQF \
MAMBAf8wGgYJKoZIhvZ9B0EABA0wCxsFVjMuMGMDAgbAMA0GCSqGSIb3DQEBBQUA \
A4GBAFjOKer89961zgK5F7WF0bnj4JXMJTENAKaSbn+2kmOeUJXRmm/kEd5jhW6Y \
7qj/WsjTVbJmcVfewCHrPSqnI0kBBIZCe/zuf6IWUrVnZ9NA2zsmWLIodz2uFHdh \
1voqZiegDfqnc1zqcPGUIWVEX/r87yloqaKHee9570+sB3c4 \
-----END CERTIFICATE----- \
"

var intermediate = " \
-----BEGIN CERTIFICATE----- \
MIID8DCCAtigAwIBAgIDAjp2MA0GCSqGSIb3DQEBBQUAMEIxCzAJBgNVBAYTAlVT \
MRYwFAYDVQQKEw1HZW9UcnVzdCBJbmMuMRswGQYDVQQDExJHZW9UcnVzdCBHbG9i \
YWwgQ0EwHhcNMTMwNDA1MTUxNTU1WhcNMTYxMjMxMjM1OTU5WjBJMQswCQYDVQQG \
EwJVUzETMBEGA1UEChMKR29vZ2xlIEluYzElMCMGA1UEAxMcR29vZ2xlIEludGVy \
bmV0IEF1dGhvcml0eSBHMjCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEB \
AJwqBHdc2FCROgajguDYUEi8iT/xGXAaiEZ+4I/F8YnOIe5a/mENtzJEiaB0C1NP \
VaTOgmKV7utZX8bhBYASxF6UP7xbSDj0U/ck5vuR6RXEz/RTDfRK/J9U3n2+oGtv \
h8DQUB8oMANA2ghzUWx//zo8pzcGjr1LEQTrfSTe5vn8MXH7lNVg8y5Kr0LSy+rE \
ahqyzFPdFUuLH8gZYR/Nnag+YyuENWllhMgZxUYi+FOVvuOAShDGKuy6lyARxzmZ \
EASg8GF6lSWMTlJ14rbtCMoU/M4iarNOz0YDl5cDfsCx3nuvRTPPuj5xt970JSXC \
DTWJnZ37DhF5iR43xa+OcmkCAwEAAaOB5zCB5DAfBgNVHSMEGDAWgBTAephojYn7 \
qwVkDBF9qn1luMrMTjAdBgNVHQ4EFgQUSt0GFhu89mi1dvWBtrtiGrpagS8wEgYD \
VR0TAQH/BAgwBgEB/wIBADAOBgNVHQ8BAf8EBAMCAQYwNQYDVR0fBC4wLDAqoCig \
JoYkaHR0cDovL2cuc3ltY2IuY29tL2NybHMvZ3RnbG9iYWwuY3JsMC4GCCsGAQUF \
BwEBBCIwIDAeBggrBgEFBQcwAYYSaHR0cDovL2cuc3ltY2QuY29tMBcGA1UdIAQQ \
MA4wDAYKKwYBBAHWeQIFATANBgkqhkiG9w0BAQUFAAOCAQEAJ4zP6cc7vsBv6JaE \
+5xcXZDkd9uLMmCbZdiFJrW6nx7eZE4fxsggWwmfq6ngCTRFomUlNz1/Wm8gzPn6 \
8R2PEAwCOsTJAXaWvpv5Fdg50cUDR3a4iowx1mDV5I/b+jzG1Zgo+ByPF5E0y8tS \
etH7OiDk4Yax2BgPvtaHZI3FCiVCUe+yOLjgHdDh/Ob0r0a678C/xbQF9ZR1DP6i \
vgK66oZb+TWzZvXFjYWhGiN3GhkXVBNgnwvhtJwoKvmuAjRtJZOcgqgXe/GFsNMP \
WOH7sf6coaPo/ck/9Ndx3L2MpBngISMjVROPpBYCCX65r+7bU2S9cS+5Oc4wt7S8 \
VOBHBw== \
-----END CERTIFICATE----- \
"

var net = require('net');
var forge = require('node-forge');

var socket = new net.Socket();
var client = forge.tls.createConnection({
  server: false,
  caStore: [root], // <=====
  connected: function(connection) {
    console.log('[tls] connected');
    connection.close();
  },
  tlsDataReady: function(connection) {
    socket.write(connection.tlsData.getBytes(), 'binary');
  },
  dataReady: function(connection) {
    console.log('[tls] data received from the server: ' + connection.data.getBytes());
  },
  closed: function() {
    console.log('[tls] disconnected');
  },
  error: function(connection, error) {
    console.log('[tls] error', error);
  }
});

socket.on('connect', function() {
  console.log('[socket] connected');
  client.handshake();
});
socket.on('data', function(data) {
  client.process(data.toString('binary'));
});
socket.on('end', function() {
  console.log('[socket] disconnected');
});

socket.on('end', function() {
  var socket2 = new net.Socket();
  var client2 = forge.tls.createConnection({
    server: false,
    caStore: [intermediate], // <=====
    connected: function(connection) {
      console.log('[tls] connected');
      connection.close();
    },
    tlsDataReady: function(connection) {
      socket2.write(connection.tlsData.getBytes(), 'binary');
    },
    dataReady: function(connection) {
      console.log('[tls] data received from the server: ' + connection.data.getBytes());
    },
    closed: function() {
      console.log('[tls] disconnected');
    },
    error: function(connection, error) {
      console.log('[tls] error', error);
    }
  });

  socket2.on('connect', function() {
    console.log('[socket] connected');
    client2.handshake();
  });
  socket2.on('data', function(data) {
    client2.process(data.toString('binary'));
  });
  socket2.on('end', function() {
    console.log('[socket] disconnected');
  });

  socket2.connect(993, 'imap.gmail.com');
});

socket.connect(993, 'imap.gmail.com');

produces

[socket] connected
[tls] connected
[tls] disconnected
[socket] disconnected
[socket] connected
[tls] error { message: 'Certificate is not trusted.',
  error: 'forge.pki.UnknownCertificateAuthority',
  send: true,
  alert: { level: 2, description: 48 },
  origin: 'client' }
[tls] disconnected
[socket] disconnected

You might want to look at the certificates using
openssl s_client -showcerts -connect imap.gmail.com:993 < /dev/null

@bantu
Copy link
Author

bantu commented Nov 17, 2014

I suppose this is not exactly what you wanted, but was easy to produce for me.

@dlongley
Copy link
Member

Thanks @bantu. I see why this is failing and can change the behavior so long as it doesn't cause some other issue (I don't think it would). I'll look into it when I get a chance.

dlongley referenced this issue Dec 1, 2014
- If an intermediate CA is the last in a chain and is
  present in the CA store, the chain will now be verified
  instead of rejected with an unknown CA error.
- Did some refactoring of verifyCertificateChain to clarify
  when certificates are self-signed and to ensure self-signed
  cert signatures are checked and that they enter the keyUsage
  and other extension checking code path.
@dlongley
Copy link
Member

dlongley commented Dec 1, 2014

@bantu, I think this is fixed in the latest version of forge (0.6.19). Can you confirm?

@bantu
Copy link
Author

bantu commented Dec 1, 2014

@dlongley Tried running the code above. Does not seem to work. 0e034ee mentions "If an intermediate CA is the last in a chain". This is not what I was referring to. I was thinking of this even when intermediate is not the last in the presented chain.

@dlongley
Copy link
Member

dlongley commented Dec 1, 2014

Ok, so there's another case to cover:

not-trusted trusted not-trusted
end-entity intermediate root
@dlongley
Copy link
Member

dlongley commented Dec 1, 2014

In this case, what would the verify callback report for the root CA? It's not in the CA store, so you'd expect it to report that the CA is unknown, but this would be confusing since the chain would still be verified. Is this a case that is best handled by writing a custom verify callback?

@bantu
Copy link
Author

bantu commented Dec 2, 2014

@dlongley I am not sure. I think it is fine to verify that the presented certificate chain is internally valid. However, I would not expect verify() to be called again after the intermediate was accepted as a trusted root.

@dlongley
Copy link
Member

dlongley commented Dec 2, 2014

That's one way of doing it that I'm considering; just trying to figure out if it's the right approach. I need to take a look around at other libs like OpenSSL to see what they do in this case. It may be that OpenSSL just requires the root CA to be present in the CA store as well.

@bantu
Copy link
Author

bantu commented Dec 2, 2014

It may be that OpenSSL just requires the root CA to be present in the CA store as well.

That would however mean that everything else that root signs is considered trusted too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
2 participants