AdvancedTlsX509TrustManager throws confusing exception if certificate file is missing

[Hodkinson]

What version of gRPC-Java are you using?

1.51

What is your environment?

Linux, Mac, JDK 17

What did you expect to see?

A file not found exception.

What did you see instead?

java.security.GeneralSecurityException: Files were unmodified before their initial update. Probably a bug.
       at io.grpc.util.AdvancedTlsX509TrustManager.updateTrustCredentialsFromFile(AdvancedTlsX509TrustManager.java:226) ~[grpc-core-1.51.0.jar:1.51.0]

Steps to reproduce the bug

make this call with a file created where there is no path:

AdvancedTlsX509TrustManager.newBuilder()
		.build()
		.also {
			it.updateTrustCredentialsFromFile(File(noFileAtPath), 1, TimeUnit.HOURS, executor)
		}

The updatedTime comes back as 0 from the readAndUpdate call.

[erm-g]

Hi @Hodkinson, thank you for the reporting! So I see the same exception after executing AdvancedTlsX509TrustManager.newBuilder().build(). updateTrustCredentialsFromFile(new File("I_don't_exist"), 1, TimeUnit.SECONDS, Executors.newSingleThreadScheduledExecutor()); What do you mean by

The updatedTime comes back as 0 from the readAndUpdate call. ?

[Hodkinson]

At line 224 of AdvancedTlsX509TrustManager: https://github.com/grpc/grpc-java/blob/master/core/src/main/java/io/grpc/util/AdvancedTlsX509TrustManager.java#L224

there's a call to readAndUpdate, which assumes the file exists, but because it doesn't, a zero is returned, as lastModified returns 0, and the oldTime is also zero. This readAndUpdate seems a little fragile due to this assumption - for example a certificate could be removed on disk, rather than replaced. In this case, the last modified would be zero, which would not match the oldTime, and then it would try to construct the FileInputStream from a missing file.

One solution would be to assume in readAndUpdate that if the lastModified call returns zero, then the file does not exist, and throw a GeneralSecurityException reporting that the file is missing.

[Hodkinson]

Hi, any thoughts on this, do you need any extra information?