search

crocs-muni / usable-cert-validation

Research initiative to make TLS certificate validation usable.
https://x509errors.org
MIT License
19 stars 3 forks source link

Validate chains using TLS clients, link their errors automatically #98

Closed zacikpa closed 3 years ago

zacikpa commented 3 years ago

This restructures the repository quite a bit.

Main changes:

The code is not yet ready to merge, it still needs a bit of refactoring and documenting, otherwise it's functional. You should be able to build everything locally, just make sure you have all the necessary prerequisites installed (See README.md or .travis.yml).

mukrop commented 3 years ago

Notes for discussion:

ericvalcik commented 3 years ago

I followed the installation guide in the readme.

I'm using WSL with Ubuntu 20.04, and when copy-pasting the command from Readme apt install libopenssl-dev libgnutls28-dev botan libbotan-2-dev libmbedtls-dev libboost-program-options-dev openjdk-16-jdk the packages libopenssl-dev and openjdk-16-jdk couldn't be located. Instead of libopenssl-dev I used libssl-dev, and that worked. Openjdk I had to download manually from their website.

make install went fine, but then with make local I got stuck on Validating chain: NOTAFTER_FIELD_INVALID. I tried redoing the entire process but got stuck on the same thing. It is possible the issue is on my side. I terminated the validation, and just run the website with bundle exec jekyll serve. Some observations:

Is it building without issues at your local machines? If so, do you see anything I am doing horribly wrong?

zacikpa commented 3 years ago

@valcikeric

Regarding the packages, libopenssl-dev is clearly a typo. Seems like openjdk 14/15 is the latest version in ubuntu repositories, we should probably mention that, it should run well on both.

I got stuck on Validating chain: NOTAFTER_FIELD_INVALID.

This sounds weird, I will commit some changes with the possibility to print debug info and I will then ask you to try it again, please.

The corresponding errors are no longer present. I can take a look into that. It is possible this is due to the interrupted setup, but I don't see the mapping files being created in the new Makefile anyway.

This is most certainly due to the interrupted setup. You don't have to waste time looking into it.

I couldn't test the example certs, because I couldn't find any on the website. Every error has the icon of having example cert, the paragraph is present but no download link. Again it could be because of the interrupted make local command.

You are right about the cause as well.

I will let you know when it's ready to run with debug info on.

zacikpa commented 3 years ago

@valcikeric A bit of a dirty fix, but you should be able to run make debug.

It will print a LOT of very uglily formatted stuff, so don't get scared. What I'm interested in, is what happens after Validating chain: NOTAFTER_FIELD_INVALID.

Please post all output following that line here. Thank you!

mukrop commented 3 years ago

Thanks @valcikeric for the testing. The packages were only estimated for Ubuntu from my real usage on Fedora, thanks for cleaning that up. As Palo mentions, the rest is probably due to a broken setup. It works OK at my machine – the the suggested debug and let us know.

ericvalcik commented 3 years ago

I'm having issues with my WSL, so I cannot test it right now. I'm trying to resolve it as fast as possible.

ericvalcik commented 3 years ago

Hi, my WSL is finally running. This is the requested text after running make debug:

Validating chain: NOTAFTER_FIELD_INVALID
DEBUG_INFO: Running python TLS server with NOTAFTER_FIELD_INVALID

  File "/home/eric/job/usable-cert-validation/validation/certs/../servers/server.py", line 9
    def __init__(self, key_path: str, cert_path: str):
                               ^
SyntaxError: invalid syntax

When I run python it runs version 2.7.18 where these type hints weren't added yet, so that is probably the source of the problem. So I just changed python to python3 in validation/certs/results/validate.sh and then it started working. These are the two changes I made:

diff --git a/validation/certs/results/validate.sh b/validation/certs/results/validate.sh
index 4b1c68b..da665f7 100755
--- a/validation/certs/results/validate.sh
+++ b/validation/certs/results/validate.sh
@@ -127,13 +127,13 @@ run_servers() {
         if [ "$DEBUG" == "true" ]
         then
           printf "\nDEBUG_INFO: Running python TLS server with %s\n" "${CHAIN_NAME}"
-          python "${SERVERS_DIR}"/server.py --chain_file "${CHAIN_FILE}" \
+          python3 "${SERVERS_DIR}"/server.py --chain_file "${CHAIN_FILE}" \
                                             --key_file "${KEY_FILE}" \
                                             --host "${HOST}" \
                                             --port "${MAIN_PORT}" &
           printf "\n"
         else
-          python "${SERVERS_DIR}"/server.py --chain_file "${CHAIN_FILE}" \
+          python3 "${SERVERS_DIR}"/server.py --chain_file "${CHAIN_FILE}" \
                                             --key_file "${KEY_FILE}" \
                                             --host "${HOST}" \
                                             --port "${MAIN_PORT}" \

I will follow up soon (hopefully tomorrow) with the actual tests of the page.

zacikpa commented 3 years ago

Thanks for looking into it.

I am quite surprised that the certificate generation went successfully when you run Python2.7 as default. I should specify python3 to run there explicitly as well.

Looking forward to see what happens when you run it! :)

