Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Not validating #3

Open
greanie opened this issue Sep 28, 2017 · 8 comments
Open

Not validating #3

greanie opened this issue Sep 28, 2017 · 8 comments

Comments

@greanie
Copy link

greanie commented Sep 28, 2017

Thank you for this great bit of code! I am having a problem validating, however.

I created a jwt on the command line using php. I validated it via the jwt.io debugger. I am reading it into my application then passing it into an instanteation of your QJsonWebToken class through ::fromTokenAndSecret(). The header and payload are correct but when I call isValid(), it returns false.

By stepping through, I noticed that the call for getHeaderQStr( JsonDocument::Compact ) made from getSignature() which is made by isValid() returns a header QString this already diffferent than the header that is in the original JWT.

I noticed that when the header is first read, the format is QJsonDocument::Indented and during the validation, the header is encoded with QJsonDocument::Compact. Changing that to ::Indented in both cases did not correct the issue.

Below is my JWT
eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzUxMiJ9.eyJpYXQiOiIyMDE3LTA5LTIyVDE5OjAzOjUyLTA0OjAwIiwiaXNzIjoiQWRkIEhhcHRpY3MiLCJleHAiOiIyMDE4LTA5LTIyVDE5OjAzOjUyLTA0OjAwIiwiYXVkIjoibWlrZS5ncmVlbmlzaEBhZGRoYXB0aWNzLmNvbSIsImRhdGEiOnsiY2lkIjoiMTMyMDEwODAwMCIsImxpY19pZCI6IjUiLCJtb2R1bGVzIjoxfX0.E42BptdM4nqmv-LDuMGIzsYnHjQRrvS0OScc4ctzDen8KAsfba9vQ2OVjveSDz0po6Ynm6lLjzkSnXnLNavH7Q

I am using base64 encoding

Any help you could provide with figuring out what I am doing wrong would be greatly appreciated.

@greanie
Copy link
Author

greanie commented Sep 28, 2017

It seems that QJsonDocument switches the key-value pairs when it decodes the string.

QJsonDocument::toJson( QJsonDocument::fromJson( QByteArray::fromBase64( QString("eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzUxMiJ9").toUtf8() ) ) );

results in the key-value pairs being switched. I expect the similar thing is happening with the payload, and so the signature calculated is different from the one in the certificate and validation fails.

What version of QT was this written for? I am compiling on OS X with QT v5.5.

@juangburgos
Copy link
Owner

Take a look at the QJsonWebToken::getSignature source code.

Specially the first two lines, you can see that internally, QJsonWebToken uses the QJsonDocument to store the header and payload (see here).

The problem might be that when the QJsonWebToken class gets the header and payload as string and converts them to a QJsonDocument from the Qt API. Internally, I believe that QJsonDocument does not respect the order of the json you provided, instead in sorts the Json attributes alphabetically. In the specific example you provided, take a look at how at how jwt.io parses your Json:

issue_01

Then take a look at how Qt parses the same Json:

issue_02

See the order difference in the payload? This might result in a different hash being computed. So validation the signature of both implementations will be different.

The implications of this is that for the most part QJsonWebToken can ONLY verify tokens created by QJsonWebToken.

I would have loved to make this class compatible with other implementations, but that would require me to bypass Qt's Json parser and API and create my own and basically:

issue_03

@greanie
Copy link
Author

greanie commented Sep 29, 2017

Love that video!

I understand that your goal was not to rewrite the JSON library for QT. There are lots of people complaining about that fact that the order is not maintained and you are right it is indeed alphabetical. However, the JSON spec does not require that order be maintained so it is not a bug.

I have managed to change the order of the keys such that they are ordered alphabetically when the server creates that JWT. The jwt is now:

eyJhbGciOiJIUzUxMiIsInR5cCI6IkpXVCJ9.eyJhdWQiOiJtaWtlLmdyZWVuaXNoQGFkZGhhcHRpY3MuY29tIiwiZGF0YSI6eyJjaWQiOiIxMDgwMTA4MDAwIiwibGljX2lkIjoiNSIsIm1vZHVsZXMiOjF9LCJleHAiOiIyMDE4LTA5LTI5VDE2OjE0OjQ0LTA0OjAwIiwiaWF0IjoiMjAxNy0wOS0yOVQxNjoxNDo0NC0wNDowMCIsImlzcyI6IkFkZCBIYXB0aWNzIn0.wiJ5D6zkk_K0JYizYQ-nAjlrEfZKyhIjOF8NcDLXnwWEGEzAjovcfUsLAcioVYBKhzVlOMydAPluTOfAD81H0Q

