It is nice to see a loyalty card applet being open sourced as most loyalty card applets are usually designed behind close doors and it require much more effort to reverse engineer and then crack the protocol for security auditing purposes.
As per usual, I will give my commentaries on the security aspect and enhancements (security and non-security) here.
The first taboo of any smart card or device is clear channel without encryption and MAC-ing or to leave security as opportunistic. The card initialization is weak on security because it does not use an explicit secure channel and to make matters worse, this is the most critical point for a card where the secret PIN of the user and any other keys are sent over to the card before locking the card (switching to post-issuance).
Note that such loyalty cards are expected to operate over NFC/Contactless and one could imagine the impact of transactions over insecure contactless channel. I cannot emphasize more strongly on the absolute need of security especially for contactless applications.
An integrated board for sniffing contactless can be easily bought off the counter (
https://hydrabus.com/buy-online/) and some of them are simply very cheap to purchase. With more contactless payments and loyalty programs becoming more widespread but the security of these cards are still close to non-existent, proper use of security needs to be the key to prevent more fraud and interruption of businesses.
At the very least, SCP02 with APDU Command encryption MUST BE used in this phase otherwise a proprietary secure channel needs to be implemented. I would recommend that the card generate a RSA keypair and sends it's Public Key to the personalization station, the personalization station uses it's Private Key to sign and generate a certificate for the card and then establish a secure channel to transfer whatever initialization details needed including the certificate.
Updating and verification of PINs are not done over a secure channel and this is poor security practice. Please use some form of secure channel or explicitly force SCP02 or 03.
The DES crypto uses a fixed all 0x00 bytes IV and this is a poor crypto practice as it weakens the CBC mode with a predictable IV. The use of IV was designed in mind to add a random factor to make the CBD mode more robust but consistently using all 0x00 bytes simply goes against the design principle of proper crypto usage.
Balance and points enquiry do not need to be encrypted but at least a MAC using the 2DES key or some Signature via RSA would be very handy. This prevents fraud of the point system. You can imagine a MiTM over NFC could be used to send the wrong balance points and cause the POS machines to react in an expected manner.
Credit function is insecure without crypto and MAC. If that is the case, the card owner can buy a card reader and simply credit a bogus amount without restriction. I noticed an "isExternalMark" being commented out and I assume that is the indicator whether authentication has been done. This means no authentication is required to credit any amount of credit as long as it does not exceed the limit of 0x7FFF. Also, the Credit method is too long winded where you could have taken the 2 bytes of credit data from APDU and then use Util.makeShort() to convert the binary values for credit amount request into short, convert the credit limit of 0x7FFF into short as well and check whether balance + proposed credit amount is >= credit limit all in short form. It saves you many many lines of codes. Then to store back into balance_new, you simply reverse the process of short values back to bytes. In fact, it is better to not store the balance or credit amount in binary form but in short form since you can do math much easier.
Good to know the Purchase method checks for PIN validation
. At least something went right there.
Now the pain of storing credit amount into some bytes come in the Purchase method where you have some Integral (is it Integer ???) calculation. Why not store in short since the max credit limit is short, then a purchase cannot exit a short type amount and thus it will always be a 2 byte short. Why bother use Integer when using short, you can do math more efficiently ? The codes look awful there. Please switch everything for credit and balance amount to short types and refrain from using Integer or bytes when handling numbers. It hurts a lot to see ugly codes when the obvious method is a simple short.
GetChannel attempts to use a random nonce to prevent replay attack which is a good effort and a right direction but it has a few flaws. Firstly, do not seed the RNG with a single default value. What should have been done is for the card initialization to receive a unique seed. Random nonces are very important to make prediction attacks harder and a fix seed is a very very bad idea. Please use card initialization to provision a unique seed instead of a hard-coded seed. Also the code here can be improved:
Code: Select all
myRandomS.generateData(buf, (short)ISO7816.OFFSET_CDATA, responseLength);
Util.arrayCopy(buf, (short)ISO7816.OFFSET_CDATA, input, (short)0, responseLength);
with the following code:
Code: Select all
myRandomS.generateData(input, (short)ISO7816.OFFSET_CDATA, responseLength);
This simply avoids the need to do a arrayCopy since you directly generateData into your target input array.
Since this is a random nonce generation to prevent replay attacks, please use a dedicated nonce byte array with memory type set to transient so that you can consistently refer and update the nonce. To make matters worse, the nonce is never updated after use. The GetChannel broadcast the nonce without protection and thus can be manipulated and sniffed. That means an attacker can pick up the broadcast of the nonce over some form of MiTM (either via a hardware sniffer that I showed previously) and then find a way to know the DES key. Note that the DES key is highly susceptible to Meet-in-the-Middle attack (
https://en.wikipedia.org/wiki/Meet-in-the-middle_attack) and one reason 2DES is never used is it can be broken with such an attack. To make matters worst, the nonce is transmitted in plain and the attacker knows that the nonce will be encrypted inside the 2DES cryptogram and thus the attacker can be certain that Meet-in-the-Middle attack can be used to break the DES cipher and the fact that the seed is static and the key is also static, that means the attacker can take all the time the attacker needs to mount a Meet-in-the-Middle and recover the key (even if the key were not intercepted during card initialization phase).
This simply breaks the entire underpinning security of having crypto in the first place with so many tiny mistakes mounting to a single point where the attacker can recover the key if needed.
A quote from the Wikipedia article should suffice in the severity of the attack:
Double DES can be broken with 2^57 encryption and decryption operations.
Note that 2^57 operations is actually very low cost and parallel computing using multiple Raspberry Pis networked together would make it easier and the DES cipher is also susceptible to lower security margins than it claims due to the fact that techniques to make DES operation faster exists thus allowing software acceleration as need be.
InternalAuth is never used and should be removed from the codes.
Please fix all the security flaws as I mentioned and if you need help, I am available here for you to seek advise to correct the security deficiencies.
------------
I would like to propose a Password-based Authenticated Key Exchange (PAKE) setup which will be very helpful especially during card initialization where keys and PINs are transferred to the card. PAKE is a Key Exchange over a password and so the password is used as a symmetric key. The card would need to hard code a password and two byte array of magic bytes. For security reasons, once the card initialization session successfully completes, the PAKE password is removed (overwrite with 0x00).
0.) The card is hard-coded with the password and two sets of magic bytes. The magic bytes should be at least 8 bytes in length for each set of magic bytes. The magic bytes are used to deterministically derive the encryption and MAC keys for the PAKE session.
1.) To derive the encryption key for the PAKE session, the first set of magic bytes are hashed with the password and to derive the MAC key for the session, the second set of magic bytes are hashed with the password. The hash should be SHA-256 to derive 256-bit secret key material. If the key size needs to be smaller (i.e. 128 bit key), simply extract as much bytes as you need (just remove 128 bits).
2.) Terminal generates a Terminal 20 byte nonce and the card also generates it's own random Card 20 byte nonce.
3.) Using the PAKE encryption key, encrypt the nonces and send to the terminal/card and use the MAC key to MAC the ciphered payload.
4.) The first 8 bytes of the Terminal's nonce and the first 8 bytes of the Card's nonce are combined as a 128-bit key and the next 8 bytes of the Terminal's nonce and the next 8 bytes of the Card's nonce are combined as the 128-bit MAC key. The final 4 bytes of the Terminal's nonce and the final 4 bytes of the Card's nonce are combined as the 8 byte session counter.
5.) Once the actual session keys and counters have been derived, the card and terminal may communicate securely and the counters must be incremented after use.
Once the card initializes, zeroize all the PAKE parameters to prevent misuse.
