[Alerting] Secret Service #28894
[Alerting] Secret Service #28894
Conversation
The secret store should define saved object middleware.
Secret store should use hard crypto algorithms to secure data. Secret store should use object hashes to validate saved objects are not modified.
| } | ||
| }; | ||
|
|
||
| this.validateKey = async () => { |
There was a problem hiding this comment.
Validating the key during init in this manner is going to prevent us from ever changing the encryption key.
There was a problem hiding this comment.
Can you please elaborate on this a bit? What would be the recommended way of changing/rotating encryption keys in Kibana (e.g. for Reporting)?
There was a problem hiding this comment.
You would validate you have a valid key first then you would generate a new key, encrypt the new key in an object with the old key or a temporary key indicating a rotation, and begin fetching all the secret objects and updating them by decrypting with the old key and encrypting with the new key until all are fully changed, then update the keystore with the new key.
There would probably need to be a method for marking the secrets as needing to be re-encrypted or you'd just have to force them all to be processed each time the rotation happens starting over but validating all objects have been rotated properly.
| }).default(); | ||
| }, | ||
|
|
||
| async init(server: any) { |
There was a problem hiding this comment.
nit: maybe server: Legacy.Server (import { Legacy } from 'kibana';) instead of server: any?
| return dummyObj; | ||
| }); | ||
| const isValid = await subject.validateKey(); | ||
| expect(isValid).toBeTruthy(); |
There was a problem hiding this comment.
optional nit: expect(isValid).toBe(true); feels a bit more correct (and similar note for test cases below).
| return dummyObj; | ||
| } | ||
| ); | ||
| savedObjectsClient.get.mockImplementation((type: string, attributes: any, options?: any) => { |
There was a problem hiding this comment.
nit: there is no need to define unused arguments (here and below)
| throw new Error('test error'); | ||
| } | ||
| ); | ||
| savedObjectsClient.get.mockImplementation((type: string, attributes: any, options?: any) => { |
There was a problem hiding this comment.
nit: looks like savedObjectsClient.get won't be called? If so, we can just use this:
savedObjectsClient.create.mockRejectedValue(new Error('test error'));| }); | ||
|
|
||
| it('should throw any error other than conflict', async () => { | ||
| savedObjectsClient.create.mockImplementation( |
There was a problem hiding this comment.
nit: savedObjectsClient.create.mockRejectedValue(new Error('test error'));
| } | ||
| }; | ||
|
|
||
| this.validateKey = async () => { |
There was a problem hiding this comment.
Can you please elaborate on this a bit? What would be the recommended way of changing/rotating encryption keys in Kibana (e.g. for Reporting)?
If we wanted to change/rotate the encryption keys for Reporting/Security, we could just change the keys and all pending Jobs and cookies would be invalid, but outside of that we'd require no additional changes. The other issue with doing this validation on |
SavedObjectsRepository needs full typing.
Extract config key constant. Remove unused mock class. Convert to correct type for SavedObjectsRepository. Use typescript auto declared members for class. Rename the key to encryptionKey. Use uuid v4 for id generation. Remove unnecessary data from log messages.
💚 Build Succeeded |
💔 Build Failed |
💔 Build Failed |
💔 Build Failed |
Summary
Main implementation PR for meta issue #26975
Blocked by #28722
Checklist
Use
strikethroughsto remove checklist items you don't feel are applicable to this PR.keystoreto autogenerate encryption key if not in config.Pass encryption keys to pluginMiddleware for SavedObjectService that filters secret saved objectsThis was checked for cross-browser compatibility, including a check against IE11Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n supportThis was checked for keyboard-only and screenreader accessibilityFor maintainers