Skip to content

Commit

Permalink
Fix handling of large signature base strings.
Browse files Browse the repository at this point in the history
Add tests that use very long query strings to ensure the code works
properly on very large buffers. These tests use 16K long query strings,
resulting in even larger buffers for generating signatures.

Based on these tests, fix the HMAC_SHA1 signing code to handle these
long buffers. The code that was breaking was pointless anyway -- fixed
sized buffers were used to construct combined buffers that were then
passed into the SHA1 function. Instead, we can just take the two
separate buffers and call Update() for each one, avoiding allocation of
buffers, copying of data, etc.

As a side effect, this got rid of most of the buffers used during
computation, got rid of any heap allocation, and reduced the size of
some other buffers. Now we only have 3 buffers, each a SHA1 block
size (64 bytes) in length. All are allocated directly as arrays in the
CHMAC_SHA1 object. Two are the buffers used in computing the HMAC
signature and one holds the SHA1-based key (either the data directly if
short enough or the SHA1 of the input data).

Fixes #12.
  • Loading branch information
ewencp committed Mar 30, 2014
1 parent ef55074 commit 502403e
Show file tree
Hide file tree
Showing 4 changed files with 92 additions and 44 deletions.
15 changes: 5 additions & 10 deletions src/HMAC_SHA1.cpp
Expand Up @@ -34,15 +34,13 @@ void CHMAC_SHA1::HMAC_SHA1(BYTE *text, int text_len, BYTE *key, int key_len, BYT
m_ipad[i] ^= SHA1_Key[i];
}

/* STEP 3 */
memcpy(AppendBuf1, m_ipad, sizeof(m_ipad));
memcpy(AppendBuf1 + sizeof(m_ipad), text, text_len);

/* STEP 4 */
CSHA1::Reset();
CSHA1::Update((UINT_8 *)AppendBuf1, sizeof(m_ipad) + text_len);
CSHA1::Update((UINT_8 *)m_ipad, sizeof(m_ipad));
CSHA1::Update((UINT_8 *)text, text_len);
CSHA1::Final();

char szReport[SHA1_DIGEST_LENGTH];
CSHA1::GetHash((UINT_8 *)szReport);

/* STEP 5 */
Expand All @@ -51,13 +49,10 @@ void CHMAC_SHA1::HMAC_SHA1(BYTE *text, int text_len, BYTE *key, int key_len, BYT
m_opad[j] ^= SHA1_Key[j];
}

/* STEP 6 */
memcpy(AppendBuf2, m_opad, sizeof(m_opad));
memcpy(AppendBuf2 + sizeof(m_opad), szReport, SHA1_DIGEST_LENGTH);

/*STEP 7 */
CSHA1::Reset();
CSHA1::Update((UINT_8 *)AppendBuf2, sizeof(m_opad) + SHA1_DIGEST_LENGTH);
CSHA1::Update((UINT_8 *)m_opad, sizeof(m_opad));
CSHA1::Update((UINT_8 *)szReport, SHA1_DIGEST_LENGTH);
CSHA1::Final();

CSHA1::GetHash((UINT_8 *)digest);
Expand Down
52 changes: 18 additions & 34 deletions src/HMAC_SHA1.h
Expand Up @@ -13,40 +13,24 @@ typedef unsigned char BYTE ;

class CHMAC_SHA1 : public CSHA1
{
private:
BYTE m_ipad[64];
BYTE m_opad[64];

char * szReport ;
char * SHA1_Key ;
char * AppendBuf1 ;
char * AppendBuf2 ;


public:

enum {
SHA1_DIGEST_LENGTH = 20,
SHA1_BLOCK_SIZE = 64,
HMAC_BUF_LEN = 4096
} ;

CHMAC_SHA1()
:szReport(new char[HMAC_BUF_LEN]),
SHA1_Key(new char[HMAC_BUF_LEN]),
AppendBuf1(new char[HMAC_BUF_LEN]),
AppendBuf2(new char[HMAC_BUF_LEN])
{}

~CHMAC_SHA1()
{
delete[] szReport ;
delete[] AppendBuf1 ;
delete[] AppendBuf2 ;
delete[] SHA1_Key ;
}

void HMAC_SHA1(BYTE *text, int text_len, BYTE *key, int key_len, BYTE *digest);
public:

enum {
SHA1_DIGEST_LENGTH = 20,
SHA1_BLOCK_SIZE = 64
} ;

private:
BYTE m_ipad[SHA1_BLOCK_SIZE];
BYTE m_opad[SHA1_BLOCK_SIZE];

// This holds one SHA1 block's worth of data, zero padded if necessary.
char SHA1_Key[SHA1_BLOCK_SIZE];

public:
CHMAC_SHA1() {}

void HMAC_SHA1(BYTE *text, int text_len, BYTE *key, int key_len, BYTE *digest);
};


