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

forge.pkcs7.messageFromPem stop working since 1.3.0 version #975

Open
albertoinch opened this issue Apr 19, 2022 · 7 comments
Open

forge.pkcs7.messageFromPem stop working since 1.3.0 version #975

albertoinch opened this issue Apr 19, 2022 · 7 comments

Comments

@albertoinch
Copy link

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,

@davidlehn
Copy link
Member

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.

@albertoinch
Copy link
Author

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-----

@davidlehn
Copy link
Member

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
Copy link
Member

Patch available in #977. Still pondering if that's the best approach. Will release something soon.

@albertoinch
Copy link
Author

Thanks, I really appreciate your help.

@DePasqualeOrg
Copy link

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

@pke
Copy link

pke commented Feb 29, 2024

@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.

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

No branches or pull requests

4 participants