The jwt.io debugger says it is valid. However, the QJsonWebToken says it is not valid.

I have checked the base64 encoded compacted json header matches exactly the original header passed. For the payload, the calculated base64 encoded compacted json payload matches exactly but has an additional '=' on the end.

In isValid()
qDebug() << "All data: " << tempTokenObj.m_byteAllData;;
qDebug() << "original signature: " << m_byteSignature;
qDebug() << "calculated signature: " << tempTokenObj.m_byteSignature;

All data: "eyJhbGciOiJIUzUxMiIsInR5cCI6IkpXVCJ9.eyJhdWQiOiJtaWtlLmdyZWVuaXNoQGFkZGhhcHRpY3MuY29tIiwiZGF0YSI6eyJjaWQiOiIxMDgwMTA4MDAwIiwibGljX2lkIjoiNSIsIm1vZHVsZXMiOjF9LCJleHAiOiIyMDE4LTA5LTI5VDE2OjE0OjQ0LTA0OjAwIiwiaWF0IjoiMjAxNy0wOS0yOVQxNjoxNDo0NC0wNDowMCIsImlzcyI6IkFkZCBIYXB0aWNzIn0="
original signature: "\xC2"y\x0F\xAC\xE4\x90\xAD\tb,\xD8""Bp#\x96\xB1\x1F""d\xAC\xA1"3\x85\xF0\xD7\x03-y\xF0XA\x84\xCC\b\xE8\xBD\xC7\xD4\xB0\xB0\x1C\x8A\x85X\x04\xA8sVS\x8C\xC9\xD0\x0F\x96\xE4\xCE|\x00\xFC\xD4}\x10"
calculated signature: "\x84\fA%\xE1\td \x9A\x1F\xCDH\xA7\x83\xB1\x85g\xAC\xE6\x1B\xA9\x00,\x1C""c\xCF\x00\x83\xD6\xC4\x98~X\xF1}D\x9C\x9A\xEE-O\xDAT\x1E\i=2\xDA%T\x16""f\xC2\xA3M\x99\x1C\x17?\xA6\x15)R"

If I truncate the last byte of the array to remove the extra '=' as below, it still fails.

All data: "eyJhbGciOiJIUzUxMiIsInR5cCI6IkpXVCJ9.eyJhdWQiOiJtaWtlLmdyZWVuaXNoQGFkZGhhcHRpY3MuY29tIiwiZGF0YSI6eyJjaWQiOiIxMDgwMTA4MDAwIiwibGljX2lkIjoiNSIsIm1vZHVsZXMiOjF9LCJleHAiOiIyMDE4LTA5LTI5VDE2OjE0OjQ0LTA0OjAwIiwiaWF0IjoiMjAxNy0wOS0yOVQxNjoxNDo0NC0wNDowMCIsImlzcyI6IkFkZCBIYXB0aWNzIn0"
original signature: "\xC2"y\x0F\xAC\xE4\x90\xAD\tb,\xD8""Bp#\x96\xB1\x1F""d\xAC\xA1"3\x85\xF0\xD7\x03-y\xF0XA\x84\xCC\b\xE8\xBD\xC7\xD4\xB0\xB0\x1C\x8A\x85X\x04\xA8sVS\x8C\xC9\xD0\x0F\x96\xE4\xCE|\x00\xFC\xD4}\x10"
calculated signature: "\xBA\x05\x99""E\xDB?gr\xD3\x9BR\xC5~\xAEL\xA2\xF7[\xCDM\xA7\x13\x90\xAC\xEC\xB0\xDA\xBC\x04\xE9\xCB\xB4\xB7\xC9\xA2w\xFD\xCB\xE9\xCEIJ\xC2\xC0\xD3\xD3\x9B\x03""72Z\xF5\xF8""A\xC1\x9C.A\xDDx\xBF\t\xE7'"

I know you are following the standard provided by jwt.io and you are using standard QT functions. And your code obviously worked for you. So may be it is a code base version issue? Can you run my jwt through on your end and see if it works?

