Test failed due to premature clearing of xA in ProveOVHdr - Re-open

[bkgoodman]

Could not re-open Issue #20

I just got another failure.

I did notice that you added the defer clear - but never removed the old Runtime Finalizer. It appears as though the finalizer cleared in the case I ran:

SIGN OVHPROOF with key RSA2048 Fingerprint: 1dc0424e52b976c298d2988ee973541b2ae1be46822a5b1222827f19ade94987 FINALIZER CLEARING!!!! FINALIZER CLEARING!!!! VERIFY OVHPROOF with key RSA2048 Fingerprint: 1dc0424e52b976c298d2988ee973541b2ae1be46822a5b1222827f19ade94987 *** KeyExchangeA Set to [0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0] defer clear(proveOVHdr.Payload.Val.KeyExchangeA) ... is still in sendHelloDevice, but...

    runtime.SetFinalizer(proof, func(proof *cose.Sign1Tag[ovhProof, []byte]) {
            fmt.Printf("*** FINALIZER CLEARING!!!!\n")
            clear(proof.Payload.Val.KeyExchangeA)
    })

... is still in proveOVHHdr.

[ben-krieger]

The finalizer shouldn't be removed, because it is used to ensure that whatever the transport implementation, the value is cleared in memory after use. Generally this is after the transport performs CBOR marshaling. For proveOVHdr, this is important in (s *TO2Server) proveOVHdr.

In the case of fdotest, the mock transport actually reuses the in-memory type. Before #21, this type would go out of scope at the end of a function (sendHelloDevice) that created a key exchange session, even though that key exchange session referenced the xA byte slice. This meant that whenever the finalizer ran, it would modify an internal field of the key exchange session - breaking crypto operations.

However, now that the key exchange session uses a copy (bytes.Clone) of the xA byte slice, it doesn't matter when the finalizer runs. There shouldn't be any way for it to run before bytes.Clone, since that function call references the proveOVHdr value.

Are you sure it's the exact same error occurring?

[ben-krieger]

By spec:

deferred functions are executed after any result parameters are set by that return statement but before the function returns to its caller

[bkgoodman]

Are you sure it's the exact same error occurring?

Yes, definitely. Those are fresh copy/pastes from my verry recent test. Note - this is actually running on a different machine than my previous ones.