Re: Review Request 66621: Add support for alg RS256 to JWT library.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66621/ --- (Updated avr. 19, 2018, 11:10 après-midi) Review request for mesos, Alexander Rojas and Till Toenshoff. Changes --- Fixed all issues reported by Alexander Bugs: MESOS-8788 https://issues.apache.org/jira/browse/MESOS-8788 Repository: mesos Description --- Add support for alg RS256 to JWT library. Diffs (updated) - 3rdparty/libprocess/include/process/jwt.hpp 768cbf6fa91537ff9f45f236f4033097c5cea959 3rdparty/libprocess/include/process/ssl/utilities.hpp b7cc31c33fd35c93754407f8b350eeb993177f1d 3rdparty/libprocess/src/jwt.cpp 921031e6fe3ced5a6be6bc96190fae6d8282ae26 3rdparty/libprocess/src/ssl/utilities.cpp 4d3727daf53ec62a19255da5a9804d342e770ec2 3rdparty/libprocess/src/tests/jwt_keys.hpp PRE-CREATION 3rdparty/libprocess/src/tests/jwt_tests.cpp eb36a9aed3b11208c7cdc6f20b5347f46821a207 Diff: https://reviews.apache.org/r/66621/diff/3/ Changes: https://reviews.apache.org/r/66621/diff/2-3/ Testing --- make check I added the same tests than the ones for HS256 (i.e., validation in following cases: bad header, bad payload, unknown alg, unsupported alg, valid token etc.. and creation of a valid token). I also added a test to verify that the validation of a RS256 token fails when using the wrong public key. Thanks, Clément Michaud
Re: Review Request 66621: Add support for alg RS256 to JWT library.
> On avr. 16, 2018, 9:52 après-midi, Mesos Reviewbot Windows wrote:
> > FAIL: Failed to apply the current review.
> >
> > Failed command: `python.exe .\support\apply-reviews.py -n -r 66621`
> >
> > All the build artifacts available at:
> > http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/66621
> >
> > Relevant logs:
> >
> > -
> > [apply-review-66621-stderr.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/66621/logs/apply-review-66621-stderr.log):
> >
> > ```
> > Traceback (most recent call last):
> > File ".\support\apply-reviews.py", line 447, in
> > main()
> > File ".\support\apply-reviews.py", line 442, in main
> > reviewboard(options)
> > File ".\support\apply-reviews.py", line 432, in reviewboard
> > apply_review(options)
> > File ".\support\apply-reviews.py", line 163, in apply_review
> > commit_patch(options)
> > File ".\support\apply-reviews.py", line 292, in commit_patch
> > shell(cmd, options['dry_run'])
> > File ".\support\apply-reviews.py", line 147, in shell
> > error_code = subprocess.call(command, stderr=subprocess.STDOUT,
> > shell=True)
> > File "C:\Program Files\Python27\lib\subprocess.py", line 168, in call
> > return Popen(*popenargs, **kwargs).wait()
> > File "C:\Program Files\Python27\lib\subprocess.py", line 390, in __init__
> > errread, errwrite)
> > File "C:\Program Files\Python27\lib\subprocess.py", line 610, in
> > _execute_child
> > args = '{} /c "{}"'.format (comspec, args)
> > UnicodeEncodeError: 'ascii' codec can't encode character u'\xe9' in
> > position 23: ordinal not in range(128)
> > ```
>
> Clément Michaud wrote:
> Can anyone help me find what is wrong with the bot?The script is running
> well on my laptop.
Can it be the accent in my first name?
- Clément
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66621/#review201264
---
On avr. 16, 2018, 9 après-midi, Clément Michaud wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66621/
> ---
>
> (Updated avr. 16, 2018, 9 après-midi)
>
>
> Review request for mesos.
>
>
> Bugs: https://issues.apache.org/jira/browse/MESOS-8788
>
> https://issues.apache.org/jira/browse/https://issues.apache.org/jira/browse/MESOS-8788
>
>
> Repository: mesos
>
>
> Description
> ---
>
> Add support for alg RS256 to JWT library.
>
>
> Diffs
> -
>
> 3rdparty/libprocess/include/process/jwt.hpp
> 768cbf6fa91537ff9f45f236f4033097c5cea959
> 3rdparty/libprocess/include/process/ssl/utilities.hpp
> b7cc31c33fd35c93754407f8b350eeb993177f1d
> 3rdparty/libprocess/src/jwt.cpp 921031e6fe3ced5a6be6bc96190fae6d8282ae26
> 3rdparty/libprocess/src/ssl/utilities.cpp
> 4d3727daf53ec62a19255da5a9804d342e770ec2
> 3rdparty/libprocess/src/tests/jwt_keys.hpp PRE-CREATION
> 3rdparty/libprocess/src/tests/jwt_tests.cpp
> eb36a9aed3b11208c7cdc6f20b5347f46821a207
>
>
> Diff: https://reviews.apache.org/r/66621/diff/2/
>
>
> Testing
> ---
>
> make check
>
> I've added the same tests than the ones for HS256 (i.e., validation in
> following cases: bad header, bad payload, unknown alg, unsupported alg, valid
> token etc.. and creation of a valid token).
>
>
> Thanks,
>
> Clément Michaud
>
>
Re: Review Request 66621: Add support for alg RS256 to JWT library.
> On avr. 16, 2018, 9:52 après-midi, Mesos Reviewbot Windows wrote:
> > FAIL: Failed to apply the current review.
> >
> > Failed command: `python.exe .\support\apply-reviews.py -n -r 66621`
> >
> > All the build artifacts available at:
> > http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/66621
> >
> > Relevant logs:
> >
> > -
> > [apply-review-66621-stderr.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/66621/logs/apply-review-66621-stderr.log):
> >
> > ```
> > Traceback (most recent call last):
> > File ".\support\apply-reviews.py", line 447, in
> > main()
> > File ".\support\apply-reviews.py", line 442, in main
> > reviewboard(options)
> > File ".\support\apply-reviews.py", line 432, in reviewboard
> > apply_review(options)
> > File ".\support\apply-reviews.py", line 163, in apply_review
> > commit_patch(options)
> > File ".\support\apply-reviews.py", line 292, in commit_patch
> > shell(cmd, options['dry_run'])
> > File ".\support\apply-reviews.py", line 147, in shell
> > error_code = subprocess.call(command, stderr=subprocess.STDOUT,
> > shell=True)
> > File "C:\Program Files\Python27\lib\subprocess.py", line 168, in call
> > return Popen(*popenargs, **kwargs).wait()
> > File "C:\Program Files\Python27\lib\subprocess.py", line 390, in __init__
> > errread, errwrite)
> > File "C:\Program Files\Python27\lib\subprocess.py", line 610, in
> > _execute_child
> > args = '{} /c "{}"'.format (comspec, args)
> > UnicodeEncodeError: 'ascii' codec can't encode character u'\xe9' in
> > position 23: ordinal not in range(128)
> > ```
Can anyone help me find what is wrong with the bot?The script is running well
on my laptop.
- Clément
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66621/#review201264
---
On avr. 16, 2018, 9 après-midi, Clément Michaud wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66621/
> ---
>
> (Updated avr. 16, 2018, 9 après-midi)
>
>
> Review request for mesos.
>
>
> Bugs: https://issues.apache.org/jira/browse/MESOS-8788
>
> https://issues.apache.org/jira/browse/https://issues.apache.org/jira/browse/MESOS-8788
>
>
> Repository: mesos
>
>
> Description
> ---
>
> Add support for alg RS256 to JWT library.
>
>
> Diffs
> -
>
> 3rdparty/libprocess/include/process/jwt.hpp
> 768cbf6fa91537ff9f45f236f4033097c5cea959
> 3rdparty/libprocess/include/process/ssl/utilities.hpp
> b7cc31c33fd35c93754407f8b350eeb993177f1d
> 3rdparty/libprocess/src/jwt.cpp 921031e6fe3ced5a6be6bc96190fae6d8282ae26
> 3rdparty/libprocess/src/ssl/utilities.cpp
> 4d3727daf53ec62a19255da5a9804d342e770ec2
> 3rdparty/libprocess/src/tests/jwt_keys.hpp PRE-CREATION
> 3rdparty/libprocess/src/tests/jwt_tests.cpp
> eb36a9aed3b11208c7cdc6f20b5347f46821a207
>
>
> Diff: https://reviews.apache.org/r/66621/diff/2/
>
>
> Testing
> ---
>
> I've added the same tests than the ones for HS256 (i.e., validation in
> following cases: bad header, bad payload, unknown alg, unsupported alg, valid
> token etc.. and creation of a valid token).
>
>
> Thanks,
>
> Clément Michaud
>
>
Re: Review Request 66621: Add support for alg RS256 to JWT library.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66621/ --- (Updated avr. 16, 2018, 9 après-midi) Review request for mesos. Changes --- Fix style issues and add one test validating RS256 token with wrong public key Summary (updated) - Add support for alg RS256 to JWT library. Bugs: https://issues.apache.org/jira/browse/MESOS-8788 https://issues.apache.org/jira/browse/https://issues.apache.org/jira/browse/MESOS-8788 Repository: mesos Description (updated) --- Add support for alg RS256 to JWT library. Diffs (updated) - 3rdparty/libprocess/include/process/jwt.hpp 768cbf6fa91537ff9f45f236f4033097c5cea959 3rdparty/libprocess/include/process/ssl/utilities.hpp b7cc31c33fd35c93754407f8b350eeb993177f1d 3rdparty/libprocess/src/jwt.cpp 921031e6fe3ced5a6be6bc96190fae6d8282ae26 3rdparty/libprocess/src/ssl/utilities.cpp 4d3727daf53ec62a19255da5a9804d342e770ec2 3rdparty/libprocess/src/tests/jwt_keys.hpp PRE-CREATION 3rdparty/libprocess/src/tests/jwt_tests.cpp eb36a9aed3b11208c7cdc6f20b5347f46821a207 Diff: https://reviews.apache.org/r/66621/diff/2/ Changes: https://reviews.apache.org/r/66621/diff/1-2/ Testing --- I've added the same tests than the ones for HS256 (i.e., validation in following cases: bad header, bad payload, unknown alg, unsupported alg, valid token etc.. and creation of a valid token). Thanks, Clément Michaud
Re: Review Request 66621: Add alg RS256 support for JWT generator and validator.
> On avr. 16, 2018, 9:25 matin, Alexander Rojas wrote: > > I haven't review it very thoroughly yet, but I found something that needs > > to be fixed. SSL support is an optional configurable feature (`--with-ssl` > > in the configure options). Since your patch depends on SSL, you need the > > guards to enable the feature just when SSL is enabled. Hello Alexander, thanks for the review. Actually, the code is compiled only when the flag is set to true. Check Makefile.am, `src/jwt.cpp` and `src/ssl/utilities.cpp` are in `if ENABLE_SSL` block. - Clément --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66621/#review201187 --- On avr. 14, 2018, 10:19 après-midi, Clément Michaud wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/66621/ > --- > > (Updated avr. 14, 2018, 10:19 après-midi) > > > Review request for mesos. > > > Bugs: https://issues.apache.org/jira/browse/MESOS-8788 > > https://issues.apache.org/jira/browse/https://issues.apache.org/jira/browse/MESOS-8788 > > > Repository: mesos > > > Description > --- > > Add alg RS256 support for JWT generator and validator. > > Currently, the JWT library only supports unsecured and HS256 tokens. I > implemented RS256 to use asymmetrical keys so that Mesos can use it at some > point. > > https://issues.apache.org/jira/browse/MESOS-8788 > > > Diffs > - > > 3rdparty/libprocess/include/process/jwt.hpp > 768cbf6fa91537ff9f45f236f4033097c5cea959 > 3rdparty/libprocess/include/process/ssl/utilities.hpp > b7cc31c33fd35c93754407f8b350eeb993177f1d > 3rdparty/libprocess/src/jwt.cpp 921031e6fe3ced5a6be6bc96190fae6d8282ae26 > 3rdparty/libprocess/src/ssl/utilities.cpp > 4d3727daf53ec62a19255da5a9804d342e770ec2 > 3rdparty/libprocess/src/tests/jwt_keys.hpp PRE-CREATION > 3rdparty/libprocess/src/tests/jwt_tests.cpp > eb36a9aed3b11208c7cdc6f20b5347f46821a207 > > > Diff: https://reviews.apache.org/r/66621/diff/1/ > > > Testing > --- > > I've added the same tests than the ones for HS256 (i.e., validation in > following cases: bad header, bad payload, unknown alg, unsupported alg, valid > token etc.. and creation of a valid token). > > > Thanks, > > Clément Michaud > >
Re: Review Request 66621: Add alg RS256 support for JWT generator and validator.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66621/ --- (Updated avr. 14, 2018, 10:19 après-midi) Review request for mesos. Bugs: https://issues.apache.org/jira/browse/MESOS-8788 https://issues.apache.org/jira/browse/https://issues.apache.org/jira/browse/MESOS-8788 Repository: mesos Description --- Add alg RS256 support for JWT generator and validator. Currently, the JWT library only supports unsecured and HS256 tokens. I implemented RS256 to use asymmetrical keys so that Mesos can use it at some point. https://issues.apache.org/jira/browse/MESOS-8788 Diffs - 3rdparty/libprocess/include/process/jwt.hpp 768cbf6fa91537ff9f45f236f4033097c5cea959 3rdparty/libprocess/include/process/ssl/utilities.hpp b7cc31c33fd35c93754407f8b350eeb993177f1d 3rdparty/libprocess/src/jwt.cpp 921031e6fe3ced5a6be6bc96190fae6d8282ae26 3rdparty/libprocess/src/ssl/utilities.cpp 4d3727daf53ec62a19255da5a9804d342e770ec2 3rdparty/libprocess/src/tests/jwt_keys.hpp PRE-CREATION 3rdparty/libprocess/src/tests/jwt_tests.cpp eb36a9aed3b11208c7cdc6f20b5347f46821a207 Diff: https://reviews.apache.org/r/66621/diff/1/ Testing --- I've added the same tests than the ones for HS256 (i.e., validation in following cases: bad header, bad payload, unknown alg, unsupported alg, valid token etc.. and creation of a valid token). Thanks, Clément Michaud
Review Request 66621: Add alg RS256 support for JWT generator and validator.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66621/ --- Review request for mesos. Repository: mesos Description --- Add alg RS256 support for JWT generator and validator. Currently, the JWT library only supports unsecured and HS256 tokens. I implemented RS256 to use asymmetrical keys so that Mesos can use it at some point. https://issues.apache.org/jira/browse/MESOS-8788 Diffs - 3rdparty/libprocess/include/process/jwt.hpp 768cbf6fa91537ff9f45f236f4033097c5cea959 3rdparty/libprocess/include/process/ssl/utilities.hpp b7cc31c33fd35c93754407f8b350eeb993177f1d 3rdparty/libprocess/src/jwt.cpp 921031e6fe3ced5a6be6bc96190fae6d8282ae26 3rdparty/libprocess/src/ssl/utilities.cpp 4d3727daf53ec62a19255da5a9804d342e770ec2 3rdparty/libprocess/src/tests/jwt_keys.hpp PRE-CREATION 3rdparty/libprocess/src/tests/jwt_tests.cpp eb36a9aed3b11208c7cdc6f20b5347f46821a207 Diff: https://reviews.apache.org/r/66621/diff/1/ Testing --- I've added the same tests than the ones for HS256 (i.e., validation in following cases: bad header, bad payload, unknown alg, unsupported alg, valid token etc.. and creation of a valid token). Thanks, Clément Michaud