@greanie
Copy link
Author

greanie commented Oct 1, 2017

I have looked into the php implementation of the JWT library. Between the base64 encoding and the generation of the hash, there is a filter to make sure the base64-encoded string is url safe, as follows:

$segments[] = static::urlsafeB64Encode(static::jsonEncode($header));
$segments[] = static::urlsafeB64Encode(static::jsonEncode($payload));

public static function urlsafeB64Encode($input) {
    return str_replace('=', '', strtr(base64_encode($input), '+/', '-_'));
}

That would expain the stripping of the extra '=' on the end. Otherwise, the base64 encoded payloads are the same so that doesn't explain the difference in the hash. I will post the question on the qt forum to see if anyone can provide ideas on what may be the problem.

Although the default character set of HTML5 is utf8, I'm not sure that a utf8 string is by default url-safe. Interestingly, stripping the extra '=' seems to have no impact on QT's abililty to convert the string back to the correct json object.

@juangburgos
Copy link
Owner

juangburgos commented Oct 1, 2017

Hi,

If you use the jwtcreator example to create a JWT and then try to validate it with the jwtverifier example it will work for sure.

Having this two examples running which actually use the same code (QJsonWebToken class), you can easily debug the JWT validation process.

I believe the validation of the payload is failing for you because the QJsonDocument is formatting the JSON in a very different way that the PHP implementation (maybe is removing or adding spaces, tabs, etc.). Anyway, I think you could easily adapt the QJsonWebToken class to avoid using QJsonDocument at least for simple tests, and find out the root cause of the problem.

I don't know how are your C++ skills, but the QJsonWebToken class is very simple, really not rocket science. I think you could modify the QJsonWebToken::getSignature method somehow in the lines of:

QByteArray QJsonWebToken::getSignature()
{
	// get header in compact mode and base64 encoded
	QByteArray byteHeaderBase64  = QByteArray("PasteYourHeaderHereInBase64");
	// get payload in compact mode and base64 encoded
	QByteArray bytePayloadBase64 = QByteArray("PasteYourPayloadHereInBase64");
	// calculate signature based on chosen algorithm and secret
	m_byteAllData = byteHeaderBase64 + "." + bytePayloadBase64;

        // etc...

}

See if the calculated signature matches what you expect, if it does, then the problem indeed the use of QJsonDocument, if not, then you have to look somewhere else. Maybe the Base64 encoding, or the HS256 implementation. Hard to tell at this point.

Good luck.

@randomstuff
Copy link

randomstuff commented Mar 14, 2019

One problem is that you are parsing the protected header and payload into QJsonDocuments (base-64 decoding and . When you want to check the signature, you convert these QJSOnDocument back into JSON.

However there is not a single way to convert your data into JSON because of:

  • field order,
  • indentation
  • escaping of strings
  • (precision/truncation of floating point numbers)

The spec does not mandate that way the JSON should be encoded. What you really should do instead is keep the original string and validate against that.

Morover, if you check one example of your implementation:

eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJhdWQiOiJldmVyeWJvZHkiLCJleHAiOiIxNTUzMTY0MTk2IiwiaWF0IjoiMTU1MjU1OTM5NiIsImlzcyI6Imp1YW5nYnVyZ29zIiwic3ViIjoiaGV5IHRoZXJlIn0=.jp8RV+4JOqsDqu56R7oIaD/U45KYEeUpqDc+/LeO6wg=

Use are using base64 with padding:

  • you have padding =
  • you have +/ instead of -_.

Moreover, the JWT/JWS spec mandates base64 URL without padding so this implementation does not generate valid JWTs and cannot validate many JWTs for this reason.

@juangburgos
Copy link
Owner

Do you think any of the available QByteArray::Base64Options options could fix the encoding?

@randomstuff
Copy link

@juangburgos, Yes, the base64 encoding part should be fixeable with: Base64UrlEncoding|OmitTrailingEquals.

juangburgos pushed a commit that referenced this issue Nov 13, 2022
* add additional header and payload bytearray members to fix signiture verification problem

* base64 encoding is now Base64UrlEncoding as per standard

* added OmitTrailingEquals when generating signiture

* cmake and qt6 support

* build examples on condition

* fix minor bug and update examples

Co-authored-by: Sadeq <sadeq.albana@snono-systems.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants