On Thu, 13 Apr 2023 21:43:24 GMT, Xue-Lei Andrew Fan <xue...@openjdk.org> wrote:
>>> Currently, `provider()` is a method of `KEM.Encapsulator`. If `KEMSpi.
>>> newEncapsulator` also returns this interface, then what value should its
>>> `provider()` method return? This is what I meant registering itself to a
>>> provider.
>>>
>>> When I said different instances, I was asking
>>>
>>> ```
>>> var k = KEM.getInstance("DHKEM", p);
>>> var e = k.newEncapsulator(pk);
>>> // now, is p == e.provider()?
>>> ```
>>>
>>> Or, are you suggesting we should define `provider()` somewhere else? It's
>>> possible, but I have difficulty making every class immutable.
>>
>> If the provider() method in KEM.Encapsulator is the only reason, the cost to
>> support it may be too high with so many duplicated/similar
>> specifications/names and code.
>>
>> Option 1: Remove the KEM.Encapsulator.provider() method, and provide no
>> access to the underlying provider object.
>>
>>> do you expect it to return new SunJCE()? This means the p in
>>> getInstance("DHKEM", p) will be a different instance from the value
>>> returned by getProvider().
>>
>> The Provider class is mutable, we may not want to change the provider object
>> asked for "DHKEM". I think you have used a solution to pass the provider
>> object in the KEM.java implementation currently. Maybe, it could be twitted
>> a little bit so that the provider can be passed to a delegated
>> KM.Encapsulator interface implementation.
>>
>> Option 2:
>>
>> public final class KEM {
>> interface Encapsulator {
>> ...
>> KEM.Encapsulated encapsulate(...);
>> ...
>>
>> default Provider provider() {
>> return null;
>> }
>> }
>>
>> private static class DelegatedEncapsulator implements Encapsulator {
>> private final Provider p;
>> private DelegatedEncapsulator(Encapsulator e, Provider p) {
>> this.p = p;
>> ...
>> }
>> public Provider provider() {
>> return this.p;
>> }
>> }
>>
>> ...
>> KEMSpi spi = (KEMSpi) service.newInstance(null);
>> return new DelegatedEncapsulator(
>> spi.engineNewEncapsulator(pk, spec, secureRandom),
>> // This is the interface implementation, use the same provider as KEM.
>> service.getProvider()); // This is the provider passed to
>> the delegated KEM.Encapsulator object.
>> ...
>> }
>
> For more details about option 2, please refer to
> https://github.com/openjdk/jdk/pull/13470/files. The KEM.java and
> KEMSpi.java is pretty much the same except the clean up of En/Decapsulator(s)
> in this PR.
I see. So the security providers are told:
1. Don't implement `provider()` (If you do, we won't look at it)
2. Do validate parameters on your own (because no one else does)
Let me think about it. I can even ask a security provider what their opinion is.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/13256#discussion_r1166948493
