Re: Upgrading to 8.5.20 - issue when certificateKeyAlias is not set

[Jesse Schulman]

Will do, thanks Mark!

On Tue, Aug 22, 2017 at 8:42 AM Mark Thomas  wrote:

> On 21/08/17 22:54, Jesse Schulman wrote:
> > I'm pretty sure this is a bug/regression related to a recent change by
> > markt: http://svn.apache.org/viewvc?view=revision=1800868
> >
> > I think the issue was there before but we weren't hitting it, because the
> > logic of taking the first alias from the keystore (even if it does not
> > alias a key) was already there, but only after this change did we start
> to
> > hit that code.
> >
> > We have worked around the issue with a "getFirstKeyAlias" method that we
> > use to set the certificateKeyAlias in our SSLHostConfigCertificate:
> >
> >private String getFirstKeyAlias(KeyStore keyStore) {
> >   try {
> >  Enumeration aliases = keyStore.aliases();
> >  while(aliases.hasMoreElements()) {
> > String alias = aliases.nextElement();
> > if (keyStore.isKeyEntry(alias))
> >return alias;
> > }
> >   } catch (KeyStoreException e) {
> >   LOGGER.error("Failed to find first key alias in keystore", e);
> >   }
> >
> >   return null;
> >}
> >
> > I think that something like this should around line 219 of JSSEUtil,
> where
> > currently it looks like this:
> >
> > Enumeration aliases = ks.aliases();
> > if (!aliases.hasMoreElements()) {
> > throw new IOException(sm.getString("jsse.noKeys"));
> > }
> > keyAlias = aliases.nextElement();
> >
> >
> > Should I send this to the dev list instead?
>
> If you could create a Bugzilla issue for it, that would be great.
>
> Thanks,
>
> Mark
>
>
> >
> > Thanks!
> > Jesse
> >
> > On Wed, Aug 16, 2017 at 3:02 PM Jesse Schulman 
> wrote:
> >
> >> We use tomcat-embed and we have a test that is breaking with an upgrade
> >> from 8.5.12 to 8.5.20, it seems due to the fact that we do not set the
> >> certificateKeyAlias when we configure an SSLHostConfigCertificate.
> >>
> >> The documentation for certificateKeyAlias states "If not specified, the
> >> first *key* read from the keystore will be used."
> >>
> >> It seems that the first alias is being used and there is no check that
> it
> >> references a key.
> >>
> >> The result is that in JSSEUtil.getKeyManagers there is a call to
> >> KeyStore.getKey(keyAlias, keyPassArray) where keyAlias is actually an
> alias
> >> for a certificate, which leads to inMemoryKeyStore.setKeyEntry being
> passed
> >> null for the Key argument and eventually a KeyStoreException("Cannot
> store
> >> non-PrivateKeys").
> >>
> >> This worked previously with certificatekeyAlias being null.  I can
> confirm
> >> that this works just fine if I set that with the alias used when
> creating
> >> the KeyStore but I would rather not pass that alias around our code
> when I
> >> did not previously need to.
> >>
> >> Thanks!
> >> Jesse
> >>
> >>
> >>
> >
>
>
> -
> To unsubscribe, e-mail: users-unsubscr...@tomcat.apache.org
> For additional commands, e-mail: users-h...@tomcat.apache.org
>
>

Re: Upgrading to 8.5.20 - issue when certificateKeyAlias is not set

[Mark Thomas]

On 21/08/17 22:54, Jesse Schulman wrote:
> I'm pretty sure this is a bug/regression related to a recent change by
> markt: http://svn.apache.org/viewvc?view=revision=1800868
> 
> I think the issue was there before but we weren't hitting it, because the
> logic of taking the first alias from the keystore (even if it does not
> alias a key) was already there, but only after this change did we start to
> hit that code.
> 
> We have worked around the issue with a "getFirstKeyAlias" method that we
> use to set the certificateKeyAlias in our SSLHostConfigCertificate:
> 
>private String getFirstKeyAlias(KeyStore keyStore) {
>   try {
>  Enumeration aliases = keyStore.aliases();
>  while(aliases.hasMoreElements()) {
> String alias = aliases.nextElement();
> if (keyStore.isKeyEntry(alias))
>return alias;
> }
>   } catch (KeyStoreException e) {
>   LOGGER.error("Failed to find first key alias in keystore", e);
>   }
> 
>   return null;
>}
> 
> I think that something like this should around line 219 of JSSEUtil, where
> currently it looks like this:
> 
> Enumeration aliases = ks.aliases();
> if (!aliases.hasMoreElements()) {
> throw new IOException(sm.getString("jsse.noKeys"));
> }
> keyAlias = aliases.nextElement();
> 
> 
> Should I send this to the dev list instead?

If you could create a Bugzilla issue for it, that would be great.

Thanks,

Mark


> 
> Thanks!
> Jesse
> 
> On Wed, Aug 16, 2017 at 3:02 PM Jesse Schulman  wrote:
> 
>> We use tomcat-embed and we have a test that is breaking with an upgrade
>> from 8.5.12 to 8.5.20, it seems due to the fact that we do not set the
>> certificateKeyAlias when we configure an SSLHostConfigCertificate.
>>
>> The documentation for certificateKeyAlias states "If not specified, the
>> first *key* read from the keystore will be used."
>>
>> It seems that the first alias is being used and there is no check that it
>> references a key.
>>
>> The result is that in JSSEUtil.getKeyManagers there is a call to
>> KeyStore.getKey(keyAlias, keyPassArray) where keyAlias is actually an alias
>> for a certificate, which leads to inMemoryKeyStore.setKeyEntry being passed
>> null for the Key argument and eventually a KeyStoreException("Cannot store
>> non-PrivateKeys").
>>
>> This worked previously with certificatekeyAlias being null.  I can confirm
>> that this works just fine if I set that with the alias used when creating
>> the KeyStore but I would rather not pass that alias around our code when I
>> did not previously need to.
>>
>> Thanks!
>> Jesse
>>
>>
>>
> 


-
To unsubscribe, e-mail: users-unsubscr...@tomcat.apache.org
For additional commands, e-mail: users-h...@tomcat.apache.org

Re: Upgrading to 8.5.20 - issue when certificateKeyAlias is not set

[Jesse Schulman]

I'm pretty sure this is a bug/regression related to a recent change by
markt: http://svn.apache.org/viewvc?view=revision=1800868

I think the issue was there before but we weren't hitting it, because the
logic of taking the first alias from the keystore (even if it does not
alias a key) was already there, but only after this change did we start to
hit that code.

We have worked around the issue with a "getFirstKeyAlias" method that we
use to set the certificateKeyAlias in our SSLHostConfigCertificate:

   private String getFirstKeyAlias(KeyStore keyStore) {
  try {
 Enumeration aliases = keyStore.aliases();
 while(aliases.hasMoreElements()) {
String alias = aliases.nextElement();
if (keyStore.isKeyEntry(alias))
   return alias;
}
  } catch (KeyStoreException e) {
  LOGGER.error("Failed to find first key alias in keystore", e);
  }

  return null;
   }

I think that something like this should around line 219 of JSSEUtil, where
currently it looks like this:

Enumeration aliases = ks.aliases();
if (!aliases.hasMoreElements()) {
throw new IOException(sm.getString("jsse.noKeys"));
}
keyAlias = aliases.nextElement();


Should I send this to the dev list instead?

Thanks!
Jesse

On Wed, Aug 16, 2017 at 3:02 PM Jesse Schulman  wrote:

> We use tomcat-embed and we have a test that is breaking with an upgrade
> from 8.5.12 to 8.5.20, it seems due to the fact that we do not set the
> certificateKeyAlias when we configure an SSLHostConfigCertificate.
>
> The documentation for certificateKeyAlias states "If not specified, the
> first *key* read from the keystore will be used."
>
> It seems that the first alias is being used and there is no check that it
> references a key.
>
> The result is that in JSSEUtil.getKeyManagers there is a call to
> KeyStore.getKey(keyAlias, keyPassArray) where keyAlias is actually an alias
> for a certificate, which leads to inMemoryKeyStore.setKeyEntry being passed
> null for the Key argument and eventually a KeyStoreException("Cannot store
> non-PrivateKeys").
>
> This worked previously with certificatekeyAlias being null.  I can confirm
> that this works just fine if I set that with the alias used when creating
> the KeyStore but I would rather not pass that alias around our code when I
> did not previously need to.
>
> Thanks!
> Jesse
>
>
>

Upgrading to 8.5.20 - issue when certificateKeyAlias is not set

[Jesse Schulman]

We use tomcat-embed and we have a test that is breaking with an upgrade
from 8.5.12 to 8.5.20, it seems due to the fact that we do not set the
certificateKeyAlias when we configure an SSLHostConfigCertificate.

The documentation for certificateKeyAlias states "If not specified, the
first *key* read from the keystore will be used."

It seems that the first alias is being used and there is no check that it
references a key.

The result is that in JSSEUtil.getKeyManagers there is a call to
KeyStore.getKey(keyAlias, keyPassArray) where keyAlias is actually an alias
for a certificate, which leads to inMemoryKeyStore.setKeyEntry being passed
null for the Key argument and eventually a KeyStoreException("Cannot store
non-PrivateKeys").

This worked previously with certificatekeyAlias being null.  I can confirm
that this works just fine if I set that with the alias used when creating
the KeyStore but I would rather not pass that alias around our code when I
did not previously need to.

Thanks!
Jesse