kdenhartog
commented
6 months ago This issue is about reducing the number of pkcs7 algorithms in the copied implementation down to the ones that we actually are using or intend to use.
Right now, the copied version contains many unnecessary algorithms of which some are insecure as well. These are the following algorithms we should support:
OIDDigestAlgorithmSHA256 OIDDigestAlgorithmECDSAP256 OIDEncryptionAlgorithmAES256GCM
The other's can be dropped unless we have a good reason to support them. Also, if I've got the 3 selected wrong we can replace them with one of the alternatives currently supported, so please let me know if we need to do that instead.
https://github.com/brave-intl/bat-go/pull/1691#discussion_r1494038229 https://github.com/brave-intl/bat-go/pull/1691#discussion_r1494038272 https://github.com/brave-intl/bat-go/pull/1691#discussion_r1494038647 https://github.com/brave-intl/bat-go/pull/1691#discussion_r1494040006 https://github.com/brave-intl/bat-go/pull/1691#discussion_r1494040716
Do we need all of these algorithms to be supported? It seems like many of them would be unnecessary and in some cases some of these are insecure. I think we should only support the algorithms we intend to use with the nitro-shim. https://github.com/brave-intl/bat-go/blob/nitro-payments-dev/libs/nitro/pkcs7/decrypt.go#L44
Can we stick with just this one here. The DES should be dropped anyways and the CBC method can be attacked if padding isn't done safely. https://github.com/brave-intl/bat-go/blob/475eab4d0ca4a7706c0225d2f43c359968401933/libs/nitro/pkcs7/decrypt.go#L84