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

Figure out implications of missing fields in JSON schema #3481

Closed
simitt opened this issue Mar 12, 2020 · 4 comments
Closed

Figure out implications of missing fields in JSON schema #3481

simitt opened this issue Mar 12, 2020 · 4 comments

Comments

@simitt
Copy link
Contributor

simitt commented Mar 12, 2020

So far Intake JSON schema defines all of the fields that will be processed (with the exception of custom, which is not indexed). Additional fields are explicitly allowed to keep future versions of the agents compatible with older server versions. In case additional fields are sent, they are not processed and neither indexed.
With the changes introduced in #3414 the Intake JSON schema for RUM only validates a part of the fields that are processed, as some fields were removed from the spec, but all Intake endpoints share the same model logic. Since the RUM endpoints are not protected, there is no protection from someone else sending up fields processed but not JSON schema validated.

We should investigate whether or not this could lead to runtime issues.

@jalvz
Copy link
Contributor

jalvz commented Mar 18, 2020

From what I understood today, this is about testing non-defined yet processed fields to ensure that if rum sends them up (even if it shouldn't), the server doesn't panic.
Correct?

@simitt
Copy link
Contributor Author

simitt commented Mar 18, 2020

Yes, only the RUM endpoint is not protected by design, so it also concerns malicious uploads. All fields that are not validated by the JSON schema but processed by the server logic should be checked for not causing any server panics etc. when e.g. sent with unexpected values.

@jalvz
Copy link
Contributor

jalvz commented Mar 19, 2020

Thanks for clarifying, I'll link this in the meta issue then.

@simitt
Copy link
Contributor Author

simitt commented May 14, 2020

Looking through the changes and sending data that are not part of the JSON spec anymore, I didn't find any situation where the server would panic. LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants