search

zenhack / simp_le

Simple Let's Encrypt client
GNU General Public License v3.0
223 stars 38 forks source link

CI: run tests against pebble #123

Closed zenhack closed 4 years ago

zenhack commented 5 years ago

Which apparently would have caught #122.

buchdag commented 5 years ago

The biggest issue I'm having right now is https://github.com/letsencrypt/pebble#avoiding-client-https-errors

I tried various method to make the integration test use the test CA of pebble, without success so far.

zenhack commented 5 years ago

Is your WIP available somewhere for me to look at?

buchdag commented 5 years ago

https://github.com/buchdag/simp_le/tree/pebble, more specifically https://github.com/buchdag/simp_le/commit/3171a587fd3028cbaa9bb2f4cb585985a166ed48

buchdag commented 4 years ago

Okay, one working solution is to drop SSL verification at the acme module level: https://github.com/buchdag/simp_le/commit/3631faedac8a5cf0a451c4c43451ec426d23a7f8

The fact that this is what certbot does too makes me think that the acme module actually does not follow Pebble recommendation that

client should offer a runtime option to specify a list of trusted root CAs

and that skipping SSL verification is currently the only way to use Pebble with the Python acme module.

@cpu, @bmw, any insight on the above ?

cpu commented 4 years ago

Disabling validation at the acme level seems like the wrong solution if its possible to avoid it.

that skipping SSL verification is currently the only way to use Pebble with the Python acme module.

I don't think that's the case, but the solution isn't super well documented. The Boulder/Pebble team uses the Certbot acme Python package in a test client we call "chisel2" for Boulder/Pebble tests. The version that runs against Pebble is here. When we invoke it during CI we pass a modified environment that includes REQUESTS_CA_BUNDLE set to the Pebble CA certificate. That leaves the TLS verification enabled while also allowing Pebble's non-standard root to be considered trusted. Would a similar approach work for simp_le?

@bmw would know more about what Certbot does now and is planning to do in the future. This topic of conversation came up last week in https://github.com/certbot/certbot/issues/7170 and I think that's probably an interesting thread for you folks too.

Hope that helps!

bmw commented 4 years ago

Yeah I'd recommend setting REQUESTS_CA_BUNDLE as cpu described.

You're right that Certbot just disables certificate verification entirely during testing through the flag you linked above and the verify_ssl attribute of the ClientNetwork class. We've had this setup for a very long time and we don't have any plans to change it, however, since as far as I know simp_le doesn't already have a flag to toggle this behavior, REQUESTS_CA_BUNDLE seems like a simpler way to me for you all to solve this problem.

zenhack commented 4 years ago

Yeah, I don't want to introduce security-defeating features just for testing. If we had to disable SSL verification to get this to work I'd rather just keep testing against Boulder. (I actually think there's value in testing against both, especially since we've already got Boulder set up).

buchdag commented 4 years ago

Nevermind, I'm a retard, been using the non raw Github URL to fetch the certificate file for weeks. :man_facepalming:

The clean solution I had last week is actually working, PR is pending.

buchdag commented 4 years ago

@zenhack you can close this issue