ProtonMail / go-crypto

Fork of go/x/crypto, providing an up-to-date OpenPGP implementation
https://pkg.go.dev/github.com/ProtonMail/go-crypto
BSD 3-Clause "New" or "Revised" License
328 stars 99 forks source link

Armor decoding fails for malformed(?) headers #172

Closed ckcr4lyf closed 1 year ago

ckcr4lyf commented 1 year ago

Hi guys, I need to work with an external service, who's PGP detached signature includes a line Version: as a header, with no space or no value (just a newline after it).

As per section 6.2 of RFC 4880, quote:

The format of an Armor Header is that of a key-value pair. A colon (':' 0x38) and a single space (0x20) separate the key and value. OpenPGP should consider improperly formatted Armor Headers to be corruption of the ASCII Armor. Unknown keys should be reported to the user, but OpenPGP should continue to process the message.

Indeed, if I use gpg to try and verify such a signature, it works.

Plaintext signature, hexdump, and gpg verification passing ``` $ cat signature.asc -----BEGIN PGP SIGNATURE----- Version: iQEzBAABCAAdFiEEoSOgqNCnAp+EumHncWO0DMTgrJ8FAmSL6xMACgkQcWO0DMTg rJ8KrQf6A5AgMpGyekIiBeG2lPeqe5PIulKBI0kHDo01ofXt31ahnVrQmfHw7AMc sHLk3LkS85aR2pU8M1+NY4vc7TBQszwyCILKxOV43hCXmxMlhEuNsARJ/MOWmdfd dQaFSmx9doXeKtHU6xO0sWrMFgj69rnH9Q4ZvvqrrLkVEHpMJar0xPNGVZyegZ8h arlmBseDFHavcdmhrcxUt0ry0IITTu0NBzRnyZB40e01jnzCVHQiGeitgCuTFh4r NJGg9zRmWUpKTIVl7upSds9SiaVEJLfJPG/oDSTOJDyJBR7sKcJ/4z2OemRK89Ft JX24K2nnjlT5RNxSTF0lQ8auQXhmRQ== =nQKz -----END PGP SIGNATURE----- $ xxd signature.asc 00000000: 2d2d 2d2d 2d42 4547 494e 2050 4750 2053 -----BEGIN PGP S 00000010: 4947 4e41 5455 5245 2d2d 2d2d 2d0a 5665 IGNATURE-----.Ve 00000020: 7273 696f 6e3a 0a0a 6951 457a 4241 4142 rsion:..iQEzBAAB 00000030: 4341 4164 4669 4545 6f53 4f67 714e 436e CAAdFiEEoSOgqNCn 00000040: 4170 2b45 756d 486e 6357 4f30 444d 5467 Ap+EumHncWO0DMTg 00000050: 724a 3846 416d 534c 3678 4d41 4367 6b51 rJ8FAmSL6xMACgkQ 00000060: 6357 4f30 444d 5467 0a72 4a38 4b72 5166 cWO0DMTg.rJ8KrQf 00000070: 3641 3541 674d 7047 7965 6b49 6942 6547 6A5AgMpGyekIiBeG 00000080: 326c 5065 7165 3550 4975 6c4b 4249 306b 2lPeqe5PIulKBI0k 00000090: 4844 6f30 316f 6658 7433 3161 686e 5672 HDo01ofXt31ahnVr 000000a0: 516d 6648 7737 414d 630a 7348 4c6b 334c QmfHw7AMc.sHLk3L 000000b0: 6b53 3835 6152 3270 5538 4d31 2b4e 5934 kS85aR2pU8M1+NY4 000000c0: 7663 3754 4251 737a 7779 4349 4c4b 784f vc7TBQszwyCILKxO 000000d0: 5634 3368 4358 6d78 4d6c 6845 754e 7341 V43hCXmxMlhEuNsA 000000e0: 524a 2f4d 4f57 6d64 6664 0a64 5161 4653 RJ/MOWmdfd.dQaFS 000000f0: 6d78 3964 6f58 654b 7448 5536 784f 3073 mx9doXeKtHU6xO0s 00000100: 5772 4d46 676a 3639 726e 4839 5134 5a76 WrMFgj69rnH9Q4Zv 00000110: 7671 7272 4c6b 5645 4870 4d4a 6172 3078 vqrrLkVEHpMJar0x 00000120: 504e 4756 5a79 6567 5a38 680a 6172 6c6d PNGVZyegZ8h.arlm 00000130: 4273 6544 4648 6176 6364 6d68 7263 7855 BseDFHavcdmhrcxU 00000140: 7430 7279 3049 4954 5475 304e 427a 526e t0ry0IITTu0NBzRn 00000150: 795a 4234 3065 3031 6a6e 7a43 5648 5169 yZB40e01jnzCVHQi 00000160: 4765 6974 6743 7554 4668 3472 0a4e 4a47 GeitgCuTFh4r.NJG 00000170: 6739 7a52 6d57 5570 4b54 4956 6c37 7570 g9zRmWUpKTIVl7up 00000180: 5364 7339 5369 6156 454a 4c66 4a50 472f Sds9SiaVEJLfJPG/ 00000190: 6f44 5354 4f4a 4479 4a42 5237 734b 634a oDSTOJDyJBR7sKcJ 000001a0: 2f34 7a32 4f65 6d52 4b38 3946 740a 4a58 /4z2OemRK89Ft.JX 000001b0: 3234 4b32 6e6e 6a6c 5435 524e 7853 5446 24K2nnjlT5RNxSTF 000001c0: 306c 5138 6175 5158 686d 5251 3d3d 0a3d 0lQ8auQXhmRQ==.= 000001d0: 6e51 4b7a 0a2d 2d2d 2d2d 454e 4420 5047 nQKz.-----END PG 000001e0: 5020 5349 474e 4154 5552 452d 2d2d 2d2d P SIGNATURE----- 000001f0: 0a . $ gpg --verify signature.asc plaintext.txt gpg: WARNING: unsafe permissions on homedir '/home/raghu/Documents/gitops/applications/common-go/pgp' gpg: Signature made Fri 16 Jun 2023 12:54:43 PM HKT gpg: using RSA key A123A0A8D0A7029F84BA61E77163B40CC4E0AC9F gpg: Good signature from "Alice (Alice for Unit Test) " [unknown] gpg: WARNING: This key is not certified with a trusted signature! gpg: There is no indication that the signature belongs to the owner. Primary key fingerprint: A123 A0A8 D0A7 029F 84BA 61E7 7163 B40C C4E0 AC9F ```

However due to how this library is currently implementing armored decoding, it will fail, because when it finds a non empty line immediately after the initial header (without a double newline, i.e. \n\n) , it will try and parse it as a header as well. If this fails, it will skip everything it did so far, and jump to the start of the function, to try and find a new block from the next line onwards .

Would you guys be open to accepting an MR which fixes this behavior? i.e. to ignore such header lines and proceed with decoding the rest of it.

The buggy part:

https://github.com/ProtonMail/go-crypto/blob/6f98819771a1bdb86d5b5379d209796b5b7368ff/openpgp/armor/armor.go#L211-L214

lubux commented 1 year ago

Hi. Thanks for pointing out this issue and the pull request. We addressed this issue in the version 2 branch, and so it will be fixed in the next release.

I created #174 to merge this change to the main branch before v2.

ckcr4lyf commented 1 year ago

Awesome, thanks! I'll leave the issue open for now so if anyone stumbles across the same problem they have some reference.

Cheers

twiss commented 1 year ago

Addressed by #174 :)