google / jwt_verify_lib

Provide c++ library to verify JWT tokens
Apache License 2.0
41 stars 43 forks source link

Add RSASSA-PSS support #53

Closed ackerleytng closed 4 years ago

ackerleytng commented 4 years ago

Would appreciate any feedback on this work in progress! I'll be adding more test cases

googlebot commented 4 years ago

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

:memo: Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

ℹ️ Googlers: Go here for more info.

ackerleytng commented 4 years ago

@googlebot I signed it!

googlebot commented 4 years ago

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

qiwzhang commented 4 years ago

Thanks you for your contribution.

ackerleytng commented 4 years ago

I'm a little confused about variable naming on the google style guide, do let me know if there's anything I should change!

ackerleytng commented 4 years ago

I'd like to propose using the direct initializer syntax (without new). Specifically for this implementation, the handle to implemented_args never leaves the isImplemented function, so C++ is able to insert the destructor appropriately (just before the program exits).

I wrote a small program to illustrate this here and here's the output:

$ g++ ok.cc && valgrind --leak-check=full --show-leak-kinds=all ./a.out
==354120== Memcheck, a memory error detector
==354120== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==354120== Using Valgrind-3.16.0.GIT and LibVEX; rerun with -h for copyright info
==354120== Command: ./a.out
==354120==
Foobar()
11111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111
Foobar()
11111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111~Foobar()
==354120==
==354120== HEAP SUMMARY:
==354120==     in use at exit: 32 bytes in 1 blocks
==354120==   total heap usage: 3 allocs, 2 frees, 73,760 bytes allocated
==354120==
==354120== 32 bytes in 1 blocks are still reachable in loss record 1 of 1
==354120==    at 0x4839DEF: operator new(unsigned long) (vg_replace_malloc.c:342)
==354120==    by 0x1092A1: leak(char) (in /home/ackerleytng/scratch/a.out)
==354120==    by 0x109424: main (in /home/ackerleytng/scratch/a.out)
==354120==
==354120== LEAK SUMMARY:
==354120==    definitely lost: 0 bytes in 0 blocks
==354120==    indirectly lost: 0 bytes in 0 blocks
==354120==      possibly lost: 0 bytes in 0 blocks
==354120==    still reachable: 32 bytes in 1 blocks
==354120==         suppressed: 0 bytes in 0 blocks
==354120==
==354120== For lists of detected and suppressed errors, rerun with: -s
==354120== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)

The output shows that when leak() is called, the destructor ~Foobar() is never called, and valgrind reports a leak due to the call to new, whereas the destructor does its work correctly in noLeak(), not in every iteration of the loop, but at the end of the program.

qiwzhang commented 4 years ago

OK, thanks.

ackerleytng commented 4 years ago

Thanks! I suppose we can close #49 ?