Skip to content
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

[Fleet] [Message Signing] Respond with a generic error when rotate key pair fails #156144

Merged

Conversation

ashokaditya
Copy link
Member

@ashokaditya ashokaditya commented Apr 28, 2023

Summary

Responds with a generic error message for the rotate key pair API, instead of a trace message of where the error was generated.

We log the detailed errors and this is to avoid a chance of any sensitive data (key generation details) being exposed through the API response.

follow up of /pull/155864

Checklist

@ashokaditya ashokaditya added release_note:skip Skip the PR/issue when compiling release notes Team:Defend Workflows “EDR Workflows” sub-team of Security Solution OLM Sprint v8.8.0 v8.9.0 labels Apr 28, 2023
@ashokaditya ashokaditya self-assigned this Apr 28, 2023
@ashokaditya ashokaditya force-pushed the tsk/dw-rotate-key-pair-api-error-response branch from 305736c to 83eefa2 Compare April 28, 2023 08:52
@ashokaditya ashokaditya marked this pull request as ready for review April 28, 2023 08:52
@ashokaditya ashokaditya requested a review from a team as a code owner April 28, 2023 08:52
@elasticmachine
Copy link
Contributor

Pinging @elastic/security-defend-workflows (Team:Defend Workflows)

@ashokaditya ashokaditya force-pushed the tsk/dw-rotate-key-pair-api-error-response branch from 83eefa2 to 5778d80 Compare April 28, 2023 09:02
@botelastic botelastic bot added the Team:Fleet Team label for Observability Data Collection Fleet team label Apr 28, 2023
@elasticmachine
Copy link
Contributor

Pinging @elastic/fleet (Team:Fleet)

@@ -41,6 +41,6 @@ export const rotateKeyPairHandler: FleetRequestHandler<
});
} catch (error) {
logger.error(error.meta);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we log the whole error instead of error.meta here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, why not. I'll update it.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done 723461e

Copy link
Member

@nchaulet nchaulet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Unknown metric groups

ESLint disabled line counts

id before after diff
enterpriseSearch 19 21 +2
securitySolution 399 402 +3
total +5

Total ESLint disabled count

id before after diff
enterpriseSearch 20 22 +2
securitySolution 479 482 +3
total +5

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @ashokaditya

Copy link
Contributor

@dasansol92 dasansol92 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you check that the error obj is logged correctly? I guess logger will take care of json obj stringify but just in case. Other than that this looks good, thanks for the changes!

@ashokaditya
Copy link
Member Author

Did you check that the error obj is logged correctly? I guess logger will take care of json obj stringify but just in case. Other than that this looks good, thanks for the changes!
Yes, I verified this locally.

Here's a screenshot of logging the error from the tests that goes into log.error
Screenshot 2023-04-28 at 17 15 32

@ashokaditya ashokaditya merged commit 13486f5 into elastic:main Apr 28, 2023
@ashokaditya ashokaditya deleted the tsk/dw-rotate-key-pair-api-error-response branch April 28, 2023 15:17
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Apr 28, 2023
…y pair fails (elastic#156144)

## Summary

Responds with a generic error message for the rotate key pair API,
instead of a trace message of where the error was generated.

We log the detailed errors and this is to avoid a chance of any
sensitive data (key generation details) being exposed through the API
response.

follow up of elastic/pull/155864

### Checklist

- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios

(cherry picked from commit 13486f5)
@kibanamachine
Copy link
Contributor

💚 All backports created successfully

Status Branch Result
8.8

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

kibanamachine added a commit that referenced this pull request Apr 28, 2023
…ate key pair fails (#156144) (#156188)

# Backport

This will backport the following commits from `main` to `8.8`:
- [[Fleet] [Message Signing] Respond with a generic error when rotate
key pair fails (#156144)](#156144)

<!--- Backport version: 8.9.7 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT
[{"author":{"name":"Ashokaditya","email":"1849116+ashokaditya@users.noreply.github.com"},"sourceCommit":{"committedDate":"2023-04-28T15:17:06Z","message":"[Fleet]
[Message Signing] Respond with a generic error when rotate key pair
fails (#156144)\n\n## Summary\r\n\r\nResponds with a generic error
message for the rotate key pair API,\r\ninstead of a trace message of
where the error was generated.\r\n\r\nWe log the detailed errors and
this is to avoid a chance of any\r\nsensitive data (key generation
details) being exposed through the API\r\nresponse.\r\n\r\nfollow up of
/pull/155864\r\n\r\n### Checklist\r\n\r\n- [x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common
scenarios","sha":"13486f50deb6070f4e9ab00de63827caf0ed6040","branchLabelMapping":{"^v8.9.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","Team:Fleet","Team:Defend
Workflows","OLM
Sprint","v8.8.0","v8.9.0"],"number":156144,"url":"https://github.com/elastic/kibana/pull/156144","mergeCommit":{"message":"[Fleet]
[Message Signing] Respond with a generic error when rotate key pair
fails (#156144)\n\n## Summary\r\n\r\nResponds with a generic error
message for the rotate key pair API,\r\ninstead of a trace message of
where the error was generated.\r\n\r\nWe log the detailed errors and
this is to avoid a chance of any\r\nsensitive data (key generation
details) being exposed through the API\r\nresponse.\r\n\r\nfollow up of
/pull/155864\r\n\r\n### Checklist\r\n\r\n- [x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common
scenarios","sha":"13486f50deb6070f4e9ab00de63827caf0ed6040"}},"sourceBranch":"main","suggestedTargetBranches":["8.8"],"targetPullRequestStates":[{"branch":"8.8","label":"v8.8.0","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v8.9.0","labelRegex":"^v8.9.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/156144","number":156144,"mergeCommit":{"message":"[Fleet]
[Message Signing] Respond with a generic error when rotate key pair
fails (#156144)\n\n## Summary\r\n\r\nResponds with a generic error
message for the rotate key pair API,\r\ninstead of a trace message of
where the error was generated.\r\n\r\nWe log the detailed errors and
this is to avoid a chance of any\r\nsensitive data (key generation
details) being exposed through the API\r\nresponse.\r\n\r\nfollow up of
/pull/155864\r\n\r\n### Checklist\r\n\r\n- [x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common
scenarios","sha":"13486f50deb6070f4e9ab00de63827caf0ed6040"}}]}]
BACKPORT-->

Co-authored-by: Ashokaditya <1849116+ashokaditya@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
OLM Sprint release_note:skip Skip the PR/issue when compiling release notes Team:Defend Workflows “EDR Workflows” sub-team of Security Solution Team:Fleet Team label for Observability Data Collection Fleet team v8.8.0 v8.9.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants