From 9fb97e878da8d908d467cb8c62408c93ed0107b9 Mon Sep 17 00:00:00 2001 From: Han-Wen Nienhuys Date: Mon, 11 May 2015 16:18:18 +0200 Subject: [PATCH] x/crypto/ssh: add padding oracle countermeasures for AES-CBC. This deprives an attacker of feedback for guesses against the packet length given by the connection dropping. Change-Id: I14939a82e5243a86d192bb18be93d45589227147 Reviewed-on: https://go-review.googlesource.com/9908 Reviewed-by: Adam Langley --- ssh/cipher.go | 66 ++++++++++++++++++++++++++++++++-------------- ssh/cipher_test.go | 63 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 109 insertions(+), 20 deletions(-) diff --git a/ssh/cipher.go b/ssh/cipher.go index b173183..3e06da0 100644 --- a/ssh/cipher.go +++ b/ssh/cipher.go @@ -14,6 +14,7 @@ import ( "fmt" "hash" "io" + "io/ioutil" ) const ( @@ -350,6 +351,7 @@ func (c *gcmCipher) readPacket(seqNum uint32, r io.Reader) ([]byte, error) { // cbcCipher implements aes128-cbc cipher defined in RFC 4253 section 6.1 type cbcCipher struct { mac hash.Hash + macSize uint32 decrypter cipher.BlockMode encrypter cipher.BlockMode @@ -357,6 +359,10 @@ type cbcCipher struct { seqNumBytes [4]byte packetData []byte macResult []byte + + // Amount of data we should still read to hide which + // verification error triggered. + oracleCamouflage uint32 } func newAESCBCCipher(iv, key, macKey []byte, algs directionAlgorithms) (packetCipher, error) { @@ -364,12 +370,18 @@ func newAESCBCCipher(iv, key, macKey []byte, algs directionAlgorithms) (packetCi if err != nil { return nil, err } - return &cbcCipher{ + + cbc := &cbcCipher{ mac: macModes[algs.MAC].new(macKey), decrypter: cipher.NewCBCDecrypter(c, iv), encrypter: cipher.NewCBCEncrypter(c, iv), packetData: make([]byte, 1024), - }, nil + } + if cbc.mac != nil { + cbc.macSize = uint32(cbc.mac.Size()) + } + + return cbc, nil } func maxUInt32(a, b int) uint32 { @@ -385,42 +397,58 @@ const ( cbcMinPaddingSize = 4 ) +// cbcError represents a verification error that may leak information. +type cbcError string + +func (e cbcError) Error() string { return string(e) } + func (c *cbcCipher) readPacket(seqNum uint32, r io.Reader) ([]byte, error) { + p, err := c.readPacketLeaky(seqNum, r) + if err != nil { + if _, ok := err.(cbcError); ok { + // Verification error: read a fixed amount of + // data, to make distinguishing between + // failing MAC and failing length check more + // difficult. + io.CopyN(ioutil.Discard, r, int64(c.oracleCamouflage)) + } + } + return p, err +} + +func (c *cbcCipher) readPacketLeaky(seqNum uint32, r io.Reader) ([]byte, error) { blockSize := c.decrypter.BlockSize() // Read the header, which will include some of the subsequent data in the // case of block ciphers - this is copied back to the payload later. // How many bytes of payload/padding will be read with this first read. - firstBlockLength := (prefixLen + blockSize - 1) / blockSize * blockSize + firstBlockLength := uint32((prefixLen + blockSize - 1) / blockSize * blockSize) firstBlock := c.packetData[:firstBlockLength] if _, err := io.ReadFull(r, firstBlock); err != nil { return nil, err } + c.oracleCamouflage = maxPacket + 4 + c.macSize - firstBlockLength + c.decrypter.CryptBlocks(firstBlock, firstBlock) length := binary.BigEndian.Uint32(firstBlock[:4]) if length > maxPacket { - return nil, errors.New("ssh: packet too large") + return nil, cbcError("ssh: packet too large") } if length+4 < maxUInt32(cbcMinPacketSize, blockSize) { // The minimum size of a packet is 16 (or the cipher block size, whichever // is larger) bytes. - return nil, errors.New("ssh: packet too small") + return nil, cbcError("ssh: packet too small") } // The length of the packet (including the length field but not the MAC) must // be a multiple of the block size or 8, whichever is larger. if (length+4)%maxUInt32(cbcMinPacketSizeMultiple, blockSize) != 0 { - return nil, errors.New("ssh: invalid packet length multiple") + return nil, cbcError("ssh: invalid packet length multiple") } paddingLength := uint32(firstBlock[4]) if paddingLength < cbcMinPaddingSize || length <= paddingLength+1 { - return nil, errors.New("ssh: invalid packet length") - } - - var macSize uint32 - if c.mac != nil { - macSize = uint32(c.mac.Size()) + return nil, cbcError("ssh: invalid packet length") } // Positions within the c.packetData buffer: @@ -428,7 +456,7 @@ func (c *cbcCipher) readPacket(seqNum uint32, r io.Reader) ([]byte, error) { paddingStart := macStart - paddingLength // Entire packet size, starting before length, ending at end of mac. - entirePacketSize := macStart + macSize + entirePacketSize := macStart + c.macSize // Ensure c.packetData is large enough for the entire packet data. if uint32(cap(c.packetData)) < entirePacketSize { @@ -440,8 +468,10 @@ func (c *cbcCipher) readPacket(seqNum uint32, r io.Reader) ([]byte, error) { c.packetData = c.packetData[:entirePacketSize] } - if _, err := io.ReadFull(r, c.packetData[firstBlockLength:]); err != nil { + if n, err := io.ReadFull(r, c.packetData[firstBlockLength:]); err != nil { return nil, err + } else { + c.oracleCamouflage -= uint32(n) } remainingCrypted := c.packetData[firstBlockLength:macStart] @@ -455,7 +485,7 @@ func (c *cbcCipher) readPacket(seqNum uint32, r io.Reader) ([]byte, error) { c.mac.Write(c.packetData[:macStart]) c.macResult = c.mac.Sum(c.macResult[:0]) if subtle.ConstantTimeCompare(c.macResult, mac) != 1 { - return nil, errors.New("ssh: MAC failure") + return nil, cbcError("ssh: MAC failure") } } @@ -474,13 +504,9 @@ func (c *cbcCipher) writePacket(seqNum uint32, w io.Writer, rand io.Reader, pack length := encLength - 4 paddingLength := int(length) - (1 + len(packet)) - var macSize uint32 - if c.mac != nil { - macSize = uint32(c.mac.Size()) - } // Overall buffer contains: header, payload, padding, mac. // Space for the MAC is reserved in the capacity but not the slice length. - bufferSize := encLength + macSize + bufferSize := encLength + c.macSize if uint32(cap(c.packetData)) < bufferSize { c.packetData = make([]byte, encLength, bufferSize) } else { diff --git a/ssh/cipher_test.go b/ssh/cipher_test.go index 2fb75d0..d67a93a 100644 --- a/ssh/cipher_test.go +++ b/ssh/cipher_test.go @@ -62,3 +62,66 @@ func TestPacketCiphers(t *testing.T) { } } } + +func TestCBCOracleCounterMeasure(t *testing.T) { + cipherModes[aes128cbcID] = &streamCipherMode{16, aes.BlockSize, 0, nil} + defer delete(cipherModes, aes128cbcID) + + kr := &kexResult{Hash: crypto.SHA1} + algs := directionAlgorithms{ + Cipher: aes128cbcID, + MAC: "hmac-sha1", + Compression: "none", + } + client, err := newPacketCipher(clientKeys, algs, kr) + if err != nil { + t.Fatalf("newPacketCipher(client): %v", err) + } + + want := "bla bla" + input := []byte(want) + buf := &bytes.Buffer{} + if err := client.writePacket(0, buf, rand.Reader, input); err != nil { + t.Errorf("writePacket: %v", err) + } + + packetSize := buf.Len() + buf.Write(make([]byte, 2*maxPacket)) + + // We corrupt each byte, but this usually will only test the + // 'packet too large' or 'MAC failure' cases. + lastRead := -1 + for i := 0; i < packetSize; i++ { + server, err := newPacketCipher(clientKeys, algs, kr) + if err != nil { + t.Fatalf("newPacketCipher(client): %v", err) + } + + fresh := &bytes.Buffer{} + fresh.Write(buf.Bytes()) + fresh.Bytes()[i] ^= 0x01 + + before := fresh.Len() + _, err = server.readPacket(0, fresh) + if err == nil { + t.Errorf("corrupt byte %d: readPacket succeeded ", i) + continue + } + if _, ok := err.(cbcError); !ok { + t.Errorf("corrupt byte %d: got %v (%T), want cbcError", i, err, err) + continue + } + + after := fresh.Len() + bytesRead := before - after + if bytesRead < maxPacket { + t.Errorf("corrupt byte %d: read %d bytes, want more than %d", i, bytesRead, maxPacket) + continue + } + + if i > 0 && bytesRead != lastRead { + t.Errorf("corrupt byte %d: want %d, got %d bytes read", bytesRead, lastRead) + } + lastRead = bytesRead + } +}