Implement PKCS#7 signature verification.

[roysjosh]

[roysjosh]

There are a few improvements that could be made but the code works. For example, if there is a timestamp in the signature it isn't verified to be within the signing certificate's valid range. It also might make sense to add some sort of callback for each signature that is verified so the caller can display "(in)valid signature by $DN" etc.

Please let me know what you think. This PR is in support of accordproject/cicero#262.

[roysjosh]

Also, verification of whether or not the certificate is trusted is currently left to the caller. It might make sense to add a caStore parameter to the verify method and handle that in forge.

[roysjosh]

I have added CA chain verification and a per-signer status callback. I can squash the commits if needed.

[joosep-wm]

@roysjosh We have been working on PKCS7 verification on our own fork. So it's a nice surprise to see another implementation of it. We tried your solution with other PKCS7 signatures, namely one produced with openssl. The verification does not pass for some reason. Do you have an idea why not?

Here's an example test case

var ASSERT = require('assert');
var PKCS7 = require('../../lib/pkcs7');
var PKI = require('../../lib/pki');
​
(function() {
  var _pem = {
    signature: "-----BEGIN PKCS7-----\n" +
      "MIIIKAYJKoZIhvcNAQcCoIIIGTCCCBUCAQExDTALBglghkgBZQMEAgEwGAYJKoZI\n" +
      "hvcNAQcBoAsECW15IGRhdGENCqCCBXEwggVtMIIEVaADAgECAhIDzPiv475hpMq1\n" +
      "1ghTQFdshHwwDQYJKoZIhvcNAQELBQAwSjELMAkGA1UEBhMCVVMxFjAUBgNVBAoT\n" +
      "DUxldCdzIEVuY3J5cHQxIzAhBgNVBAMTGkxldCdzIEVuY3J5cHQgQXV0aG9yaXR5\n" +
      "IFgzMB4XDTE5MDgyODA4MjgyMloXDTE5MTEyNjA4MjgyMlowHDEaMBgGA1UEAwwR\n" +
      "Ki56Lmd1YXJkdGltZS5jb20wggEiMA0GCSqGSIb3DQEBAQUAA4IBDwAwggEKAoIB\n" +
      "AQDPeNmcy7HYxfpmlLY488FC8IRQL0BHTu/IZRWIXBw+JN6hdahAUcoKd9mNDmii\n" +
      "R33IuB3mazQ2V5KyPHcHR6LqkGDgW9ZKmj5jf74Cv6DVvw4FnLlAy+4cErK4udmZ\n" +
      "pAT8L8YaLA+m5a1bAHNf4hG4sheE4jf+IkJ6VtZXhEpsew+FFjCNEmt8lYtRQita\n" +
      "5wWmEIii3qiOv/vQPmtW5mlVCnM1M8nY5LUDVCBtBmpP24uBif/kSUiIEQ6W0Puy\n" +
      "AbZbCc1N3el/yKY1RKgZcnu99zwDAM5gyBfDSn5E2jBbbkRDtT0LKtmRLGBtLgoo\n" +
      "Ho7tMZ+hKdPeEBPXj+nn/PzPAgMBAAGjggJ5MIICdTAOBgNVHQ8BAf8EBAMCBaAw\n" +
      "HQYDVR0lBBYwFAYIKwYBBQUHAwEGCCsGAQUFBwMCMAwGA1UdEwEB/wQCMAAwHQYD\n" +
      "VR0OBBYEFI7gRMD4mehG1v3a0N3SRxHHf3yUMB8GA1UdIwQYMBaAFKhKamMEfd26\n" +
      "5tE5t6ZFZe/zqOyhMG8GCCsGAQUFBwEBBGMwYTAuBggrBgEFBQcwAYYiaHR0cDov\n" +
      "L29jc3AuaW50LXgzLmxldHNlbmNyeXB0Lm9yZzAvBggrBgEFBQcwAoYjaHR0cDov\n" +
      "L2NlcnQuaW50LXgzLmxldHNlbmNyeXB0Lm9yZy8wLQYDVR0RBCYwJIIRKi56Lmd1\n" +
      "YXJkdGltZS5jb22CD3ouZ3VhcmR0aW1lLmNvbTBMBgNVHSAERTBDMAgGBmeBDAEC\n" +
      "ATA3BgsrBgEEAYLfEwEBATAoMCYGCCsGAQUFBwIBFhpodHRwOi8vY3BzLmxldHNl\n" +
      "bmNyeXB0Lm9yZzCCAQYGCisGAQQB1nkCBAIEgfcEgfQA8gB3AOJpS64m6OlACeiG\n" +
      "G7Y7g9Q+5/50iPukjyiTAZ3d8dv+AAABbNeN/FIAAAQDAEgwRgIhALXcp2zae1wa\n" +
      "lmxb9iLBf+AKhM83ROfrNVMg7X7yuc92AiEA6QnZFrPcs0r1Yg99waISQGoirwl5\n" +
      "iVZJsZnIx6N3LUQAdwApPFGWVMg5ZbqqUPxYB9S3b79Yeily3KTDDPTlRUf0eAAA\n" +
      "AWzXjfxGAAAEAwBIMEYCIQCrUYNqkXq5/JzUGsHDT7nHrpbr54Lw2eb57jwbxf2i\n" +
      "9gIhAL0zZ0kHuzV0GeobVU8oZd2KTvWbBoKF6GdPOKg0lCfmMA0GCSqGSIb3DQEB\n" +
      "CwUAA4IBAQBXcYIkIwildnIfkabesGSSnTMiJpiknqlRrAfRdQCCzmTgeJOHuWcn\n" +
      "zFbIkK1p/Y7v1mo0q7avI4OTrevvwNhBqbzgJU5XHTITDqPydf6Yi3cExgJJEhVP\n" +
      "1IGcoHF/B1qLTSj5sUNBQpLHh5XOpqftCtQyEnjZrDPLG/X8Fb+8LaqWwsZL3GNf\n" +
      "RswWXqm7RCpm/UVhaA/0eYkzqt1YJfdP/r+ZV0kJr9RWTheAoG7m2wF8sKRG0Mmv\n" +
      "ezy7GcNiXvzHZW3zY55ZrLHv6C4T/LWeDyfdv6Mj1SBT6TquVe1YrqnrGUy9UvRL\n" +
      "H2g8wPkBe0R3Z2L4LIHtJISJvSq5etHjMYICcDCCAmwCAQEwYDBKMQswCQYDVQQG\n" +
      "EwJVUzEWMBQGA1UEChMNTGV0J3MgRW5jcnlwdDEjMCEGA1UEAxMaTGV0J3MgRW5j\n" +
      "cnlwdCBBdXRob3JpdHkgWDMCEgPM+K/jvmGkyrXWCFNAV2yEfDALBglghkgBZQME\n" +
      "AgGggeQwGAYJKoZIhvcNAQkDMQsGCSqGSIb3DQEHATAcBgkqhkiG9w0BCQUxDxcN\n" +
      "MTkwOTAzMTAzODIzWjAvBgkqhkiG9w0BCQQxIgQgJJLU6KuD8gawc+WYypnHBE9M\n" +
      "lcb5lG/lENcFJZv1GYcweQYJKoZIhvcNAQkPMWwwajALBglghkgBZQMEASowCwYJ\n" +
      "YIZIAWUDBAEWMAsGCWCGSAFlAwQBAjAKBggqhkiG9w0DBzAOBggqhkiG9w0DAgIC\n" +
      "AIAwDQYIKoZIhvcNAwICAUAwBwYFKw4DAgcwDQYIKoZIhvcNAwICASgwDQYJKoZI\n" +
      "hvcNAQEBBQAEggEAH+drkNfXK2uR6TmE02NriCHkf5PLN0kZGm/Mg2mhDWSt5b1f\n" +
      "RULwJZC5SYSNlpXnCnE0uyrZVrgcOz0W10ZisztASvzr0MRdQvx1nrlOMi79x2kw\n" +
      "RYlI1VD5NkhIIAxU1Tmn+xBM5GjqUOuEEphByknam+NEYC513vBxcSm1RR0zSOcB\n" +
      "q5yBv7JuAw7RHcqS9L5npJQ/KkrIwxUTFTr0kbl6739U2VbUlb/mPOGXNqZdWTsn\n" +
      "GZJDMT1xHDCP4vtE/0HVXjdSJirmI2loVGgarKc3QaJ36rOUcvPyCswk5CnIwcX9\n" +
      "rm3VxOUwDh4LkEorBaya68TOeycBgXcPxqPeOg==\n" +
      "-----END PKCS7-----",
    cert:   "-----BEGIN CERTIFICATE-----\n" +
      "MIIEkjCCA3qgAwIBAgIQCgFBQgAAAVOFc2oLheynCDANBgkqhkiG9w0BAQsFADA/\n" +
      "MSQwIgYDVQQKExtEaWdpdGFsIFNpZ25hdHVyZSBUcnVzdCBDby4xFzAVBgNVBAMT\n" +
      "DkRTVCBSb290IENBIFgzMB4XDTE2MDMxNzE2NDA0NloXDTIxMDMxNzE2NDA0Nlow\n" +
      "SjELMAkGA1UEBhMCVVMxFjAUBgNVBAoTDUxldCdzIEVuY3J5cHQxIzAhBgNVBAMT\n" +
      "GkxldCdzIEVuY3J5cHQgQXV0aG9yaXR5IFgzMIIBIjANBgkqhkiG9w0BAQEFAAOC\n" +
      "AQ8AMIIBCgKCAQEAnNMM8FrlLke3cl03g7NoYzDq1zUmGSXhvb418XCSL7e4S0EF\n" +
      "q6meNQhY7LEqxGiHC6PjdeTm86dicbp5gWAf15Gan/PQeGdxyGkOlZHP/uaZ6WA8\n" +
      "SMx+yk13EiSdRxta67nsHjcAHJyse6cF6s5K671B5TaYucv9bTyWaN8jKkKQDIZ0\n" +
      "Z8h/pZq4UmEUEz9l6YKHy9v6Dlb2honzhT+Xhq+w3Brvaw2VFn3EK6BlspkENnWA\n" +
      "a6xK8xuQSXgvopZPKiAlKQTGdMDQMc2PMTiVFrqoM7hD8bEfwzB/onkxEz0tNvjj\n" +
      "/PIzark5McWvxI0NHWQWM6r6hCm21AvA2H3DkwIDAQABo4IBfTCCAXkwEgYDVR0T\n" +
      "AQH/BAgwBgEB/wIBADAOBgNVHQ8BAf8EBAMCAYYwfwYIKwYBBQUHAQEEczBxMDIG\n" +
      "CCsGAQUFBzABhiZodHRwOi8vaXNyZy50cnVzdGlkLm9jc3AuaWRlbnRydXN0LmNv\n" +
      "bTA7BggrBgEFBQcwAoYvaHR0cDovL2FwcHMuaWRlbnRydXN0LmNvbS9yb290cy9k\n" +
      "c3Ryb290Y2F4My5wN2MwHwYDVR0jBBgwFoAUxKexpHsscfrb4UuQdf/EFWCFiRAw\n" +
      "VAYDVR0gBE0wSzAIBgZngQwBAgEwPwYLKwYBBAGC3xMBAQEwMDAuBggrBgEFBQcC\n" +
      "ARYiaHR0cDovL2Nwcy5yb290LXgxLmxldHNlbmNyeXB0Lm9yZzA8BgNVHR8ENTAz\n" +
      "MDGgL6AthitodHRwOi8vY3JsLmlkZW50cnVzdC5jb20vRFNUUk9PVENBWDNDUkwu\n" +
      "Y3JsMB0GA1UdDgQWBBSoSmpjBH3duubRObemRWXv86jsoTANBgkqhkiG9w0BAQsF\n" +
      "AAOCAQEA3TPXEfNjWDjdGBX7CVW+dla5cEilaUcne8IkCJLxWh9KEik3JHRRHGJo\n" +
      "uM2VcGfl96S8TihRzZvoroed6ti6WqEBmtzw3Wodatg+VyOeph4EYpr/1wXKtx8/\n" +
      "wApIvJSwtmVi4MFU5aMqrSDE6ea73Mj2tcMyo5jMd6jmeWUHK8so/joWUoHOUgwu\n" +
      "X4Po1QYz+3dszkDqMp4fklxBwXRsW10KXzPMTZ+sOPAveyxindmjkW8lGy+QsRlG\n" +
      "PfZ+G6Z6h7mjem0Y+iWlkYcV4PIWL1iwBi8saCbGS5jN2p8M+X+Q7UNKEkROb3N6\n" +
      "KOqkqm57TH2H3eDJAkSnh6/DNFu0Qg==\n" +
      "-----END CERTIFICATE-----"
  };
​
  describe('pkcs7', function() {
    it('should verify PKCS#7 signature', function() {
      var p7 = PKCS7.messageFromPem(_pem.signature);
      var verified = p7.verify(PKI.createCaStore([_pem.cert]));
      ASSERT.equal(verified, true);
    });
  });
})();

