mirror of
https://github.com/golang/go
synced 2024-11-12 09:30:25 -07:00
crypto/tls: remove a forgotten note to future self
Now, this is embarrassing. While preparing CL 142818, I noticed a possible vulnerability in the existing code which I was rewriting. I took a note to go back and assess if it was indeed an issue, and in case start the security release process. The note unintentionally slipped into the commit. Fortunately, there was no vulnerability. What caught my eye was that I had fixed the calculation of the minimum encrypted payload length from roundUp(explicitIVLen+macSize+1, blockSize) to (using the same variable names) explicitIVLen + roundUp(macSize+1, blockSize) The explicit nonce sits outside of the encrypted payload, so it should not be part of the value rounded up to the CBC block size. You can see that for some values of the above, the old result could be lower than the correct value. An unexpectedly short payload might cause a panic during decryption (a DoS vulnerability) or even more serious issues due to the constant time code that follows it (see for example Yet Another Padding Oracle in OpenSSL CBC Ciphersuites [1]). In practice, explicitIVLen is either zero or equal to blockSize, so it does not change the amount of rounding up necessary and the two formulations happen to be identical. Nothing to see here. It looked more suspicious than it is in part due to the fact that the explicitIVLen definition moved farther into hc.explicitNonceLen() and changed name from IV (which suggests a block length) to nonce (which doesn't necessarily). But anyway it was never meant to surface or be noted, except it slipped, so here we are for a boring explanation. [1] https://blog.cloudflare.com/yet-another-padding-oracle-in-openssl-cbc-ciphersuites/ Change-Id: I365560dfe006513200fa877551ce7afec9115fdf Reviewed-on: https://go-review.googlesource.com/c/147637 Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
This commit is contained in:
parent
d258bac41c
commit
05a85f493c
@ -350,7 +350,7 @@ func (hc *halfConn) decrypt(record []byte) ([]byte, recordType, error) {
|
||||
}
|
||||
case cbcMode:
|
||||
blockSize := c.BlockSize()
|
||||
minPayload := explicitNonceLen + roundUp(hc.mac.Size()+1, blockSize) // TODO: vuln?
|
||||
minPayload := explicitNonceLen + roundUp(hc.mac.Size()+1, blockSize)
|
||||
if len(payload)%blockSize != 0 || len(payload) < minPayload {
|
||||
return nil, 0, alertBadRecordMAC
|
||||
}
|
||||
|
Loading…
Reference in New Issue
Block a user