On Wed, 21 Dec 2022 00:22:37 GMT, David Schlosnagle <d...@openjdk.org> wrote:
>> Volker Simonis has updated the pull request incrementally with two >> additional commits since the last revision: >> >> - Some refactoring and simplification. Moved most of the implementation >> from SessionTicketExtension to SSLSessionContextImpl >> - Moving currentKeyID/keyHashMap to SSLSessionContextImpl as requested by >> @XueleiFan > > src/java.base/share/classes/sun/security/ssl/SSLSessionContextImpl.java line > 75: > >> 73: private int timeout; // timeout in seconds >> 74: >> 75: private int currentKeyID = new SecureRandom().nextInt(); > > Could this instantiation of new `SecureRandom` become a concurrency > bottleneck on entropy? Should the `SecureRandom` from `SSLContext` be > injected to constructor (though there's currently a slightly inverted > dependency as `SSLContextImpl` creates client & server > `SSLSessionContextImpl`s in its constructor, but the `SecureRandom` is set on > `SSLContextImpl::engineInit`)? > > Also note the comment in `SSLContextImpl::engineInit`: > > https://github.com/openjdk/jdk/blob/5e862c49ea74a408d32812d96ee15324a342a585/src/java.base/share/classes/sun/security/ssl/SSLContextImpl.java#L122-L133 Hm, I'm not sure if this results in a real problem but I agree that it is better to be on the safe side :) >From my understanding (and please correct me if I'm wrong) >`SSLContextImpl::engineInit()` has to be called before we can start to >negotiate a connection and create a session key (i.e. before >`SSLContextImpl::getKey()` and subsequently >`SSLSessionContextImpl::getCurrentKeyID()` will be called for the first time >on a context). This means that we can use the random "*priming date*" which is created in `SSLContextImpl::engineInit()` anyway (thanks for that link :) to initialize `SSLSessionContextImpl::currentKeyID` and don't need any additional call to `SecureRandom` at all. ------------- PR: https://git.openjdk.org/jdk/pull/11590
