Bug #4962
ndncert-client: insufficient input validation
50%
Description
I can crash ndncert-client
interactive procedure with a large variety of inputs. This is not very user friendly.
Some inputs I found so far (note that the crash reasons are all different from each other)
*************************************** Index: 0 CA prefix:/ndn/edu/ucla-ndncert Introduction: NDN Testbed CA *************************************** Step 0: Please type in the CA INDEX that you want to apply or type in NONE if your expected CA is not in the list 1 Segmentation fault (core dumped)
*************************************** Index: 0 CA prefix:/ndn/edu/ucla-ndncert Introduction: NDN Testbed CA *************************************** Step 0: Please type in the CA INDEX that you want to apply or type in NONE if your expected CA is not in the list -1 terminate called after throwing an instance of 'std::length_error' what(): basic_string::_M_create Aborted (core dumped)
*************************************** Index: 0 CA prefix:/ndn/edu/ucla-ndncert Introduction: NDN Testbed CA *************************************** Step 0: Please type in the CA INDEX that you want to apply or type in NONE if your expected CA is not in the list foo terminate called after throwing an instance of 'std::invalid_argument' what(): stoi Aborted (core dumped)
*************************************** Index: 0 CA prefix:/ndn/edu/ucla-ndncert Introduction: NDN Testbed CA *************************************** Step 0: Please type in the CA INDEX that you want to apply or type in NONE if your expected CA is not in the list NONE Step 1: Please type in the CA Name Got NACK
*************************************** Index: 0 CA prefix:/ndn/edu/ucla-ndncert Introduction: NDN Testbed CA *************************************** Step 0: Please type in the CA INDEX that you want to apply or type in NONE if your expected CA is not in the list 0 Step 1: Please provide information for name assignment Please provide the argument: email : foo@bar.com Got it. This is what you've provided: email : foo@bar.com If everything is right, please type in OK; otherwise, type in REDO foo Got NACK
(I'm not sure about the last two, could be a different bug)
Updated by Davide Pesavento over 1 year ago
In the last case from the issue description, the "got NACK" part is due to a missing route on the testbed (unrelated problem), but there is still a bug because every user input except "REDO" is interpreted as "OK" and the procedure continues.
Updated by Davide Pesavento over 1 year ago
This is also very unfriendly to the user (note that I typed "email" all lowercase):
Step 3: Please type in the challenge ID from the following challenges PIN Email email Cannot recognize the specified challenge. Exit
Updated by Davide Pesavento over 1 year ago
What's worse is that during all these failed tries, ndncert did create a few (self-signed) certificates in my local keychain, a new one each time, but they were never removed when the procedure failed or was interrupted. When I finally managed to get it working and obtain a certificate from the CA, ndnsec list
was showing a bunch of certificates with very similar names and no indication about which one was the "right" one.
So, at the very least, ndncert-client
should: 1) delete the temporary self-signed certs if the procedure fails or is aborted; 2) print the name of the issued cert at the end of the procedure.
Updated by Davide Pesavento over 1 year ago
The user input to the "Please type in your expected validity period..." prompt is not validated either. I entered a negative number and the tool happily accepted it.
Interestingly, the CA was also perfectly happy to give me a certificate with a negative validity period!
+-> /ndn/edu/ucla-ndncert/3403139456692156406/KEY/%D7%BCH%1F%BF%92-%94/NDNCERT/5297390542062471952 Certificate name: /ndn/edu/ucla-ndncert/3403139456692156406/KEY/%D7%BCH%1F%BF%92-%94/NDNCERT/5297390542062471952 Validity: NotBefore: 20190627T235508 NotAfter: 20190627T225441 Public key bits: MFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEa0I+e916g4ebABXBplUpX2KYhYpW qGXi9MEUeuXD9nQ8LoU4J6O12UTakpblD/DRKm/4KNlMw9DhwAfyA+K/PQ== Signature Information: Signature Type: SignatureSha256WithRsa Key Locator: Name=/ndn/edu/ucla-ndncert/KEY/%82%F4%E94R%0B%26%18
Updated by Yufeng Zhang over 1 year ago
Davide Pesavento wrote:
I can crash
ndncert-client
interactive procedure with a large variety of inputs. This is not very user friendly.Some inputs I found so far (note that the crash reasons are all different from each other)
*************************************** Index: 0 CA prefix:/ndn/edu/ucla-ndncert Introduction: NDN Testbed CA *************************************** Step 0: Please type in the CA INDEX that you want to apply or type in NONE if your expected CA is not in the list 1 Segmentation fault (core dumped)*************************************** Index: 0 CA prefix:/ndn/edu/ucla-ndncert Introduction: NDN Testbed CA *************************************** Step 0: Please type in the CA INDEX that you want to apply or type in NONE if your expected CA is not in the list -1 terminate called after throwing an instance of 'std::length_error' what(): basic_string::_M_create Aborted (core dumped)*************************************** Index: 0 CA prefix:/ndn/edu/ucla-ndncert Introduction: NDN Testbed CA *************************************** Step 0: Please type in the CA INDEX that you want to apply or type in NONE if your expected CA is not in the list foo terminate called after throwing an instance of 'std::invalid_argument' what(): stoi Aborted (core dumped)*************************************** Index: 0 CA prefix:/ndn/edu/ucla-ndncert Introduction: NDN Testbed CA *************************************** Step 0: Please type in the CA INDEX that you want to apply or type in NONE if your expected CA is not in the list NONE Step 1: Please type in the CA Name Got NACK*************************************** Index: 0 CA prefix:/ndn/edu/ucla-ndncert Introduction: NDN Testbed CA *************************************** Step 0: Please type in the CA INDEX that you want to apply or type in NONE if your expected CA is not in the list 0 Step 1: Please provide information for name assignment Please provide the argument: email : foo@bar.com Got it. This is what you've provided: email : foo@bar.com If everything is right, please type in OK; otherwise, type in REDO foo Got NACK(I'm not sure about the last two, could be a different bug)
Thank you, Davide. I've fixed most of the bugs you mentioned in this issue. However, I am not sure about the length validation part. I need to ask Zhiyi where the maximum time limit is stored in the configuration. I'll push it to Gerrit soon.
Updated by Zhiyi Zhang over 1 year ago
The CA configuration file contains the maximum validity period while the client side does not know.
I will add the minus value check to both the CA and the client command-line tool.
Updated by Davide Pesavento about 1 year ago
- % Done changed from 0 to 50
As of ndncert commit 55d9860fc346d04f4c844e80304dee7cb47de6da
, most of the above problems are fixed, except the following one:
*************************************** Index: 0 CA prefix:/ndn/edu/ucla/yufeng Introduction: UCLA CA in NDN Testbed *************************************** Step 0: Please type in the CA INDEX that you want to apply or type in NONE if your expected CA is not in the list NONE Step 1: Please type in the CA Name Got NACK
I discovered another problem though: if I press Ctrl+D (EOF) at the email, validity period, challenge type, or any of the confirmation prompts, the program enters an infinite loop. Example below:
Please provide the argument: email : Got it. This is what you've provided: email : If everything is correct, please type in OK; otherwise, type in REDO Please provide the argument: email : Got it. This is what you've provided: email : If everything is correct, please type in OK; otherwise, type in REDO Please provide the argument: email : [...]
Updated by Davide Pesavento about 1 year ago
Another bug:
*************************************** Index: 0 CA prefix:/ndn/edu/ucla/yufeng Introduction: UCLA CA in NDN Testbed *************************************** Step 0: Please type in the CA INDEX that you want to apply or type in NONE if your expected CA is not in the list 0 Step 1: Please provide information for name assignment Please provide the argument: email : foo Got it. This is what you've provided: email : foo If everything is correct, please type in OK; otherwise, type in REDO ok Step 2: Please type in your expected validity period of your certificate. Type the number of hours (168 for week, 730 for month, 8760 for year). The CA may change the validity period if your expected period is too long. 1 The validity period of your certificate will be: 1 hours Step 3: Please type in the challenge index that you want to perform 0 : email 1 : pin 0 The challenge has been selected: email Step 4: Please satisfy following instruction(s) Please_input_your_email_address Please provide the argument: email : foo Got it. This is what you've provided: email : foo If everything is correct, please type in OK; otherwise, type in REDO ok terminate called after throwing an instance of 'boost::exception_detail::clone_impl<boost::exception_detail::error_info_injector<boost::property_tree::ptree_bad_path> >' what(): No such node (selected-challenge) Aborted (core dumped)
Updated by Davide Pesavento about 1 year ago
- Subject changed from ndncert-client: poor input validation to ndncert-client: insufficient input validation
Updated by Davide Pesavento about 1 year ago
Davide Pesavento wrote:
What's worse is that during all these failed tries, ndncert did create a few (self-signed) certificates in my local keychain, a new one each time, but they were never removed when the procedure failed or was interrupted. When I finally managed to get it working and obtain a certificate from the CA,
ndnsec list
was showing a bunch of certificates with very similar names and no indication about which one was the "right" one.So, at the very least,
ndncert-client
should: 1) delete the temporary self-signed certs if the procedure fails or is aborted; 2) print the name of the issued cert at the end of the procedure.
This is still a problem in the current code too. Implementing (2) would be really trivial and could help the user a lot already. (1) is a bit more involved, I could open a separate issue for that one.
Also, when the certificate is issued successfully and installed locally, the self-signed cert is still set as the "default" cert for the key. Maybe this logic could be smarter and set the ndncert-issued cert as default if there are no other pre-existing certificates set as default.
Updated by Davide Pesavento about 1 year ago
Another one...
*************************************** Index: 0 CA prefix:/ndn/edu/ucla/yufeng Introduction: UCLA CA in NDN Testbed *************************************** Step 0: Please type in the CA INDEX that you want to apply or type in NONE if your expected CA is not in the list 0 Step 1: Please provide information for name assignment Please provide the argument: email : foobar Got it. This is what you've provided: email : foobar If everything is correct, please type in OK; otherwise, type in REDO ok Step 2: Please type in your expected validity period of your certificate. Type the number of hours (168 for week, 730 for month, 8760 for year). The CA may change the validity period if your expected period is too long. 123456789 The validity period of your certificate will be: 123456789 hours Interest sent time out
Updated by Zhiyi Zhang 10 months ago
Davide Pesavento wrote:
Any progress here?
I think this is caused because the CA on spurs is down while the new CA has not started.
Updated by Zhiyi Zhang 10 months ago
- Status changed from In Progress to Code review
Davide Pesavento wrote:
Any progress here?
I think this is caused because the CA on spurs is down while the new CA has not started.
Updated by Davide Pesavento 10 months ago
What are you talking about? What is caused by the CA being down? I reported several problems above. Also note that that happened 5 months ago, I don't think the CA was down at that time.