New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Recorder: SslServerContext assumes the keyStore and KeyManager have the same password #2971

Closed
ttzvika opened this Issue Apr 18, 2016 · 10 comments

Comments

Projects
None yet
3 participants
@ttzvika

ttzvika commented Apr 18, 2016

In SslServerContext the keyStrore is initialized like so:

lazy val keyStore = {
      val ks = KeyStore.getInstance(keyStoreType.toString)
      withCloseable(keyStoreInitStream) { ks.load(_, password) }
      ks
    }

and the KeyManager is initialized like so

 // Set up key manager factory to use our key store
      val kmf = KeyManagerFactory.getInstance(Algorithm)
      kmf.init(keyStore, password)

The code assumes they both use the same password. In reality, if there are two different passwords needed the you will get the an exception:

java.security.UnrecoverableKeyException: Cannot recover key

@slandelle slandelle changed the title from SslServerContext assumes the keyStore and KeyManager have the same password to Recorder: SslServerContext assumes the keyStore and KeyManager have the same password Apr 18, 2016

ggsjyoon added a commit to ggsjyoon/gatling that referenced this issue Apr 22, 2016

Split password into keyStorePassword and keyManagerFactoryPassword in…
… SslServerContext, ConfigKeys, RecorderConfiguration, and recorder-defaults.conf. Issue #2971.
@ggsjyoon

This comment has been minimized.

Show comment
Hide comment
@ggsjyoon

ggsjyoon Apr 22, 2016

Hello. I created a pull-request for the issue #2971. Could you review it when you have free time?

ggsjyoon commented Apr 22, 2016

Hello. I created a pull-request for the issue #2971. Could you review it when you have free time?

@slandelle

This comment has been minimized.

Show comment
Hide comment
@slandelle

slandelle Apr 26, 2016

Member

@ttzvika Is having different passwords for the keystore and the key something you actually do? Or is it something you stumbled upon when reading the code?

Member

slandelle commented Apr 26, 2016

@ttzvika Is having different passwords for the keystore and the key something you actually do? Or is it something you stumbled upon when reading the code?

ggsjyoon added a commit to ggsjyoon/gatling that referenced this issue May 8, 2016

Split password into keyStorePassword and keyManagerFactoryPassword in…
… SslServerContext, ConfigKeys, RecorderConfiguration, and recorder-defaults.conf. Issue #2971.
@ttzvika

This comment has been minimized.

Show comment
Hide comment
@ttzvika

ttzvika Nov 21, 2016

@slandelle it is something we actually did. To work around this we changed the keymanager password to be the same as the keystore password which solved the problem

ttzvika commented Nov 21, 2016

@slandelle it is something we actually did. To work around this we changed the keymanager password to be the same as the keystore password which solved the problem

@slandelle

This comment has been minimized.

Show comment
Hide comment
@slandelle

slandelle Nov 21, 2016

Member

Good to know. As this usage is not very common, and there's an easy workaround, that's not really a priority for us. Contribs welcome.

Member

slandelle commented Nov 21, 2016

Good to know. As this usage is not very common, and there's an easy workaround, that's not really a priority for us. Contribs welcome.

@ggsjyoon

This comment has been minimized.

Show comment
Hide comment
@ggsjyoon

ggsjyoon Nov 21, 2016

Should I re-open my PR?

ggsjyoon commented Nov 21, 2016

Should I re-open my PR?

@ttzvika

This comment has been minimized.

Show comment
Hide comment
@ttzvika

ttzvika Nov 21, 2016

Only if others find interest in it as well

ttzvika commented Nov 21, 2016

Only if others find interest in it as well

@slandelle

This comment has been minimized.

Show comment
Hide comment
@slandelle

slandelle Nov 21, 2016

Member

@ggsjyoon And your PR was missing the UI part.

Member

slandelle commented Nov 21, 2016

@ggsjyoon And your PR was missing the UI part.

@ggsjyoon

This comment has been minimized.

Show comment
Hide comment
@ggsjyoon

ggsjyoon Nov 21, 2016

My bad. You're right.

ggsjyoon commented Nov 21, 2016

My bad. You're right.

@slandelle

This comment has been minimized.

Show comment
Hide comment
@slandelle

slandelle Nov 21, 2016

Member

@ggsjyoon Please ping me if you can come up with a more complete version :)

Member

slandelle commented Nov 21, 2016

@ggsjyoon Please ping me if you can come up with a more complete version :)

@slandelle

This comment has been minimized.

Show comment
Hide comment
@slandelle

slandelle Jun 14, 2018

Member

This feature request has been idle for a very long time. Closing for now.

Member

slandelle commented Jun 14, 2018

This feature request has been idle for a very long time. Closing for now.

@slandelle slandelle closed this Jun 14, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment