search

flux-framework / rfc

Flux RFC project
https://flux-framework.readthedocs.io/projects/flux-rfc/
7 stars 13 forks source link

rfc38: add rfc for flux-security key-value encoding #389

Closed garlick closed 1 year ago

garlick commented 1 year ago

Problem: the key-value format used by flux-security ostensibly for auditability is not documented.

Add an RFC.

garlick commented 1 year ago

No max key length or max overall length is currently defined. Key max would be trivial. Not sure about overall ...

garlick commented 1 year ago

A max value length would need to handle all possible serialized jobspecs, which contain a user's environment. So maybe 1MB? I think the environment is limited to something on the order of 128K so that would give some leeway. Perhaps we could just say the overall encoded length is limited to 1MB and then not set limits on key length or string value length?

Presumably preventing someone from running the IMP out of memory by passing it some bad input is one part of a defense in depth strategy?

garlick commented 1 year ago

Pushed with -DBL_MAX as a test vector.

@grondo any thoughts on limiting the size of various things?

grondo commented 1 year ago

No, but remember jobspec can now have (small) files embedded. Having a size limit seems reasonable but it would be annoying to users and admins to have jobs fail when they are started even after passing other validation (but it would be easy to place a limit and enforce that on the validator side?) Sorry I'm having trouble coming up with any coherent thought on this one...

dun commented 1 year ago

I think you're ok in having an overall length of 1MB or whatever for the total encoded length. This limit could even be specified in the imp config file or somesuch (so maybe this rfc isn't the place for it to be defined). You just need to protect against bad input exhausting resources.

garlick commented 1 year ago

I had forgotten about the files embedded in jobspec. 1MB may be on the small side. I guess the main thing is we don't want to be able to trigger a malloc failure. Heck a 1GB limit would do that. Could we set the limit at 1GB and ignore it in the rest of the system?

If we don't currently impose any limits at all on the size of generated jobspec, we might want to add something that could reject jobs at submit time to prevent abuse of the input file thing.

garlick commented 1 year ago

Ah I didn't see your comment @dun. Maybe the RFC should just say that the size SHALL be limited and not specify what the limit is.

garlick commented 1 year ago

Added:

An implementation defined limit SHALL be imposed on the maximum overall size of a serialized object to avoid resource exhaustion.