EnricoMi commented on issue #43057: URL: https://github.com/apache/arrow/issues/43057#issuecomment-2487989262
I managed to make `DatasetEncryptionTest.WriteReadDatasetWithEncryption` test case consistently fail by scanning the dataset concurrently with many threads: https://github.com/EnricoMi/arrow/pull/7/files#diff-807e200a6a36f05141d665f692de94e440d62f850a67ca186a2bf06ef0c09177R231-R233 This reveals two issues: 1. The cipher context is wiped out (probably through `AesDecryptor::WipeOut` and not the deconstructor) while later being used. Reconstructing the context for every use seems to fix the concurrent wipe outs: https://github.com/EnricoMi/arrow/pull/7/files#diff-10f9496a81a85a38fca5553486a667a49836574479f6b956839a9ae80d559167R637 It still looks like this is not a matter of wipe out and later use of the context, but of concurrent wipe out and usage of the context, so that the recreation has to be guarded: https://github.com/EnricoMi/arrow/pull/7/files#diff-10f9496a81a85a38fca5553486a667a49836574479f6b956839a9ae80d559167R431 2. Once a cipher context is always available, the EVP methods exhibit their missing thread-safeness. Using one context for the same key concurrently is not supported: https://docs.openssl.org/master/man7/openssl-threads/ Adding the reconstruction fixes 1., making the cipher context a thread local variable fixes 2. However, I think this should be fixed be rethinking the design. Why are the cipher contexts wiped out in the first place? Are this security risk concerns or is this to reduce memory footprint? I haven't yet found the cause of multiple threads getting hold of the same AesDecryptorImpl instance. It'd be great to get some pointers to the mechanics of that. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
