On Fri, 14 Feb 2025 21:13:45 GMT, Sean Mullan <mul...@openjdk.org> wrote:

>> Martin Balao has updated the pull request with a new target base due to a 
>> merge or a rebase. The pull request now contains four commits:
>> 
>>  - Merge JDK-8345139 into JDK-8315487
>>    
>>    This way, we are syncing with the dependency of this PR.
>>    
>>    Manually apply src/java.base/share/classes/java/security/Provider.java
>>    changes, as a lot of context changed after JDK-8345139 updates.
>>  - Copyright date update.
>>    
>>    Co-authored-by: Martin Balao Alonso <mba...@redhat.com>
>>    Co-authored-by: Francisco Ferrari Bihurriet <fferr...@redhat.com>
>>  - Add a debug message to inform the Filter property value read.
>>    
>>    See more in 
>> https://mail.openjdk.org/pipermail/security-dev/2024-October/041800.html
>>    
>>    Co-authored-by: Martin Balao Alonso <mba...@redhat.com>
>>    Co-authored-by: Francisco Ferrari Bihurriet <fferr...@redhat.com>
>>  - 8315487: Security Providers Filter
>>    
>>    Co-authored-by: Francisco Ferrari Bihurriet <fferr...@redhat.com>
>>    Co-authored-by: Martin Balao <mba...@redhat.com>
>
>> I have been starting to review the code, and am initially reviewing this 
>> with respect to how it complies with the current API specification.
>> 
>> All of the JCA API `getInstance` methods that do not have a provider 
>> argument have text like the following:
>> 
>> > This method traverses the list of registered security Providers, starting 
>> > with the most preferred Provider. A new `<service>` object encapsulating 
>> > the `<service>Spi` implementation from the first provider that supports 
>> > the specified algorithm is returned.
>> 
>> However, the providers filter can be configured to prevent that object from 
>> being returned. I think this is an important difference in behavior that it 
>> should be documented as an implementation note. My initial suggestion is 
>> something like the following:
>> 
>> "The JDK Reference Implementation additionally uses the 
>> `jdk.security.providers.filter` system and security property to determine 
>> which services are enabled. A provider whose algorithm is not enabled by the 
>> filter will not be selected."
>> 
>> I think similar text will need to be added in the `Provider` API, but I need 
>> to review those changes more closely first.
> 
> @martinuy Any comments on the above?

Hi @seanjmullan,

We agree, including the need for documenting this in `Provider::getService` and 
`Provider::getServices`.

We will make these changes in the coming days, we were first addressing #22613 
changes and adjusting this PR which depends on it. The two PRs are now in sync, 
so feel free to continue your review in the meantime.

-------------

PR Comment: https://git.openjdk.org/jdk/pull/15539#issuecomment-2660337392

Reply via email to