-
Notifications
You must be signed in to change notification settings - Fork 15
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
Pad short secrets #14
Conversation
I’ve always wondered what the scenario is where Splunk.secret is that short… is it caused by a system that’s been upgraded from a very old version? |
I'm not sure if there's anything that would cause it other than users configuring their own I originally noticed this when testing with the |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #14 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 1 1
Lines 201 165 -36
=========================================
- Hits 201 165 -36
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Was failing due to the function now padding the secret
Hope you don't mind me making changes; looks like another test needed updating. :) |
Not at all, good catch. I didn't look closely at all of the tests and that one was still passing 🤦 Before your change I get: ======================================================================
ERROR: test_decrypt_raises_value_error_short_secret2 (tests.TestSplunkSecrets)
----------------------------------------------------------------------
Traceback (most recent call last):
File "/workspaces/splunksecrets/tests.py", line 140, in test_decrypt_raises_value_error_short_secret2
splunksecrets.decrypt(
File "/workspaces/splunksecrets/splunksecrets.py", line 86, in decrypt
plaintext = decryptor.update(ciphertext).decode()
UnicodeDecodeError: 'utf-8' codec can't decode byte 0xee in position 1: invalid continuation byte
---------------------------------------------------------------------- when commenting out the |
732d686 ensures that the secret is bytes so that padding doesn't fail with I decided to fix it in both places so that things work if folks are using the CLI or calling |
This PR updates the
encrypt_new
anddecrypt
(for Splunk 7.2 or later secrets) functions to pad thesplunk.secret
to 254 bytes if the file is less than 254 bytes.If we strace a
splunk show-encrypted
call, we can see that the splunkd binary does the same padding for short secrets. When this happens it also writes a Read custom key data size=[bytes] message to stderr.If the contents of
splunk.secret
are 254 bytes (or more) it only reads 254 bytes and doesn't output the custom key data size message. Both of these cases are already covered bysplunksecrets.py
.Testing
I've updated the tests accordingly but for manual testing I used a
splunk/splunk
container and the updatedsplunksecrets.py
: