narupley / not-going-to-be-commons-ssl

A Java 9, 10, 11+ compliant fork of Not-Yet-Commons-SSL
Apache License 2.0
15 stars 7 forks source link

Temporary CRL cache files don't get cleaned up. #17

Open lzandman opened 1 year ago

lzandman commented 1 year ago

I came here, because we are having a problem with the OpenText Fortify product (previously owned by Micro Focus). This is a commercial product that apparently uses this library. Which of course has me frowned upon, as this is "alpha" code...

Anyway, the issue we're experiencing is that a lot (thousands a day!) of crl*.tmp files get created. The code responsible for creating these files seems to be the CRLHolder.CheckCRL() method in the Certificates class. The CRLHolder class seems to create these CRL temp files as a sort of cache. However, the cleanup of these files apparently fails in some situations. I did a quick inspection of the code and found some problems:

First, in the context of Fortify the tempFile.deleteOnExit(); has little effect. As Fortify is a long-running process, and basically never exits. So these files won't be deleted.

Secondly, and more importantly, the following code seems problematic:

https://github.com/narupley/not-going-to-be-commons-ssl/blob/346bb4e8481316687a643219800a398ecd8f62a8/src/main/java/org/apache/commons/ssl/Certificates.java#L424C21-L426C88

I think this code may throw an IOException, which is caught below. But in this situation, the temp file that was created above it never gets deleted and becomes orphaned. The code on line 432 does delete the temp file. But that code is never reached when the code linked to above is throwing an exception. The IOException handling code on line 439 should probably also contain a tempFile.delete() call.