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

New security considerations #69

Closed
wants to merge 3 commits into from
Closed

New security considerations #69

wants to merge 3 commits into from

Conversation

laurencelundblade
Copy link
Contributor

@laurencelundblade laurencelundblade commented Apr 16, 2019

I see no reason for us to be wishy-washy in any way on correctness and security for decoder input processing in this day and age and with a modern protocol like CBOR. It is not that hard to get this right.

This lines up with what I would expect to be the internal coding standard of any reasonable modern SW company.

@laurencelundblade
Copy link
Contributor Author

@laurencelundblade laurencelundblade commented Apr 16, 2019

Also, this is intended to replace all other text about security in the document, particularly the text in the strict mode section.

The decoder should also check for and reject invalid CBOR for each
data type or tagged type that is in use for a particular CBOR-based
protocol.

Copy link
Collaborator

@jimsch jimsch Apr 16, 2019

Choose a reason for hiding this comment

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

To me this says that I would need to check for invalid mime and nested types. If that is not what is meant then this needs to be clearer

appropriate resource management to mitigate these attacks. (Items for
which very large sizes are given can also attempt to exploit integer
overflow vulnerabilities.)

Copy link
Collaborator

@jimsch jimsch Apr 16, 2019

Choose a reason for hiding this comment

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

I think that this issue of large allocations should be called out specifically.

To facilitate
this, encoder and decoder implementations used in such contexts should
provide at least one strict mode of operation ({{strict-mode}}).

Copy link
Collaborator

@jimsch jimsch Apr 16, 2019

Choose a reason for hiding this comment

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

From a security point of view. This paragraph is something that is of importance. It is really necessary to make sure that duplicate keys can be prevented. Allowing duplicate tags is not necessarily verboten.

@laurencelundblade
Copy link
Contributor Author

@laurencelundblade laurencelundblade commented Apr 17, 2019

I've made updates that I hope should address most of Jim's comments.

It seems like this text might need a tweak after other changes are made to other parts of the document in related to invalid CBOR.

@cabo
Copy link
Collaborator

@cabo cabo commented Jun 18, 2019

I'm not sure that we should be deleting any of the existing text. The new text should be integrated with it, not replace it.

@laurencelundblade
Copy link
Contributor Author

@laurencelundblade laurencelundblade commented Jun 19, 2019

Comment on first existing paragraph -- It starts "A network-facing application can exhibit v...". This points out some good security characteristics intrinsic in the CBOR design, but nothing the implementor should do. It didn't seem necessary to me, but I'm fine with keeping it.

@laurencelundblade
Copy link
Contributor Author

@laurencelundblade laurencelundblade commented Jun 19, 2019

Comment on the second existing paragraph which starts "Resource exhaustion attacks might attempt to lure..." -- I think my new second paragraph addresses everything in the old paragraph. I avoided mention of stack and allocation because different languages work differently and it seems impractical to instruct the coder on these things.

@laurencelundblade
Copy link
Contributor Author

@laurencelundblade laurencelundblade commented Jun 19, 2019

Comment on the third existing. "Protocols that are used in a security context ... -- I think I have addressed everything in it in my 3rd and 4th paragraphs. I am also attempting to line up with the levels of error handling. Is there something in the 3rd paragraph that I haven't addressed?

cabo added a commit that referenced this issue Jul 2, 2019
paulehoffman added a commit that referenced this issue Jul 2, 2019
Merged #69 (security considerations) with existing text
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants