Security Considerations

[nklomp]

This PR (WIP), will add a Security Considerations Appendix to the spec for v2.1, in which suggestions are being done on several JSONPath and schema related aspects.

The idea is to create this appendix for v2.1 so implementers start following best practices, and then to include some normative text in v3.0 as it would break existing implementations that rely on function extensions for JSONPath and/or regex support for schema's.

It also removes the JSONPath implementations section and updates the JSONPath links to the IETF texts

refs #439, #398

[brentzundel]

thank you @nklomp I look forward to reviewing once this PR is out of draft state

[nklomp]

Although still in draft, I would encourage people to read it, to see whether it is in line with their expectations and/or whether I missed something.

Then I will update some refs/links and cleanup some wording.

@tlodderstedt @Sakurann @danielfett @brentzundel @csuwildcat @kimdhamilton

[brentzundel]

I have read the draft PR and like the direction you have taken.

[csuwildcat]

Can folks here confirm whether they are using regexes anywhere else in their wallet or server code? I just want to understand why folks might be using regexes elsewhere that they define which they feel is acceptable.

On Fri, Aug 11, 2023, 7:14 PM Kristina @.***> wrote:

@.**** requested changes on this pull request.

Thank you for writing this up. I think the content itself is a good start, but the sections feel duplicative and were pretty hard to follow (at least for me). some restructuring and consolidating of the sections might be helpful. For example, I would focus on the danger of regex without breaking it down to both json path and json schema...

some nits...

• I would suggest not using terms "we", or "you", since it is not clear to whom they point to. (it might be assumed that we is the editors and you is the implementers, and if that's the case please make it explicit: "it is recommended that the implementers...")
• please be consistent if regular expressions is capitalized or not.

— Reply to this email directly, view it on GitHub https://github.com/decentralized-identity/presentation-exchange/pull/441#pullrequestreview-1574567376, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABAFSV2L36Y72WL6N6G5MDXU3DFNANCNFSM6AAAAAA22KTMZI . You are receiving this because you were mentioned.Message ID: <decentralized-identity/presentation-exchange/pull/441/review/1574567376@ github.com>

[nklomp]

Thank you for writing this up. I think the content itself is a good start, but the sections feel duplicative and were pretty hard to follow (at least for me). some restructuring and consolidating of the sections might be helpful. For example, I would focus on the danger of regex without breaking it down to both json path and json schema...

some nits...

• I would suggest not using terms "we", or "you", since it is not clear to whom they point to. (it might be assumed that we is the editors and you is the implementers, and if that's the case please make it explicit: "it is recommended that the implementers...")
• please be consistent if regular expressions is capitalized or not.

Thanks for the feedback. Indeed it can be made more concise. Has to do with me overhauling some of the sections a bit. Will have a look at those later this week.

[nklomp]

Can folks here confirm whether they are using regexes anywhere else in their wallet or server code? I just want to understand why folks might be using regexes elsewhere that they define which they feel is acceptable.

Obiously there is a difference between whether a regex comes in from an untrusted party via a payload or when you run a regex created yourself against a payload. Yes in both cases there is room for exploitation, but getting a regex via a payload certainly is something to be extra cautious.

The problem is mainly that a lot of developers are aware, but there are also issues, like for instance JSONPath using (static-)eval for functions/regexes. Developers might not be automatically aware of the issues with these.

IMO the real question to ask here is: Why do we need regex support in JSONPath and Filters?

IMO we don't need it for most cases. As a RP I know exactly what types of claims and/or credential(s) combinations I want to receive. I guess one example of where a regex makes sense, would be to limit the DID methods for instance.

[csuwildcat]

I'm inclined to think we could drop JSON Path altogether for JSON Pointer, but the JSON Schema filters do, imo, need that capability.

On Sat, Aug 12, 2023, 4:34 PM Niels Klomp @.***> wrote:

Can folks here confirm whether they are using regexes anywhere else in their wallet or server code? I just want to understand why folks might be using regexes elsewhere that they define which they feel is acceptable.

Obiously there is a difference between whether a regex comes in from an untrusted party via a payload or when you run a regex created yourself against a payload. Yes in both cases there is room for exploitation, but getting a regex via a payload certainly is something to be extra cautious.

The problem is mainly that a lot of developers are aware, but there are also issues, like for instance JSONPath using (static-)eval for functions/regexes. Developers might not be automatically aware of the issues with these.

IMO the real question to ask here is: Why do we need regex support in JSONPath and Filters?

IMO we don't need it for most cases. As a RP I know exactly what types of claims and/or credential(s) combinations I want to receive. I guess one example of where a regex makes sense, would be to limit the DID methods for instance.

— Reply to this email directly, view it on GitHub https://github.com/decentralized-identity/presentation-exchange/pull/441#issuecomment-1676107894, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABAFSRMADZU2L6V6UD4GRTXU7ZETANCNFSM6AAAAAA22KTMZI . You are receiving this because you were mentioned.Message ID: @.*** com>

[csuwildcat]

As far as parsing of ingested data when malicious resource consumption issues are a concern, the most definitive way to handle it is to tell people to run their lib in a separate process/Web Worker and simply terminate it after a maximum allowed duration. It's rather trivial to do in most envs, for example:

var worker = new Worker('some-lib.js'); setTimeout(() => { worker.terminate(); }, 3000); // 3 second maximum run time

This is a far better, far more complete solution/suggestion for any spec that features parsing of ingested data.

On Sat, Aug 12, 2023, 8:15 PM Daniel Buchner @.***> wrote:

I'm inclined to think we could drop JSON Path altogether for JSON Pointer, but the JSON Schema filters do, imo, need that capability.

On Sat, Aug 12, 2023, 4:34 PM Niels Klomp @.***> wrote:

Can folks here confirm whether they are using regexes anywhere else in their wallet or server code? I just want to understand why folks might be using regexes elsewhere that they define which they feel is acceptable.

Obiously there is a difference between whether a regex comes in from an untrusted party via a payload or when you run a regex created yourself against a payload. Yes in both cases there is room for exploitation, but getting a regex via a payload certainly is something to be extra cautious.

The problem is mainly that a lot of developers are aware, but there are also issues, like for instance JSONPath using (static-)eval for functions/regexes. Developers might not be automatically aware of the issues with these.

IMO the real question to ask here is: Why do we need regex support in JSONPath and Filters?

IMO we don't need it for most cases. As a RP I know exactly what types of claims and/or credential(s) combinations I want to receive. I guess one example of where a regex makes sense, would be to limit the DID methods for instance.

— Reply to this email directly, view it on GitHub https://github.com/decentralized-identity/presentation-exchange/pull/441#issuecomment-1676107894, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABAFSRMADZU2L6V6UD4GRTXU7ZETANCNFSM6AAAAAA22KTMZI . You are receiving this because you were mentioned.Message ID: @.*** .com>

[kimdhamilton]

Addressed by #469