crypto/des: panics on incorrect block size

[crawshaw]

If you give a []byte of invalid size to the des package to Decrypt, it panics.

http://play.golang.org/p/h4yOjQmm31

I.e.

package main

import "crypto/des"

func main() {
    block, err := des.NewTripleDESCipher(make([]byte, 24))
    if err != nil {
        panic(err)
    }

    buf := []byte{135, 29}
    block.Decrypt(buf, buf)
}

Produces:

panic: runtime error: index out of range

goroutine 1 [running]:
encoding/binary.bigEndian.Uint64(0xf840029000, 0x200000002, 0xf8400290f8, 0xf840029000,
0x406f7b, ...)
    go/src/pkg/encoding/binary/binary.go:105 +0x23
crypto/des.cryptBlock(0xf840000100, 0x1000000010, 0xf840029000, 0x200000002,
0xf840029000, ...)
    go/src/pkg/crypto/des/block.go:12 +0x50

Is this preferable to an error? If so, the documentation should probably mention it
somewhere.

[minux]

Comment 1:

this is definitely a programming error so a panic is appropriate here.
(normally user shouldn't use cryptographic primitives like des directly,
and if they do use, they should know the basics)
we should add the documentation to crypto/cipher.Block that invalid
src and dst byte slice will cause panic.

[minux]

Comment 2:

btw, per the Go 1 API contract, we can't eliminate the panic here, however i wonder
if we should check invalid size of src/dst so that we can give better panic message?
(this will make crypto. primitives slower, so i'm inclined to not add the check)

[rsc]

Comment 3:

I thought we changed the other crypto routines to detect this and panic
with a better error. Maybe check AES?

[minux]

Comment 4:

AES's NewCipher check for invalid key sizes (the same for DES), but Encrypt and Decrypt
don't.

[rsc]

Comment 5:

[The time for maybe has passed.]

Labels changed: removed go1.1maybe.

[bradfitz]

Comment 6:

Over to agl to decide.

Labels changed: removed priority-triage.

Owner changed to @agl.

Status changed to Accepted.

[agl]

Comment 7:

Adding a conditional in order to have a nicer panic message is possible, but this is a
low-level call and I'd rather not, so I'm just going to call this WorkingAsIntended.

Status changed to WorkingAsIntended.