1253107 - Legacy ASN.1 decoder should offer an option as to whether or not to zero internal memory

Status: NEW

(NSS :: Libraries, defect, P3)

Description

[Ryan Sleevi]

The legacy/classic ASN.1 decoder presently always calls PORT_FreeArena(cx->our_pool, PR_TRUE) - that is, with the "zero" flag set. This, in turn, calls PL_ClearArenaList to zero-out the entirity of cx->our_pool.

While this is ostensibly for security reasons (in particular, FIPS' requirement that any code which handles secret keys properly zero the secret keys before crossing any security boundary), this has the downside of significantly impairing efficient fuzzing. In particular, with a small fix for both this bug an Bug 1253105, a LibFuzzer-based fuzzer of NSS speeds up by 100x, due to significant amount of time being spent in memset() between fuzzing iterations.

We should expose an option that, when using the classic decoder, controls whether or not the internal workspace (cx->our_pool) should be zeroed.

This would minimally enable faster fuzzing.

An open question is whether we should expose an additional one-shot interface to the legacy, to enable this flag to be passed into the automatically-generated decoding context, or whether fuzzers for the streaming mode are sufficient to also cover the one-shot use case.

Comment 1

[Julien Pierre]

Does this necessitate a new function prototype ? This could potentially be dealt with an extra flag in the template.

Comment 2

[Ryan Sleevi]

I believe it does; I don't believe putting it in the template is good or right, because it means whatever templates set it cannot be cost-effectively fuzzed.

Exposing it only via a context option on the streaming decoder is sufficient for fuzzing, since the one-shot interface just composes over the streaming decoder.

Comment 3

[Robert Relyea]

We only need to zeroize when called from softoken/freebl (and then only when dealing with keys). A reasonable patch would be to make sure softoken/freebl calls the zeroize interface everywhere it uses the decoder. I don't think the streaming method is used in softoken/freebl, however.

Julien's idea of putting it in the template has the advantage that the very few places we need to free it would be well known. The disadvantages are 1) when we need freeing, we usually need it in all the templates, 2) It will probably wreak havoc on us (Red Hat) where we can't change softoken, so we can't add the flags on the template so we need to run with different defaults. (I'm not as worried about the fuzzed issue as I don't know if we can really get meaningful fuzzed output from keys which are inputted as encrypted data blobs anyway. To be effective we'd have to have a special fuzzer that doesn't send random data, but instead properly encrypts and macs random data. Once you add that overhead, the fact that NSS clears the result shouldn't be a big issue. More importantly we don't clear the data in the 90% case that we don't need to.

My preference would be a new SEC_ASN1 primitive call that automatically sets the 'don't zero' flag, and change our calls everywhere but softoken/freebl.

bob