-
Notifications
You must be signed in to change notification settings - Fork 769
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
Comments
Hi @bantu, Can you submit a simplified code example w/certificate PEMs demonstrating the issue? |
@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
You might want to look at the certificates using |
I suppose this is not exactly what you wanted, but was easy to produce for me. |
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. |
- 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.
@bantu, I think this is fixed in the latest version of forge (0.6.19). Can you confirm? |
Ok, so there's another case to cover:
|
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? |
@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. |
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. |
That would however mean that everything else that root signs is considered trusted too. |
Assume the following certificate chain:
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.
C=US/O=Google Inc/CN=Google Internet Authority G2
thanC=US/O=GeoTrust Inc./CN=GeoTrust Global CA
orC=US/O=Equifax/OU=Equifax Secure Certificate Authority
Furthermore it might be useful to also be able to pinn non-CA certificates directly. This has been previously discussed in #96 (comment) though.
The text was updated successfully, but these errors were encountered: