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

Handle malformed license signatures #37137

Merged
merged 5 commits into from
Jan 9, 2019

Conversation

jkakavas
Copy link
Member

@jkakavas jkakavas commented Jan 4, 2019

This commit adds a more user friendly error message when a license
signature is malformed/truncated in a way that it cannot be
meaningfully parsed

Resolves #35340

This commit adds a more user friendly error message when a license
signature is malformed/truncated in a way that it cannot be
meaningfully parsed
@jkakavas jkakavas added >enhancement v7.0.0 :Security/License License functionality for commercial features v6.7.0 labels Jan 4, 2019
@jkakavas jkakavas requested a review from jaymode January 4, 2019 10:58
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-security

byte[] signatureBytes = Base64.getDecoder().decode(builder.signature);
ByteBuffer byteBuffer = ByteBuffer.wrap(signatureBytes);
version = byteBuffer.getInt();
} catch (BufferUnderflowException | IllegalArgumentException e) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it intentional that you don't just catch Exception? I don't think there's any exception that could be thrown here that we wouldn't want to wrap with a better message.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, I did it on purpose. decode() throws an IllegalArgumentException and getInt() throws a BufferUnderflowException() . I think that wrap() can't throw an IndexOutOfBoundsException when called with a byte array param. Am I missing any ?

Copy link
Member

Choose a reason for hiding this comment

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

I think it is safe to catch Exception here. Any exception that we get here would pretty much indicate a malformed signature right?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeap, I'll adjust this

Copy link
Member

@jaymode jaymode left a comment

Choose a reason for hiding this comment

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

LGTM

@jkakavas jkakavas merged commit 9049263 into elastic:master Jan 9, 2019
@jkakavas jkakavas deleted the handle-malformed-lic-sig branch January 9, 2019 05:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>enhancement :Security/License License functionality for commercial features v6.7.0 v7.0.0-beta1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants