xmlenc: clear up the confusion about padding bytes and add tests for them

xmlenc/xmlenc.go

@@ -138,20 +138,22 @@ func decryptEncryptedData(key []byte, d *encryptedData) ([]byte, error) {
 	}
 
 	mode := cipher.NewCBCDecrypter(blockCipher, iv)
-	mode.CryptBlocks(ciphertext, ciphertext)
 
-	// I've noticed a trailing 0x01 byte in the plaintext
-	// which I cannot explain and which breaks things downstream.
-	// Lacking a better option, we'll strip it here. There are
-	// probably loads of better ways to handle this, not least of
-	// which is to figure out where that strange byte is coming
-	// from.
-	// TODO(ross): figure out where this comes from
-	if ciphertext[len(ciphertext)-1] == 0x1 {
-		ciphertext = ciphertext[:len(ciphertext)-1]
+	plaintext := make([]byte, len(ciphertext))
+	mode.CryptBlocks(plaintext, ciphertext)
+
+	// Remove any padding from the end of the message.
+	//
+	// Per http://www.w3.org/TR/2002/REC-xmlenc-core-20021210/Overview.html
+	// On decryption, just take the last byte and, after sanity checking
+	// it, strip that many bytes from the end of the decrypted cipher text.
+	paddingByteCount := int(plaintext[len(plaintext)-1])
+	if paddingByteCount < 1 || paddingByteCount > len(plaintext) || paddingByteCount > blockCipher.BlockSize() {
+		return nil, fmt.Errorf("invalid padding")
 	}
+	plaintext = plaintext[:len(plaintext)-paddingByteCount]
 
-	return ciphertext, nil
+	return plaintext, nil
 }
 
 // decryptEncryptedKey returns the plaintext version of the EncryptedKey which is