pki.verifyCertificateChain fails to accept intermediate certificates in caStore

bantu commented 9 years ago

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 https://github.com/digitalbazaar/forge/issues/96#issuecomment-32988824 though.

dlongley commented 9 years ago

Hi @bantu,

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

bantu commented 9 years ago

@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 commented 9 years ago

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

dlongley commented 9 years ago

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 commented 9 years ago

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

bantu commented 9 years ago

@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 commented 9 years ago

Ok, so there's another case to cover:

not-trusted	trusted	not-trusted
end-entity	intermediate	root

dlongley commented 9 years ago

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 commented 9 years ago

@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 commented 9 years ago

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 commented 9 years ago

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.

digitalbazaar / forge