Expand Down
66 changes: 66 additions & 0 deletions tests/long_request_test.h
@@ -0,0 +1,66 @@
#ifndef __LIBOAUTHCPP_LONG_REQUEST_TEST_H__
#define __LIBOAUTHCPP_LONG_REQUEST_TEST_H__

#include "testutil.h"
#include <liboauthcpp/liboauthcpp.h>

using namespace OAuth;

namespace OAuthTest {

/** Tests long requests -- longer than a normal request such that they can cause
* problems if buffers are not big enough, etc.
**/
class LongRequestTest {
public:
static void run() {
std::string consumer_key = "wwwwxxxxyyyyzzzz";
std::string consumer_secret = "zzzzyyyyxxxxwwww";
OAuth::Consumer consumer(consumer_key, consumer_secret);

std::string oauth_token = "aaaabbbbccccdddd";
std::string oauth_token_secret = "ddddccccbbbbaaaa";
OAuth::Token token(oauth_token, oauth_token_secret);

// This sets up the client class to generate reproducible results.
Client::initialize(100, 1390268986);
OAuth::Client oauth(&consumer, &token);

// Generate a very long resource by adding very long parameters
std::string resource_arg = "arg=" +
std::string(16384, 'x') // 16K long
;
std::string resource = "resource?" + resource_arg;

// Test all request types, simple, unreserved chars in resource name, no parameters
ASSERT_EQUAL(
oauth.getURLQueryString(OAuth::Http::Head, resource),
resource_arg + "&oauth_consumer_key=wwwwxxxxyyyyzzzz&oauth_nonce=139026898664&oauth_signature=YnNugcEr0E4TDgkzR4ZFMFoHEgU%3D&oauth_signature_method=HMAC-SHA1&oauth_timestamp=1390268986&oauth_token=aaaabbbbccccdddd&oauth_version=1.0",
"Validate long HEAD request signature"
);
ASSERT_EQUAL(
oauth.getURLQueryString(OAuth::Http::Get, resource),
resource_arg + "&oauth_consumer_key=wwwwxxxxyyyyzzzz&oauth_nonce=139026898664&oauth_signature=5weTspQ0eMH5dFMDdsrGZlNrfPk%3D&oauth_signature_method=HMAC-SHA1&oauth_timestamp=1390268986&oauth_token=aaaabbbbccccdddd&oauth_version=1.0",
"Validate long GET request signature"
);
ASSERT_EQUAL(
oauth.getURLQueryString(OAuth::Http::Post, resource),
resource_arg + "&oauth_consumer_key=wwwwxxxxyyyyzzzz&oauth_nonce=139026898664&oauth_signature=thJwo%2ByzdRtxwrBqDXRCo2a1mcY%3D&oauth_signature_method=HMAC-SHA1&oauth_timestamp=1390268986&oauth_token=aaaabbbbccccdddd&oauth_version=1.0",
"Validate long POST request signature"
);
ASSERT_EQUAL(
oauth.getURLQueryString(OAuth::Http::Delete, resource),
resource_arg + "&oauth_consumer_key=wwwwxxxxyyyyzzzz&oauth_nonce=139026898664&oauth_signature=ONjZansHtzGD57pZ9S65s0a6aXs%3D&oauth_signature_method=HMAC-SHA1&oauth_timestamp=1390268986&oauth_token=aaaabbbbccccdddd&oauth_version=1.0",
"Validate long DELETE request signature"
);
ASSERT_EQUAL(
oauth.getURLQueryString(OAuth::Http::Put, resource),
resource_arg + "&oauth_consumer_key=wwwwxxxxyyyyzzzz&oauth_nonce=139026898664&oauth_signature=6FFgNsTsCl8ABh9i93rRN1m3csE%3D&oauth_signature_method=HMAC-SHA1&oauth_timestamp=1390268986&oauth_token=aaaabbbbccccdddd&oauth_version=1.0",
"Validate long PUT request signature"
);
}
};

}

#endif
3 changes: 3 additions & 0 deletions tests/main.cpp
Expand Up @@ -3,13 +3,16 @@
#include "urlencode_test.h"
#include "parsekeyvaluepairs_test.h"
#include "request_test.h"
#include "request_test.h"
#include "long_request_test.h"

using namespace OAuthTest;

int main(int argc, char** argv) {
URLEncodeTest::run();
ParseKeyValuePairsTest::run();
RequestTest::run();
LongRequestTest::run();

return TestUtil::summary();
}

1 comment on commit 502403e

@jterrace
Copy link
Member

Choose a reason for hiding this comment

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

first

Please sign in to comment.