crypto/cipher: deprecate NewOFB, NewCFBDecrypter and NewCFBEncrypter

[FiloSottile]

Both OFB and CFB are unauthenticated encryption modes, which should be used only in very particular circumstances. They are also way less popular than the most used unauthenticated encryption mode, CTR, and our implementations are less optimized and not covered by the Go+BoringCrypto mode.

OFB mode is essentially unused (four real Debian CodeSearch hits). CFB mode is almost unused (five real Debian CodeSearch hits, a few more using other code search tools)

The CFB implementation in crypto/cipher arguably misuses the Stream interface, too. Unlike CTR or OFB that turn a block cipher into a true stream cipher by producing a keystream that is then XOR'd with the plaintext or ciphertext, CFB operates on the actual plaintext or ciphertext input. This is visible in the Encrypter / Decrypter asymmetry, and makes it impossible to XOR some zeroes to then XOR the result with the plaintext/ciphertext, which would be a reasonable expectation to have of a Stream.

We'd not be adding these if we were deciding today, we are not going to take optimizations for them, and we want to generally steer users away from them and towards authenticated modes like GCM or at least towards the more popular, more optimized, and Go+BoringCrypto covered CTR.

/cc @golang/security @golang/proposal-review

[aclements]

This proposal has been added to the active column of the proposals project
and will now be reviewed at the weekly proposal review meetings.

[gopherbot]

Change https://go.dev/cl/621957 mentions this issue: crypto/cipher: add small CTR benchmark, remove CFB/OFB benchmarks

[aclements]

For the purposes of FIPS certification, is it enough to simply mark these as deprecated?

[FiloSottile]

For the purposes of FIPS certification, is it enough to simply mark these as deprecated?

These will sit outside the boundary of the FIPS module, so for the purposes of certification they don't exist, and we don't even need to deprecate them. Now felt like a good time to deprecate them to signal that we did not put them within the FIPS module boundary and did not get them validated, and to be able to make statements like "all non-deprecated functions that NIST considers approved and not legacy are validated".

[aclements]

@FiloSottile , can you write out exactly what the deprecation message would be for these, including steering people toward the right replacement? Thanks.

[FiloSottile]

// Deprecated: CFB mode is not authenticated, which generally enables active
// attacks to manipulate and recover the plaintext. It is recommended that
// applications use [AEAD] modes instead. The standard library implementation of
// CFB is also unoptimized and not validated as part of the FIPS 140-3 module.
// If an unauthenticated [Stream] mode is required, use [NewCTR] instead.

Same for OFB.

[aclements]

Based on the discussion above, this proposal seems like a likely accept.

The proposal is to deprecate the following functions from crypto/cipher: NewOFB, NewCFBDecrypter, and NewCFBEncrypter.

[gopherbot]

Change https://go.dev/cl/631019 mentions this issue: crypto/cipher: deprecate NewOFB, NewCFBDecrypter, and NewCFBEncrypter

[aclements]

No change in consensus, so accepted. 🎉
This issue now tracks the work of implementing the proposal.

The proposal is to deprecate the following functions from crypto/cipher: NewOFB, NewCFBDecrypter, and NewCFBEncrypter.

[ianlancetaylor]

Is there anything left to do here? As far as I can tell this is done.

[rolandshoemaker]

I don't think so, closing.