Initial commit - #2
Conversation
This commit is not tested.
- fixed security exception due to apdu. - changed build.xml and removed build_common.xml
- fixed some bugs related to KMByteBlob and KMEnum. - Added getHardwareInfo
divegeek
left a comment
There was a problem hiding this comment.
Why all the binary files? And can we redistribute them all?
| public KMType get(short index) { | ||
| if (index >= length) { | ||
| throw new KMException(ISO7816.SW_WRONG_LENGTH); |
There was a problem hiding this comment.
It's a bad idea to new exceptions, isn't it? As I recall I preallocated them, the same way ISOException works. Does JavaCard now have reliable garbage collection?
| @Override | ||
| public void init() { | ||
| key = 0; | ||
| val = 1; // always 1. |
There was a problem hiding this comment.
Why bother allocating space? If getVal() just returns 1, you can omit this, I think.
| return repository.newByteBlob(); | ||
| } | ||
| // copy the blob |
There was a problem hiding this comment.
Seems like an init-by-reference ctor might be useful as well. For example, you can point the array at the APDU buffer, with appropriate start and length.
| import javacard.framework.ISO7816; | ||
| public class KMByteTag extends KMTag { |
There was a problem hiding this comment.
KMBytesTag or KMByteBlobTag. The name should indicate that this is a value containing more than one byte.
| } | ||
| // validate the tag key | ||
| private static boolean validateKey(short key) { |
There was a problem hiding this comment.
Factor the bulk of this into a helper method, since you repeat it?
| * | ||
| * @param keymasterState represents current applet state. | ||
| */ | ||
| public void setKeymasterState(byte keymasterState) { |
There was a problem hiding this comment.
I wonder if these getters and setters are actually better than just making the fields public.
| } else if (len == UINT32_LENGTH) { | ||
| inst = exp.instance(buffer, startOff, (short) 4); | ||
| incrementStartOff((short) 4); | ||
| } else if (len == UINT64_LENGTH) { |
There was a problem hiding this comment.
Might just want to throw in this case. Maybe even in the UINT32 case.
| while (index < payloadLength) { | ||
| short tagInd = 0; | ||
| short tagType = peekTagType(); | ||
| while (tagInd < allowedTags.length()) { |
There was a problem hiding this comment.
Maybe add a comment that this is a likely area for optimization. This will be executed enough that it may be a hot spot, and if allowedTags is sorted a binary search would not be much more code or complexity.
- Fixed transaction buffer overflow bug. - Added new AddRngEntropy command
Merging from Venkat's master
This is tested on jcardsim and simulator.