[8u] RFR: 8076190: Customizing the generation of a PKCS12 keystore

Wed Feb 2 15:02:39 UTC 2022

Hi Alexey,

Thanks for your insights.

JDK-8245169 is a bug affecting 8u and should be trivial / no-risk to
backport. That will be easy to do, review and get maintainers approval.

As I understand it, the only argument to justify an 8u backport of
JDK-8076190 would be to minimize behavior differences between JKS and
PKCS12 keystores. Can you please explain how this is affecting users? A
real use-case would help. In my view, we would need a strong reason to move
forward because 8076190 is not an easy fit for 8u and we would need to look
at backward-compatibility issues thoroughly -which would be a time
investment for both of us-.

Thanks,
Martin.-

On Wed, Feb 2, 2022 at 9:35 AM Alexey Bakhtin <alexey at azul.com> wrote:

> Hi Martin,
>
> Thank you for review. I see the issue with SunJCE now.
> There are several reasons to request this backport. The first, as you
> mentioned, is password-less PKCS#12 keystores. It minimises behavior
> differences between JKS and PKCS12 keystores and makes keytool behavior
> close to JDK11+ version.
> Another reason of this backport is to fix incompatibility issues between
> OpenSSL and JDK8 keystores. It is described in JDK-8245169 [1] and fixed by
> JDK-8076190.
>
> In case of these changes are too major for JDK8 release, I d’like to fix
> JDK-8245169 issue (PBES2Parameters file only [2] ) in a separate review.
>
> Thank you
> Alexey
>
> [1] - https://bugs.openjdk.java.net/browse/JDK-8245169
> [2] -
> http://cr.openjdk.java.net/~abakhtin/8076190_8u/webrev.v1/src/share/classes/com/sun/crypto/provider/PBES2Parameters.java.udiff.html
>
>
> > On 2 Feb 2022, at 00:47, Martin Balao <mbalao at redhat.com> wrote:
> >
> > Hi Alexey,
> >
> > Thanks for proposing a new version of your patch.
> >
> > If we change the default behavior for an empty "storetype" parameter
> (that is: trying to get the value from the file using a SunJCE class
> instead of using the default keystore type value), we would need to include
> that in the CSR for 8u. However, I believe that this behavioral change is
> not strictly required and have a couple of concerns:
> >
> > 1) We are using a SunJCE class as the only engine prober and perhaps
> SunJCE is not even available/enabled in runtime. This is different in
> JDK-11 because all security providers are probed through a JCA API, and if
> one happens to handle PKCS#12 and identify the InputStream as such, we get
> a KeyStore instance of it.
> >
> > 2) In JDK-11, the "storetype" value is whatever the probed security
> provider wants it to be. In JDK-8, we would be fixing the algorithm name to
> SunJCE's "pkcs12" value -even if SunJCE is not available/enabled as
> mentioned in #1-. This can break backward-compatibility in my view. The
> scenario that I have in mind is for example: SunJCE disabled, a 3rd party
> security provider that handles PKCS#12 and calls the algorithm "P12", a
> java.security property set with "keystore.type=P12" and an invocation to
> Keytool without a "storetype" parameter. In that case, SunJCE will jump-in,
> say that it's a PKCS#12 keystore and rename "storetype" from the default
> "P12" to "pkcs12". The invocation will later fail at "keyStore =
> KeyStore.getInstance(storetype);".
> >
> > In general, I'm a bit concerned about the 8u backport of this
> enhancement. Can you please elaborate on the reasons why it would be
> justified for such an old release? It looks to me that we are looking into
> password-less PKCS#12 keystores and being more flexible with the algorithms
> used to encrypt certificates and keystore integrity (note that private keys
> encryption is not under the scope of this enhancement, as there is already
> flexibility to choose the algorithms). While the latter could be seen as a
> security concern, we should be able to leverage on the file-system/OS
> permissions for that. Let me know of your thoughts. I'd like the 8u
> maintainers to be involved in this discussion as well.
> >
> > Thanks,
> > Martin.-
> >
> >
> > On Wed, Sep 8, 2021 at 1:54 PM Alexey Bakhtin <alexey at azul.com> wrote:
> > Hi Martin,
> >
> > Thank you a lot for review.
> > I’ve simplified keytool code almost as you suggested but I still think
> PKCS12 probing is useful here.
> > I’m agree, the first version of kstype detection was not correct, so
> I’ve implemented it via PKCS12KeyStore.probe as you suggested in the first
> mail.
> > Without such probing the passwordless feature looks strange: "keytool
> -list” cmd prints PKCS12 store type and warning about password for the
> passwordless keystore (if storetype is not indicated explicitly). The same
> is for other commands.
> > Also, I have removed changes in the
> test/lib/jdk/test/lib/SecurityTools.java and updated tests to use
> jdk/test/lib/jdk/test/lib/SecurityTools.java
> >
> > All sun/security/pkcs12  and sun/security/tools/keytool tests passed
> >
> > New version of the patch is available at :
> http://cr.openjdk.java.net/~abakhtin/8076190_8u/webrev.v1/
> >
> > Regards
> > Alexey
> >
> > > On 24 Aug 2021, at 18:50, Martin Balao <mbalao at redhat.com> wrote:
> > >
> > > Hi Alexey,
> > >
> > > Final comments for this first round:
> > >
> > > * src/share/classes/sun/security/tools/keytool/Main.java
> > >  * @@ -2025,7 +2058,14 @@
> > >   * Seems to have an issue similar to the one before:
> > >    * 'srcstoretype' is changed based on probing (which was not a
> > > JDK-8 behavior)
> > >    * the value set is based on the assumption that
> > > '!"JKS".equalsIgnoreCase(realType)' is PKCS#12 and that may not be
> > > correct on some cases
> > >    * there might be some redundancy in the checks 'srcksfile != null
> > > && is != null && srcProviderName == null'
> > >   * I'd consider a similar approach to the one proposed earlier: take
> > > the 'srcstoretype' value for certain, compare against 'pkcs12' and
> > > handle the existing-keystore scenario. In this case, 'is' seems to be
> > > the variable to decide whether there is an existing keystore or not.
> > >
> > > * test/lib/jdk/test/lib/SecurityTools.java
> > >  * For some reason, there are 2 SecurityTools files in JDK-8:
> > >   * jdk/test/lib/testlibrary/jdk/testlibrary/SecurityTools.java
> > >   * jdk/test/lib/jdk/test/lib/SecurityTools.java
> > >  * Can we avoid this change by making the test use the other one?
> > >   * Looks like
> > > 'jdk/test/lib/testlibrary/jdk/testlibrary/SecurityTools.java' is the
> > > one that you want for the test
> > >   * If we apply your proposed change, then the 2 libs will be equal
> > > in regards to the affected method and I'm in general concerned about
> > > the side-effects of modifying libraries; the set of regression tests
> > > that you ran may not be enough to make sure that we don't introduce a
> > > regression.
> > >
> > > Thanks,
> > > Martin.-
> > >
> >
>
>