forge icon indicating copy to clipboard operation
forge copied to clipboard

forge.pkcs7.messageFromPem stop working since 1.3.0 version

Open albertoinch opened this issue 2 years ago • 7 comments

I was using this function to decode a pem of pkcs#7, but from 1.3.0 version it's returning:

Unparsed DER bytes remain after ASN.1 parsing.

Best regards,

albertoinch avatar Apr 19 '22 17:04 albertoinch

Can you provide an example PEM file?

As you read in the changelog, there were changes to make things more strict to address some other serious vulnerabilities. Perhaps there were some unintended side effects for reasonable use cases. If you have an example with valid data that is failing, we'd like to see that so it can be fixed. There are flags to fromDer to disable that behavior you see. You could try adding those to the call in messageFromPem and see what happens. If there are good use cases to start bubbling up low level flags, we could do that. But I'm afraid that might start covering up bugs due to bad data. I'm not sure what the best way to handle that is.

davidlehn avatar Apr 19 '22 19:04 davidlehn

Well, I extracted it from pdf signed with acrobat reader.

I put an example:

-----BEGIN PKCS7-----
MIIGyQYJKoZIhvcNAQcCoIIGujCCBrYCAQExDzANBglghkgBZQMEAgEFADALBgkq
hkiG9w0BBwGgggRYMIIEVDCCAzygAwIBAgIIJ4xiDDybcAAwDQYJKoZIhvcNAQEF
BQAwgZ4xCzAJBgNVBAYTAkJPMQ8wDQYDVQQIDAZMYSBQYXoxDzANBgNVBAcMBkxh
IFBhejEOMAwGA1UECgwFQURTSUIxDDAKBgNVBAsMA1VJRDEsMCoGA1UEAwwjRW50
aWRhZCBDZXJ0aWZpY2Fkb3JhIFB1YmxpY2EgQURTSUIxITAfBgkqhkiG9w0BCQEW
EmFpbmNoQGFkc2liLmdvYi5ibzAeFw0yMjA0MjExOTE3MDFaFw0yMzA0MjExOTE3
MDFaMIGhMRwwGgYDVQQDDBNXRU5EWSBWSUxMQ0EgQkxBTkNPMQswCQYDVQQuEwJD
STEUMBIGBysGAQEBAQAMBzgzMzcxOTYxITAfBgkqhkiG9w0BCQEWEmFpbmNoQGFk
c2liLmdvYi5ibzELMAkGA1UEBhMCQk8xLjAsBgNVBA0MJVBlcnNvbmEgTmF0dXJh
bCBvIEZpc2ljYSBGaXJtYSBTaW1wbGUwggEiMA0GCSqGSIb3DQEBAQUAA4IBDwAw
ggEKAoIBAQC/Zf4365XR3BFJJzw6yTaDwh7m83+wZT1o2wB1xctzrT8foF4x9Vwi
eHo/OG2sNKmOG7b8cnHCHXyNLttaTZ036HFwn3an+qazZ9RVjn9vgIxDR5TGZ5Xf
hb9/YE20IUY/2PcVNSNexa6kQx2ADM0fE0TUHQ05B7oba7HTuFj57vh1YsKOtyQM
QdD6xXOrn9wCWf8lgsVCXfaX+53A2fb9g/zToaEomr0fAAEcEied10AfJxKgxZgX
gaGoiBCoh956FZ8GMdWFo8X/g9LOoGHmq/b/axODxO19+ExK3OqCbbTfixsE9SyD
1hc8Wyd9kWbDg4Vtx7ej9g2K5E+B0ZORAgMBAAGjgZAwgY0wDAYDVR0TBAUwAwEB
/zALBgNVHQ8EBAMCBPAwHQYDVR0RBBYwFIYSYWluY2hAYWRzaWIuZ29iLmJvMFEG
A1UdHwRKMEgwRqBEoEKGQGh0dHBzOi8vZGVzYXJyb2xsby5hZHNpYi5nb2IuYm8v
ZGVzYV9hZ2VuY2lhL2xpc3RfcmV2b2NhY2lvbi5jcmwwDQYJKoZIhvcNAQEFBQAD
ggEBAIscFCdqXyAvlo1aom0beHNQlqH9AabRZxO8qcd4jHDJvB/40YBGZ+cBa3mK
MkmlJrVEMzwDpb3yAaYw1/Kvx1ndGY4wgAcTJOCYTVhjO24e+YYd1sutGX0TwKqW
XKBcYdC5Vyo14FbtDdZRqWh1Kxvze6Du7tCBgLXABN1/w5wGQBXW7wceyo3lEmxM
B03IZOD4dQkwfxvTMV/F+end+73ud9DTCcC7mLDS/hMwOvI2WcuV628xwks1rp9Z
jCzCblQuiw7Yfcos/PG5cZZ5mny+gl1omIb3S6bwvkBYbiNwqqWJpFfSmH2msqrd
rtTXHXugKE9Ix/KwzhmybRy/MEAxggI1MIICMQIBATCBqzCBnjELMAkGA1UEBhMC
Qk8xDzANBgNVBAgMBkxhIFBhejEPMA0GA1UEBwwGTGEgUGF6MQ4wDAYDVQQKDAVB
RFNJQjEMMAoGA1UECwwDVUlEMSwwKgYDVQQDDCNFbnRpZGFkIENlcnRpZmljYWRv
cmEgUHVibGljYSBBRFNJQjEhMB8GCSqGSIb3DQEJARYSYWluY2hAYWRzaWIuZ29i
LmJvAggnjGIMPJtwADANBglghkgBZQMEAgEFAKBcMA8GCSqGSIb3LwEBCDECMAAw
GAYJKoZIhvcNAQkDMQsGCSqGSIb3DQEHATAvBgkqhkiG9w0BCQQxIgQgPImuGWCk
GClMV6q70HoANspLKTjXLXaXD4hfGB09/SowDQYJKoZIhvcNAQELBQAEggEAK4/b
cPKcEGhZVZkx9dzznzuZusSxrGUp4ERpxvGxyLdh5BWdEafIaOyWqMlE6llFNgkG
loA7MNb6jJRbg/3tu4muzWL+VDiKENDr1GLwKuLjykZiH1W/Wlypptuy9E5brHMq
7cTeXIFW1lVidIVINPaA1dcnQ/KnjjADlCPJWLsbYkR7V+Q2MQp13x42xHGxVx34
dvXvQJc6ohxK+fAdZF/wl8XNhyx9bBdBBJH2lz3HhPVOHSEmA2fMR75Cs4epmyxW
upeKDuzVAl4ZHetOlbhxxz+ZsMEcizn8EYotZqkAIFkcKX6Xz7OkeRqLyk+q8gIr
ajoQMMeI85j8dag9jwAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
-----END PKCS7-----