mukrop commented 3 years ago

Yes, please specify the requirement for Python 3 also in the Readme and set the build to use it over Pyhon 2 automatically, please.

On Thu, 6 May 2021 at 08:08, Pavol Zacik @.***> wrote:

Thanks for looking into it.

I am quite surprised that the certificate generation went successfully when you run Python2.7 as default. I should specify python3 to run there explicitly as well.

Looking forward to see what happens when you run it! :)

— You are receiving this because your review was requested. Reply to this email directly, view it on GitHub https://github.com/crocs-muni/usable-cert-validation/pull/98#issuecomment-833253406, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAOI6PB5SYTHPZRRAB2K7TLTMIW4PANCNFSM43EP4YOA .

ericvalcik commented 3 years ago

Ok, I will just write all the things that I'm not sure about:

I didn't find any other errors.

zacikpa commented 3 years ago

The mapping, corresponding errors, and example cert distribution among errors are different compared to the current page, but maybe that's intentional.

This is completely intentional since the mapping is done differently and all example certs are new.

Clicking on generation script for any example cert leads to 404.

This was expected since the links point to the master branch, where the changes are not yet deployed.

I liked the validate with: .... field, or at least it helped me to manage the validation easily...

We discussed this with Martin and we certainly want the feature, though it probably won't be brought back in the next month or so.

For example, I downloaded the VALID.zip, unzipped and ran openssl verify -CAfile root/root.pem VALID/chain.pem, but got...

Oh, right, this is because OpenSSL only recognizes a single certificate (the first one) in VALID/chain.pem, which contains an intermediate CA too. With OpenSSL, you would have to split the chain into separate files and load the intermediate CAs with the --untrusted option.

This is my bad, sorry, I forgot about it. I guess the solution would be to export both the chain and the separate certs side by side for each example chain (since certtool for instance takes the whole chain as an input). It could look like this:

VALID.zip/
    chain.pem
    root.pem
    split_chain/
           endpoint.pem
           intermediate_ca.pem

What do you think? @mukrop

Good to know that the build was successful, thank you for the testing!

mukrop commented 3 years ago

I'll just add that the validation command is now a bit tricky to add, as validation for mapping is done using the library APIs, not CLI tools any longer.

As for the archives: Does the chain.pem contain all the certificates including root? What about the following structure? Archive.zip

zacikpa commented 3 years ago

As for the archives: Does the chain.pem contain all the certificates including root?

It does not contain the root, but I have no problem adding it to make it clearer (according to the TLS specification, it MAY or MAY not be present).

What about the following structure?

Looks alright, just note that the order is actually reversed in the chain (endpoint first), so this would make more sense to me:

Archive.zip
- 1-endpoint.pem
- 2-intermediate.pem
- 3-intermediate.pem
- 4-root.pem
- chain.pem (concatenation if 1–4)

Edit: I will make the changes hopefully later today.

mukrop commented 3 years ago

It does not contain the root, but I have no problem adding it to make it clearer (according to the TLS specification, it MAY or MAY not be present).

Hm, interesting. I always thought the root CA should not be included (at least the Qualys SSL test always complained). May be worth a check, though probably after the thesis. Maybe it's nit per RFC but the CA/browser forum guidelines.

Looks alright, just note that the order is actually reversed in the chain (endpoint first), so this would make more sense to me:

Yeah, I see. My point was to keep the root with a stable filename (as it's the file you often use separately even if using the chain). Maybe then chain without root, individuals from the chain also separately (maybe as chain-1-endpoint to emphasize it's in the chain?) and the root unnumberet and separate?

Anyway, I'd probably leave this for the separate issue.

Is there something that prevents merge then?

zacikpa commented 3 years ago

Maybe then chain without root, individuals from the chain also separately (maybe as chain-1-endpoint to emphasize it's in the chain?) and the root unnumberet and separate? Anyway, I'd probably leave this for the separate issue.

~Ok, I have already created an issue for it yesterday.~ Edit: I misread the article, you noticed the issue already :P

Is there something that prevents merge then?

I don't think so, you can merge and I will then check whether everything worked alright.