Protect Mifare Classic against sector lock

[GoogleCodeExporter]

What steps will reproduce the problem?
1. Create a block with random data
2. Write it on trailer block
3. Sector will be unusable

It is possible to add some verifications to mifare_classic_write() that prevent 
from writing trailer block with wrong datas.

Original issue reported on code.google.com by romu...@libnfc.org on 1 Jul 2010 at 12:57

[GoogleCodeExporter]

Original comment by romu...@libnfc.org on 18 Aug 2010 at 2:19

• Changed title: Protect Mifare Classic against sector lock
• Changed state: New

[GoogleCodeExporter]

I also think it is an important matter. I am developing now an application with 
pyscard and my main MFCSector object contains methods for not letting the 
attribute change for wrong data. Take a look:

class MFCSector(object):
    """ Mifare Classic sector represantion object class """
    def __init__(self):
        self.__sectors = []
        initsectorlayout()

    def __setattr__(self, name, value):
        """ Override __setattr__ for mutual changes in keys,ac and blocks """
        #super().__setattr__(self, name, value)
        if name == 'datablock3':
            if len(value) == 16 and issametype(name):
                if accesscond_validate(value[6:9]):
                    self.block3 = value
                    self.keya = self.block3[:6]
                    self.ac   = self.block3[6:9]
                    self.gpb  = self.block3[9:10]
                    self.keyb = self.block3[10:]
                else:
                    raise AttributeError('Incorrect AC bytes, wrong bits inversion.')
            else:
                raise AttributeError('Block length is 16, int list must be provided.')

        if name = 'keya':
            if len(value) == 6 and issametype(name):
                self.keya   = value
                self.block3 = self.keya+self.block3[6:]
            else:
                raise AttributeError('Key length is 6, int list must be provided.')

        if name = 'keyb':
            if len(value) == 6 and issametype(name):
                self.keya   = value
                self.block3 = self.block3[:10]+self.keyb
            else:
                raise AttributeError('Key length is 6, int list must be provided.')

        if name = 'ac'  :
            if len(value) == 3 and issametype(name):
                if accesscond_validate(value):
                    self.ac = value
                else:
                    raise AttributeError('Incorrect AC bytes, wrong bits inversion.')
            else:
                raise AttributeError('AC length is 3, int list must be provided.')

        if name = 'gpb' :
            if len(value) == 1 and issametype(name):
                self.gpb = value
            else:
                raise AttributeError('GPB length is 1, int list must be provided.')

    def accesscond_validate(self,ac):
        """ Access condition bytes validation
            bytes are splitted by right and left shift
            then the proper bits are cheched whether they are inverted
            with XOR operation
        """
        if  ac[0] >> 4 == (ac[2] & 0b00001111) ^ 0b1111 and \
            ac[1] >> 4 == (ac[0] & 0b00001111) ^ 0b1111 and \
            ac[2] >> 4 == (ac[1] & 0b00001111) ^ 0b1111:
            return True
        else:
            return False

I wish there was full featured python-libnfc :-) Maybe I will take care of it, 
when I have some spare time.

Original comment by mistrzipan@gmail.com on 12 Sep 2011 at 11:29

[GoogleCodeExporter]

My position on this point is that it is much more the program role to rverify 
what is being done.  Example: when you want to remove a file, it is the rm(1) 
command which will optionally request you to confirm that you actually want to 
remove the file.  Depending on the answer, it will or will not execute a system 
call to unlink(2).

If you have python bindings, you can rely on some abstraction to provide this 
in your application, but having to call a function to allow another to work 
without restrictions looks really odd to me.  Writing invalid data to a sector 
trailing block is a desired feature since it allows sector locking (« Making 
the best of Mifare Classic » gives axamples about using this AFAICR).

If you can provide new elements which may require us to reconsider this, please 
reoppen this issue.

Thanks!

Original comment by romain.t...@gmail.com on 29 Sep 2011 at 2:40

• Changed state: WontFix