albertoinch avatar Apr 21 '22 19:04 albertoinch

Thanks. 2647 bytes of data and 896 trailing zeros. Looking at the PEM RFC 7468, I see the contents are BER, not DER, so things like trailing useless data are, I think, acceptable. I'm guessing that's for some sort of padding, who knows. They do have an appendix that basically says to use DER, which I suppose most tools do. openssl trying to convert that input back to PEM just drops the zeros.

So I'm guessing everywhere the code calls fromDer on PEM data is a bit misnamed, and the new strict by default mode isn't correct. Changing it to asn1.fromDer(msg.body, {parseAllBytes: false}) will fix the issue here.

I'll make a patch soon and try to address this for all the related cases.

davidlehn avatar Apr 21 '22 21:04 davidlehn

Patch available in https://github.com/digitalbazaar/forge/pull/977. Still pondering if that's the best approach. Will release something soon.

davidlehn avatar Apr 22 '22 00:04 davidlehn

Thanks, I really appreciate your help.

albertoinch avatar Apr 22 '22 14:04 albertoinch

I'm encountering this error when parsing receipts from Apple's App Attest service.

DePasqualeOrg avatar Dec 20 '23 11:12 DePasqualeOrg

@davidlehn run into this parsing problem, when parsing padded DER data from a smart cards EF.ATR file which looked like this:

e01102020809020300800202020809020208095f520c806605444549444d739621f3d003040600d2104445494658534c433332474441040000d310444549444d4d4843475f473232020206d410444549444d4548435f39303030030005d610444549444d50565656352e3030010000cf15cfcfcfcfcfcfcfcfcfcfcfcfcfcfcfcfcfcfcfcfcf

Adding { parseAllBytes: false } fixed the error and I could parse the values.

One minor issue in typescript though: The definition file is not up to date with the current node-force function signatures.

pke avatar Feb 29 '24 14:02 pke