Feature/tls-key-logger flag added #1449
Conversation
|
Jaykumar Jayswal seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
Codecov Report
@@ Coverage Diff @@
## master #1449 +/- ##
==========================================
- Coverage 75.22% 75.15% -0.08%
==========================================
Files 150 150
Lines 10911 10936 +25
==========================================
+ Hits 8208 8219 +11
- Misses 2238 2246 +8
- Partials 465 471 +6
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Sorry for the long delay, I meant to answer you when you published it ... but forgot :( .
I do think this should just be a RuntimeOption as I've written and there is no need for all the validation. It's extremely unlikely we will change our minds, so the extra "freedom" is not really there - we won't allow this to be settable from inside the script.
Also, you will need to open and append to a file instead of reusing the logger one .. as that one is for the logger ;)
Again we are very preoccupied with PR 1007 and this is very unlikely to get merged before we get it out the door, so there is no rush, and thanks for the PR, even if we won't merge it right away :D
| @@ -225,6 +225,7 @@ type Options struct { | |||
|
|
|||
| // Accept invalid or untrusted TLS certificates. | |||
| InsecureSkipTLSVerify null.Bool `json:"insecureSkipTLSVerify" envconfig:"K6_INSECURE_SKIP_TLS_VERIFY"` | |||
| LogTLSKey null.Bool `json:"logTLSKey" envconfig:"K6_LOG_TLS_KEY"` | |||
There was a problem hiding this comment.
This should be a string to a separate file that we log the keys to. Reusing the logger output doesn't seem like a good idea at all
There was a problem hiding this comment.
On second though you should also move it to RuntimeOptions so you don't have a way to set it from inside the script, as that is totally what we want to avoid.
|
Hi jayjayswal, we have finally released v0.27.0 and v0.27.1. I will be really glad if we can get this in v0.28.0. Do you think you will have the time to work on this in the next week or two? |
Hi, Sure i will get this done in a week or two. |
Hi @mstoykov, I am sorry I am not getting time to work on this for now. If it requires to be done in this or next week I won't be able to deliver. I will be free to work on this after next week. So if you need to get this feature live please proceed with the work. Otherwise, I will take it up next to next week. |
Codecov Report
@@ Coverage Diff @@
## master #1449 +/- ##
==========================================
- Coverage 75.22% 75.15% -0.08%
==========================================
Files 150 150
Lines 10911 10936 +25
==========================================
+ Hits 8208 8219 +11
- Misses 2238 2246 +8
- Partials 465 471 +6
Continue to review full report at Codecov.
|
|
Hi @jayjayswal , sorry for the long radio silence :(. I would like if we can get this in k6, and was wondering if you would find the time to finish it? |
|
Closing this as there are two other PRs (#2210 #2481) currently open about the same and no movement here for a while. Thanks for opening this @jayjayswal |
Added TLS keylogger support
Closes #1385