[roysjosh]

@roysjosh We have been working on PKCS7 verification on our own fork. So it's a nice surprise to see another implementation of it. We tried your solution with other PKCS7 signatures, namely one produced with openssl. The verification does not pass for some reason. Do you have an idea why not?

Ah, this is because the code I added to parse the attributes back into forge objects in _attributeFromAsn1 doesn't handle S/MIME capabilities or error out to let you know of that current limitation. I'll do one of those two now. Thanks for the example!

[roysjosh]

Ping?

[roysjosh]

Is there any interest in merging this?

[davidlehn]

@roysjosh Yes. We've been busy for a long time and haven't been keeping up with forge PRs. Apologies for that, we really do appreciate the contribution! Could you add a line or two example to the README? Our README needs improvement, but good to at least have something to note a feature exists.

[dselman]

There are a few improvements that could be made but the code works. For example, if there is a timestamp in the signature it isn't verified to be within the signing certificate's valid range. It also might make sense to add some sort of callback for each signature that is verified so the caller can display "(in)valid signature by $DN" etc.

Please let me know what you think. This PR is in support of accordproject/cicero#262.

Any update on this please? We are waiting to merge a downstream change that requires this.

[hamano]

Hi, I just added verification with RSASSA-PSS. https://github.com/osstech-jp/forge/commit/370d3d3876a3a270ae906e229632752757c0a3ed

also ECDSA verification is works with #925 many thanks.

[dhensby]

@davidlehn - it'd be really great to have this merged in as pkcs7 is an area I'm working in and having signature verification implemented would be really helpful 🙏

[dhensby]

Something that would be useful would be a way to provide the signing certificate instead of the caStore and having the chain validated.

Much like the -noverify flag for openssl

Currently the user must have the full chain available, but they may only have the signing certificate, and if this is the case (and it's trusted) there's no need to verify the whole chain.