Fixed MFRC522 trailing sector overflow (#224)

If the passed key is longer than 6 bytes, it will corrupt the access
bits for the trailing sector.

Fixed the bit calculation, updates tests.

Changed signatures to accept 6-byte long arrays instead of slices.

Minor typo for HD44780

Signed-off-by: Eugene Dzhurinsky <jdevelop@gmail.com>

experimental/devices/hd44780/hd44780.go

@@ -38,7 +38,7 @@ type Dev struct {
 //	e - strobe pin
 func New(data []gpio.PinOut, rs, e gpio.PinOut) (*Dev, error) {
 	if len(data) != 4 {
-		return nil, fmt.Errorf("expected 4 data pins, passed %s", len(data))
+		return nil, fmt.Errorf("expected 4 data pins, passed %d", len(data))
 	}
 	dev := &Dev{
 		dataPins:  data,

experimental/devices/mfrc522/mfrc522.go

@@ -63,8 +63,12 @@ type BlocksAccess struct {
 	B3         SectorTrailerAccess
 }
 
+func (ba *BlocksAccess) String() string {
+	return fmt.Sprintf("B0: %d, B1: %d, B2: %d, B3: %d", ba.B0, ba.B1, ba.B2, ba.B3)
+}
+
 // DefaultKey provides the default bytes for card authentication for method B.
-var DefaultKey = []byte{0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF}
+var DefaultKey = [...]byte{0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF}
 
 // Dev is an handle to an MFRC522 RFID reader.
 type Dev struct {
@@ -432,7 +436,7 @@ func (r *Dev) ReadBlock(sector int, block int) ([]byte, error) {
 // 	block - the block within the sector to write into.
 // 	data - 16 bytes if data to write
 // 	key - the key used to authenticate the card - depends on the used auth method.
-func (r *Dev) WriteBlock(auth byte, sector int, block int, data [16]byte, key []byte) (err error) {
+func (r *Dev) WriteBlock(auth byte, sector int, block int, data [16]byte, key [6]byte) (err error) {
 	defer func() {
 		if err == nil {
 			err = r.StopCrypto()
@@ -469,7 +473,7 @@ func (r *Dev) ReadSectorTrail(sector int) ([]byte, error) {
 // 	keyB - the key used for AuthB authentication schemd.
 // 	access - the block access structure.
 // 	key - the current key used to authenticate the provided sector.
-func (r *Dev) WriteSectorTrail(auth byte, sector int, keyA [6]byte, keyB [6]byte, access *BlocksAccess, key []byte) (err error) {
+func (r *Dev) WriteSectorTrail(auth byte, sector int, keyA [6]byte, keyB [6]byte, access *BlocksAccess, key [6]byte) (err error) {
 	defer func() {
 		if err == nil {
 			err = r.StopCrypto()
@@ -488,12 +492,12 @@ func (r *Dev) WriteSectorTrail(auth byte, sector int, keyA [6]byte, keyB [6]byte
 		return
 	}
 
-	data := make([]byte, 16)
+	var data [16]byte
-	copy(data, keyA[:])
+	copy(data[:], keyA[:])
 	accessData := CalculateBlockAccess(access)
 	copy(data[6:], accessData[:4])
 	copy(data[10:], keyB[:])
-	return r.write(calcBlockAddress(sector&0xFF, 3), data)
+	return r.write(calcBlockAddress(sector&0xFF, 3), data[:])
 }
 
 // Auth authenticate the card fof the sector/block using the provided data.
@@ -503,7 +507,7 @@ func (r *Dev) WriteSectorTrail(auth byte, sector int, keyA [6]byte, keyB [6]byte
 // 	block - the block within sector to authenticate.
 // 	sectorKey - the key to be used for accessing the sector data.
 // 	serial - the serial of the card.
-func (r *Dev) Auth(mode byte, sector, block int, sectorKey []byte, serial []byte) (AuthStatus, error) {
+func (r *Dev) Auth(mode byte, sector, block int, sectorKey [6]byte, serial []byte) (AuthStatus, error) {
 	return r.auth(mode, calcBlockAddress(sector, block), sectorKey, serial)
 }
 
@@ -513,7 +517,7 @@ func (r *Dev) Auth(mode byte, sector, block int, sectorKey []byte, serial []byte
 // 	sector - the sector to authenticate on.
 // 	block - the block within sector to authenticate.
 // 	key - the key to be used for accessing the sector data.
-func (r *Dev) ReadCard(auth byte, sector int, block int, key []byte) (data []byte, err error) {
+func (r *Dev) ReadCard(auth byte, sector int, block int, key [6]byte) (data []byte, err error) {
 	defer func() {
 		if err == nil {
 			err = r.StopCrypto()
@@ -538,7 +542,7 @@ func (r *Dev) ReadCard(auth byte, sector int, block int, key []byte) (data []byt
 //
 // 	sector - the sector to authenticate on.
 // 	key - the key to be used for accessing the sector data.
-func (r *Dev) ReadAuth(auth byte, sector int, key []byte) (data []byte, err error) {
+func (r *Dev) ReadAuth(auth byte, sector int, key [6]byte) (data []byte, err error) {
 	defer func() {
 		if err == nil {
 			err = r.StopCrypto()
@@ -562,21 +566,21 @@ func (r *Dev) ReadAuth(auth byte, sector int, key []byte) (data []byte, err erro
 // CalculateBlockAccess calculates the block access.
 func CalculateBlockAccess(ba *BlocksAccess) []byte {
 	res := make([]byte, 4)
-	res[0] = (^ba.getBits(1) & 0x0F) | ((^ba.getBits(2) & 0x0F) << 4)
+	res[0] = ((^ba.getBits(2) & 0x0F) << 4) | (^ba.getBits(1) & 0x0F)
-	res[1] = (^ba.getBits(3) & 0x0F) | (ba.getBits(1) & 0x0F << 4)
+	res[1] = ((ba.getBits(1) & 0x0F) << 4) | (^ba.getBits(3) & 0x0F)
-	res[2] = (ba.getBits(2) & 0x0F) | (ba.getBits(3) & 0x0F << 4)
+	res[2] = ((ba.getBits(3) & 0x0F) << 4) | (ba.getBits(2) & 0x0F)
 	res[3] = res[0] ^ res[1] ^ res[2]
 	return res
 }
 
 // ParseBlockAccess parses the given byte array into the block access structure.
 func ParseBlockAccess(ad []byte) *BlocksAccess {
-	ba := new(BlocksAccess)
+	return &BlocksAccess{
-	ba.B0 = BlockAccess(ad[1]&0x10>>4 | ad[2]&0x01<<1 | ad[2]&0x10>>2)
+		B0: BlockAccess(((ad[1] & 0x10) >> 2) | ((ad[2] & 0x01) << 1) | ((ad[2] & 0x10) >> 5)),
-	ba.B1 = BlockAccess(ad[1]&0x20>>5 | ad[2]&0x02 | ad[2]&0x20>>3)
+		B1: BlockAccess(((ad[1] & 0x20) >> 3) | (ad[2] & 0x02) | ((ad[2] & 0x20) >> 5)),
-	ba.B2 = BlockAccess(ad[1]&0x40>>6 | ad[2]&0x04>>1 | ad[2]&0x40>>4)
+		B2: BlockAccess(((ad[1] & 0x40) >> 4) | ((ad[2] & 0x04) >> 1) | ((ad[2] & 0x40) >> 6)),
-	ba.B3 = SectorTrailerAccess(ad[1]&0x80>>7 | ad[2]&0x08>>2 | ad[2]&0x80>>5)
+		B3: SectorTrailerAccess(((ad[1] & 0x80) >> 5) | ((ad[2] & 0x08) >> 2) | ((ad[2] & 0x80) >> 7)),
-	return ba
+	}
 }
 
 func (r *Dev) String() string {
@@ -587,7 +591,7 @@ func (r *Dev) String() string {
 //		MFRC522 SPI Dev private/helper functions
 
 func (ba *BlocksAccess) getBits(bitNum uint) byte {
-	shift := bitNum - 1
+	shift := 3 - bitNum
 	bit := byte(1 << shift)
 	return (byte(ba.B0)&bit)>>shift | ((byte(ba.B1)&bit)>>shift)<<1 | ((byte(ba.B2)&bit)>>shift)<<2 | ((byte(ba.B3)&bit)>>shift)<<3
 }
@@ -651,11 +655,11 @@ func (r *Dev) write(blockAddr byte, data []byte) error {
 	return nil
 }
 
-func (r *Dev) auth(mode byte, blockAddress byte, sectorKey []byte, serial []byte) (AuthStatus, error) {
+func (r *Dev) auth(mode byte, blockAddress byte, sectorKey [6]byte, serial []byte) (AuthStatus, error) {
 	buffer := make([]byte, 2)
 	buffer[0] = mode
 	buffer[1] = blockAddress
-	buffer = append(buffer, sectorKey...)
+	buffer = append(buffer, sectorKey[:]...)
 	buffer = append(buffer, serial[:4]...)
 	_, _, err := r.CardWrite(commands.PCD_AUTHENT, buffer)
 	if err != nil {
@@ -749,6 +753,7 @@ var sequenceCommands = struct {
 
 var _ conn.Resource = &Dev{}
 var _ fmt.Stringer = &Dev{}
+var _ fmt.Stringer = &BlocksAccess{}
 
 func wrapf(format string, a ...interface{}) error {
 	return fmt.Errorf("mfrc522: "+format, a...)

experimental/devices/mfrc522/mfrc522_test.go

@@ -11,6 +11,26 @@ import (
 	"testing"
 )
 
+func fromBitString(t *testing.T, s string) (res byte) {
+	d, err := strconv.ParseUint(s, 2, 8)
+	if err != nil {
+		t.Fatal(err)
+	}
+	res = byte(d & 0xFF)
+	return
+}
+
+/*
+   C1 C2 C3
+3 : 1  0  0
+2 : 0  0  1
+1 : 1  0  1
+0 : 1  1  0
+
+1 1 1 0 0 1 0 0
+1 0 1 1 1 0 0 1
+0 1 1 0 0 0 0 1
+*/
 func TestBitCalc(t *testing.T) {
 
 	ba := BlocksAccess{
@@ -22,35 +42,57 @@ func TestBitCalc(t *testing.T) {
 
 	access := CalculateBlockAccess(&ba)
 
-	reader := func(s string) (res byte) {
+	if fromBitString(t, "1011") != ba.getBits(1) {
-		d, err := strconv.ParseUint(s, 2, 8)
+		t.Fatalf("1011 is not equal to %d", ba.getBits(1))
-		if err != nil {
-			t.Fatal(err)
-		}
-		res = byte(d & 0xFF)
-		return
-	}
-
-	if reader("0110") != ba.getBits(1) {
-		t.Fatalf("0110 is not equal to %d", ba.getBits(1))
 	}
 
 	expected := []byte{
-		reader("11101001"),
+		fromBitString(t, "11100100"),
-		reader("01100100"),
+		fromBitString(t, "10111001"),
-		reader("10110001"),
+		fromBitString(t, "01100001"),
 		0,
 	}
 
 	expected[3] = expected[0] ^ expected[1] ^ expected[2]
 
 	if !bytes.Equal(expected, access) {
-		t.Fatal("Access is incorrect")
+		t.Fatalf("Access is incorrect: %v != %v", expected, access)
 	}
 
 	parsedAccess := ParseBlockAccess(access)
 
 	if !reflect.DeepEqual(ba, *parsedAccess) {
-		t.Fatal("Parsed access mismatch")
+		t.Fatalf("Parsed access mismatch %s != %s", ba.String(), (*parsedAccess).String())
+	}
+}
+
+/*
+   C1 C2 C3
+3 : 0  0  1
+2 : 0  0  0
+1 : 0  0  0
+0 : 0  0  0
+
+
+1 1 1 1 1 1 1 1
+0 0 0 0 0 1 1 1
+1 0 0 0 0 0 0 0
+*/
+func TestByteArrayDecipher(t *testing.T) {
+	bitsData := [...]byte{
+		fromBitString(t, "11111111"),
+		fromBitString(t, "00000111"),
+		fromBitString(t, "10000000"),
+		fromBitString(t, "01111000"),
+	}
+	ba := BlocksAccess{
+		B0: AnyKeyRWID,
+		B1: AnyKeyRWID,
+		B2: AnyKeyRWID,
+		B3: KeyA_RN_WA_BITS_RA_WA_KeyB_RA_WA,
+	}
+	access := CalculateBlockAccess(&ba)
+	if !reflect.DeepEqual(bitsData[:], access) {
+		t.Fatalf("Wrong access calculation: %v != %v", bitsData, access)
 	}
 }