Skip to content

Commit

Permalink
process xor/rol optimised and fully implemented (thanks @webbnh)
Browse files Browse the repository at this point in the history
  • Loading branch information
arekbulski committed Apr 18, 2018
1 parent 14b9bd8 commit 6fccdb8
Show file tree
Hide file tree
Showing 2 changed files with 89 additions and 15 deletions.
91 changes: 79 additions & 12 deletions kaitai/kaitaistream.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -432,41 +432,108 @@ std::string kaitai::kstream::bytes_terminate(std::string src, char term, bool in
// ========================================================================

std::string kaitai::kstream::process_xor_one(std::string data, uint8_t key) {
if (key == 0)
return data;

size_t len = data.length();
std::string result(len, ' ');

for (size_t i = 0; i < len; i++)
for (size_t i = 0; i < len; i++) {
result[i] = data[i] ^ key;
}

return result;
}

std::string kaitai::kstream::process_xor_many(std::string data, std::string key) {
size_t len = data.length();
size_t kl = key.length();
size_t keylen = key.length();
if (keylen == 1)
return process_xor_one(data, key[0]);

std::string result(len, ' ');

size_t ki = 0;
size_t k = 0;
for (size_t i = 0; i < len; i++) {
result[i] = data[i] ^ key[ki];
ki++;
if (ki >= kl)
ki = 0;
result[i] = data[i] ^ key[k];
k++;
if (k == keylen)
k = 0;
}

return result;
}

std::string kaitai::kstream::process_rotate_left(std::string data, int amount) {
uint8_t precomputedSingleRotations[8][256];

// NOTE: static block of code, https://stackoverflow.com/a/34321324/2375119
computeSingleRotations {
for (int amount = 1; amount < 8; amount++) {
int anti_amount = 8 - amount;
for (uint8_t i = 0; i < 256; i++) {
precomputedSingleRotations[amount][i] = (uint8_t)((i << amount) | (i >> anti_amount));
}
}
}

std::string kaitai::kstream::process_rotate_left(std::string data, int amount, int groupSize = 1) {
if (groupSize < 1)
throw std::runtime_error("process_rotate_left: groupSize must be at least 1");

amount = mod(amount, groupSize * 8);
if (amount == 0)
return data;

int amount_bytes = amount / 8;
size_t len = data.length();
std::string result(len, ' ');

for (size_t i = 0; i < len; i++) {
uint8_t bits = data[i];
result[i] = (bits << amount) | (bits >> (8 - amount));
if (groupSize == 1) {
uint8_t *translate = &precomputedSingleRotations[amount][0];

for (size_t i = 0; i < len; i++) {
result[i] = translate[data[i]];
}

return result;
}

return result;
if (len % groupSize != 0)
throw std::runtime_error("process_rotate_left: data length must be a multiple of group size");

if (amount % 8 == 0) {
size_t indices[groupSize];
for (size_t i = 0; i < groupSize; i++) {
indices[i] = (size_t)((i + amount_bytes) % groupSize);
}

for (size_t i = 0; i < len; i += groupSize) {
for (size_t k = 0; k < groupSize; k++) {
result[i+k] = data[i + indices[k]];
}
}

return result;
}

{
int amount1 = amount % 8;
int amount2 = 8 - amount1;
size_t indices1[groupSize];
size_t indices2[groupSize];
for (size_t i = 0; i < groupSize; i++) {
indices1[i] = (size_t)((i + amount_bytes) % groupSize);
indices2[i] = (size_t)((i + 1 + amount_bytes) % groupSize);
}

for (size_t i = 0; i < len; i += groupSize) {
for (size_t k = 0; k < groupSize; k++) {
result[i+k] = (uint8_t)((data[i + indices1[k]] << amount1) | (data[i + indices2[k]] >> amount2));
}
}

return result;
}
}

#ifdef KS_ZLIB
Expand Down
13 changes: 10 additions & 3 deletions kaitai/kaitaistream.h
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,8 @@ class kstream {
/**
* Performs a XOR processing with given data, XORing every byte of input with a single
* given value.
* WARNING: May return same byte array if key is zero.
*
* @param data data to process
* @param key value to XOR with
* @return processed data
Expand All @@ -182,6 +184,8 @@ class kstream {
* Performs a XOR processing with given data, XORing every byte of input with a key
* array, repeating key array many times, if necessary (i.e. if data array is longer
* than key array).
* WARNING: May return same byte array if key is zero.
*
* @param data data to process
* @param key array of bytes to XOR with
* @return processed data
Expand All @@ -190,13 +194,16 @@ class kstream {

/**
* Performs a circular left rotation shift for a given buffer by a given amount of bits,
* using groups of 1 bytes each time. Right circular rotation should be performed
* using this procedure with corrected amount.
* using groups of some bytes each time. Right rotation can be performed by using
* negative amount.
* WARNING: May return same byte array if amount is zero (modulo-wise).
*
* @param data source data to process
* @param amount number of bits to shift by
* @param groupSize number of bytes that make a group
* @return copy of source array with requested shift applied
*/
static std::string process_rotate_left(std::string data, int amount);
static std::string process_rotate_left(std::string data, int amount, int groupSize = 1);

#ifdef KS_ZLIB
/**
Expand Down

3 comments on commit 6fccdb8

@webbnh
Copy link

@webbnh webbnh commented on 6fccdb8 Apr 19, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe all the show-stoppers have been resolved.

Were it up to me, I would decline the the changes to process_xor_one() and process_xor_many(): most of them are unnecessary code churn, and the ones which are actually operable are (very slightly) slowing down the main cases in order to optimize degenerate cases (which, if the calling code actually cares, it can easily handle itself).

And, again, I don't think adding the "warnings" to the header actually helps anything.

But, I think we're done, here.

@arekbulski
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you meant the key == 0 and key == [0] then no, the compiler cannot detect those situations if those values come from a variable, not a constant.

@webbnh
Copy link

@webbnh webbnh commented on 6fccdb8 Apr 19, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What I meant was, if a developer has a dataset where the "key" will be zero (meaning that no actual processing is required) or where a "multi-byte key" might be only one byte long (meaning that his code should use 'xor_one instead of 'xor_many), then it should be his application which pays the performance penalty, if he doesn't take the necessary steps to avoid it. (And, there actually is no loss in the zero-key case -- only a failure to "gain" -- relative to a non-zero key; and, I think an analogous statement applies to the one-byte key case.) I think this is a more appropriate approach than placing a(n albeit small) burden on all other developers' usages, given that they would have no way to avoid it.

I think you are largely correct that the KS compiler cannot detect these situations (especially, it cannot do so without burdening the normal cases), but I think a developer can use KSY directives to detect the degenerate cases and take appropriate actions if the benefits are worth the complication to him.

Please sign in to comment.