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

Put License API can return 500 #74058

Closed
jkakavas opened this issue Jun 14, 2021 · 2 comments · Fixed by #79093
Closed

Put License API can return 500 #74058

jkakavas opened this issue Jun 14, 2021 · 2 comments · Fixed by #79093
Assignees
Labels
>bug :Security/License License functionality for commercial features Team:Security Meta label for security team

Comments

@jkakavas
Copy link
Member

In cases where the license is malformed, LicenseVerifier might throw an IllegalStateException or some other unexpected Exception, which causes the elasticsearch node to throw a 500.
We could attempt to better catch these types of errors and return an appropriate HTTP status code and a helpful error message or maybe return a PutLicenseResponse with status being INVALID

Example reproduction :

curl -k -XPUT  'http://<ES_IP_ADDRESS>:9200/_license' -H "Content-Type: application/json" -d @license.file

Using a valid license with it's signature field being truncated/edited

@jkakavas jkakavas added >bug :Security/License License functionality for commercial features needs:triage Requires assignment of a team area label labels Jun 14, 2021
@elasticmachine elasticmachine added the Team:Security Meta label for security team label Jun 14, 2021
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-security (Team:Security)

@nik9000 nik9000 added team-discuss and removed needs:triage Requires assignment of a team area label labels Jun 15, 2021
@tvernum
Copy link
Contributor

tvernum commented Jul 29, 2021

We removed team-discuss because there's not really much to discuss. We shouldn't return 500 status codes just because there was bad data. It's worth fixing this when we can.

@justincr-elastic justincr-elastic self-assigned this Oct 12, 2021
justincr-elastic added a commit to justincr-elastic/elasticsearch that referenced this issue Oct 13, 2021
License.fromXContent used to throw ElasticsearchException.
ExceptionsHelper.status mapped that to HTTP 500.

License.fromXContent now throws ElasticsearchParseException.
ExceptionsHelper.status maps that to HTTP 400.

Added LicenseIT.testPutInvalidTrialLicense to test with
truncated license signature.

Closes elastic#74058
justincr-elastic added a commit to justincr-elastic/elasticsearch that referenced this issue Oct 13, 2021
License.fromXContent used to throw ElasticsearchException.
ExceptionsHelper.status mapped that to HTTP 500.

License.fromXContent now throws ElasticsearchParseException.
ExceptionsHelper.status maps that to HTTP 400.

Added LicenseIT.testPutInvalidTrialLicense to test with
truncated license signature.

Closes elastic#74058
justincr-elastic added a commit to justincr-elastic/elasticsearch that referenced this issue Oct 13, 2021
License.fromXContent used to throw ElasticsearchException.
ExceptionsHelper.status mapped that to HTTP 500.

License.fromXContent now throws ElasticsearchParseException.
ExceptionsHelper.status maps that to HTTP 400.

Added LicenseIT.testPutInvalidTrialLicense to test with
truncated license signature.

Closes elastic#74058
justincr-elastic added a commit that referenced this issue Oct 21, 2021
* Put License API can return HTTP 500

Put License now returns HTTP 400 if parsing user input fails, such as Base64 encoding or invalid signature.

Closes #74058
lockewritesdocs pushed a commit to lockewritesdocs/elasticsearch that referenced this issue Oct 28, 2021
* Put License API can return HTTP 500

Put License now returns HTTP 400 if parsing user input fails, such as Base64 encoding or invalid signature.

Closes elastic#74058
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>bug :Security/License License functionality for commercial features Team:Security Meta label for security team
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants