TLS Auth support providing a passphrase for protected private key

[xiananfan]

Feature Description

K6 currently only supports providing cleartext public key and private key for TLS Authentication. This is usually considered insecure in a production system.

  tlsAuth: [
    {
      domains: ['example.com'],
      cert: open('./mycert.pem'),
      key: open('./mycert-key.pem'),
    },
  ],

Please consider adding an additional field, like "passphrase" in the setting, so the private key is not a cleartext pem file. Then the passphrase can be retrieved from a secret store later on during test runs. So the security can be increased.

Something like below:

  tlsAuth: [
    {
      domains: ['example.com'],
      cert: open('./mycert.pem'),
      key: open('./mycert-key.pem'),
      passphrase: 'SecretPassword'
    },
  ],

Suggested Solution (optional)

No response

Already existing or connected issues / PRs (optional)

No response

[mstoykov]

Unlike #2434 which might be a lot harder to implement, I think this will be fairly straightforward.

I do hope that somebody from the community, maybe you @xiananfan, will pick it up. Otherwise, this might have to wait a bit more until someone from the team has time to look into it more closely.

[Gabrielopesantos]

Hello, I would to take this issue if possible. Can you share some details like relevant files to start looking on? Thank you in advance.

[mstoykov]

Hi @Gabrielopesantos, here is where most of the current parsing is happening.

Hope this is enough of guidance as if I have to tell you where to touch what I will probably need to write it myself :)

Good luck and thanks for taking it on

[Gabrielopesantos]

Hello @mstoykov, finally had time to look into this issue but I am not sure if the able to proceed with a possible solution with the current context. I understand the needing of a passphrase but I don't get the part "Then the passphrase can be retrieved from a secret store later on during test runs. So the security can be increased.".

Can somebody clarify what is meant with that? Thank you in advance.

[xiananfan]

@Gabrielopesantos What I meant before is that for the K6 implementation, just need to consider a clear-text passphrase will be passed in. For example, if you set up K6 to run automatically and periodically in a Jenkins pipeline, you would store the certificate passphrase in the Jenkins secret store, during each pipeline run, the Jenkins script would be able to access the certificate passphrase, and pass it into the K6 script.

I think K6 also supports multiple different arguments to the CLI tool, so if this can be exposed as a new parameter, that would be even better. But personally, I would consider that a separate PR.

[mstoykov]

@Gabrielopesantos I agree with @xiananfan - for this issue we only care about adding passphrase (maybe password 🤔 ) field and that to be used to open the provided certificate.

Any additional features on top of that will be in separate PRs.

The above will still let user get the password in a more secure ways such as having them in a env variable and using it in the script:

  tlsAuth: [
    {
      domains: ['example.com'],
      cert: open('./mycert.pem'),
      key: open('./mycert-key.pem'),
      passphrase: __ENV.SECRET
    },
  ],

[Gabrielopesantos]

Hello, did a little bit of research and the Go standard library does not seem to support parsing encrypted PKCS8 private keys (Reference: golang/go#8860). There are third party options that provide this feature (Reference: https://github.com/youmark/pkcs8). Is importing a third party library ok, as long as it is justifiable, or should the implementation only support the traditional format?

[mstoykov]

@Gabrielopesantos I think you went down the wrong path - those are x509 certificates in pem format. So the issue you probably should've ended up at is golang/go#10181 which will link you to more or less what needs to be changed IMO.

[Gabrielopesantos]

You are right, the link you sent me is more like what we are trying to solve here, but looking at the discussion it still hits what I talking yesterday. The DecryptPEMBlock function (http://golang.org/pkg/crypto/x509/#DecryptPEMBlock) can only decrypt blocks with a "DEK-Info header" which is not present in PKCS8 formatted keys. Also the function was deprecated in Go 1.16.

[mstoykov]

I am not particularly versed in cryptography terminology or RFCs, but it seems that this is exactly the function we want. We have PEM encoded certificate support, we want to support password encoded such ones. And this decrypts PEM private keys.

I am not certain how PKCS8 comes into all of that, but if we are not supporting it now, I would argue it's fine that we don't support it with password as well. Not supporting PKCS8 certificates with passwords is also probably okay as a first version.

The fact that it's deprecated has more to do (from what I can gather) with the go's team wanting to signal you probably should not be using this functionality at all. Which I (personally) am aware off but unfortunately this is the easiest thing we can add and it's still an improvement over no password.

Thinking about this some more we technically only support directly the pem formatted(text) it is actually 100% possible to do

KEY=`openssl ... <command to return the key on stdout>` k6 run script.js

and in the script to have

  tlsAuth: [
    {
      domains: ['example.com'],
      cert: open('./mycert.pem'),
      key: __ENV.KEY,
    },
  ],

Which I guess is an okay workaround for most/all(:thinking: ) cases ?

[Gabrielopesantos]

Hey, created a draft PR with a proposal for the solution of this problem. However, I am not too sure about the tests that I have created. I tried to cover the following cases:

• key is not encrypted and password is provided
• key is encrypted and provided password is invalid
• key is encrypted and provided password is valid
• key is in pkcs8 format so pem.IsEncryptedPEMBlock assumes that the block is not encrypted since it does not have DEK-info

I my opinion, these cover all cases but I wanted to make if you agree before marking the PR as ready to review.

The linter is also detecting some paralleltest issue but I intend to fix those before marking it as ready to review.

Thank you.

[mstoykov]

Thanks for making the PR 🙇 You can make it ready for review and we can discuss any required changes there.

key is in pkcs8 format so pem.IsEncryptedPEMBlock assumes that the block is not encrypted since it does not have DEK-info

Shouldn't we error out on this then? Or do you mean that this will work for not encrypted pkcs8 files?

[Gabrielopesantos]

Makes sense, made an additional commit to throw an error in that case and opened the PR to discuss the if there are any required changes.

[na--]

This should have been closed by #2488 and #2503, right?