New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SimpleFS plugin (fixes #742) #757

Merged
merged 7 commits into from Oct 17, 2015

Conversation

Projects
None yet
7 participants
@kuba
Contributor

kuba commented Sep 6, 2015

SimpleHTTP challenge solved by interacting with the file system only (requires already running web server).

@centminmod, could you test it for me, please?

kuba added some commits Sep 2, 2015

@centminmod

This comment has been minimized.

centminmod commented Sep 6, 2015

@kuba sorry didn't get any notifications for this. Is this same when you posted 4 days ago at #742 (comment)?

As I tested it then and works fine https://community.centminmod.com/posts/17965/

just retested with latest git pull from your simplefs branch and works okay

./venv/bin/letsencrypt -a simplefs --simplefs-root /home/nginx/domains/le4.http2ssl.xyz/public --text --agree-eula -d le4.http2ssl.xyz auth

outputs

./venv/bin/letsencrypt -a simplefs --simplefs-root /home/nginx/domains/le4.http2ssl.xyz/public --text --agree-eula -d le4.http2ssl.xyz auth
/root/tools/letsencrypt/venv/lib/python2.6/site-packages/cryptography/__init__.py:25: DeprecationWarning: Python 2.6 is no longer supported by the Python core team, please upgrade your Python.
  DeprecationWarning

IMPORTANT NOTES:
 - Automatic renewal and deployment has been enabled for your
   certificate. These settings can be configured in the directories
   under /etc/letsencrypt/configs.

SSL certs

ls -lah /etc/letsencrypt/live/
total 32K
drwx------ 8 root root 4.0K Sep  6 23:51 .
drwxr-xr-x 8 root root 4.0K Aug 29 07:52 ..
drwxr-xr-x 2 root root 4.0K Aug 29 07:52 le1.http2ssl.xyz
drwxr-xr-x 2 root root 4.0K Aug 30 08:07 le2.http2ssl.xyz
drwxr-xr-x 2 root root 4.0K Sep  4 07:53 le3.http2ssl.xyz
drwxr-xr-x 2 root root 4.0K Sep  4 08:07 le4.http2ssl.xyz
drwxr-xr-x 2 root root 4.0K Sep  4 08:10 le4.http2ssl.xyz-0001
drwxr-xr-x 2 root root 4.0K Sep  6 23:51 le4.http2ssl.xyz-0002

ls -lah /etc/letsencrypt/live/le4.http2ssl.xyz-0002
total 8.0K
drwxr-xr-x 2 root root 4.0K Sep  6 23:51 .
drwx------ 8 root root 4.0K Sep  6 23:51 ..
lrwxrwxrwx 1 root root   45 Sep  6 23:51 cert.pem -> ../../archive/le4.http2ssl.xyz-0002/cert1.pem
lrwxrwxrwx 1 root root   46 Sep  6 23:51 chain.pem -> ../../archive/le4.http2ssl.xyz-0002/chain1.pem
lrwxrwxrwx 1 root root   50 Sep  6 23:51 fullchain.pem -> ../../archive/le4.http2ssl.xyz-0002/fullchain1.pem
lrwxrwxrwx 1 root root   48 Sep  6 23:51 privkey.pem -> ../../archive/le4.http2ssl.xyz-0002/privkey1.pem
@kuba

This comment has been minimized.

Contributor

kuba commented Sep 7, 2015

The code has been updated slightly (previous version broke entire client unless --simplefs-root was supplied for every command, e.g. letsencrypt --simplefs-root plugins etc.). Thanks for testing :)

@jdkasten

This comment has been minimized.

Contributor

jdkasten commented Sep 7, 2015

Why not try to make this part of standalone?.( if there is already a
process running... use this to work with the existing Web server.)

I think that the number of plugin options should be kept to a minimum to
avoid confusion. I think this plugin is aimed at the same audience.
On Sep 7, 2015 1:39 AM, "Jakub Warmuz" notifications@github.com wrote:

The code has been updated slightly (previous version broke entire client
unless --simplefs-root was supplied for every command, e.g. letsencrypt
--simplefs-root plugins etc.). Thanks for testing :)


Reply to this email directly or view it on GitHub
#757 (comment)
.

@jdkasten

This comment has been minimized.

Contributor

jdkasten commented Sep 7, 2015

Hmmm... I guess the lines may get blurred... and plugin names will have to
change.

Thoughts?
On Sep 7, 2015 2:06 AM, "James Kasten" jdkasten@umich.edu wrote:

Why not try to make this part of standalone?.( if there is already a
process running... use this to work with the existing Web server.)

I think that the number of plugin options should be kept to a minimum to
avoid confusion. I think this plugin is aimed at the same audience.
On Sep 7, 2015 1:39 AM, "Jakub Warmuz" notifications@github.com wrote:

The code has been updated slightly (previous version broke entire client
unless --simplefs-root was supplied for every command, e.g. letsencrypt
--simplefs-root plugins etc.). Thanks for testing :)


Reply to this email directly or view it on GitHub
#757 (comment)
.

@kuba

This comment has been minimized.

Contributor

kuba commented Sep 7, 2015

Logic would be different for simplefs and standalone SimpleHTTP, where we would fire up a in-memory webserver that doesn't touch a file system at all. We can try to consolidate plugins later - it's just a matter of a bunch of "intelligent" switches to make it happen. Merging those 2 plugins now would be a mess (especially that I'm about to rewrite standalone).

@centminmod

This comment has been minimized.

centminmod commented Sep 7, 2015

@kuba how would simplefs handle SANs cert ?

i.e. for le4.http2ssl.xyz and le5.http2ssl.xyz

./venv/bin/letsencrypt -a simplefs --simplefs-root /home/nginx/domains/le4.http2ssl.xyz/public --simplefs-root /home/nginx/domains/le5.http2ssl.xyz/public --text --agree-eula -d le4.http2ssl.xyz -d le5.http2ssl.xyz auth

or something for later ? :)

@kuba

This comment has been minimized.

Contributor

kuba commented Sep 7, 2015

@centminmod, I recognize it as an issue, but can tackle it in a follow-up PR

@centminmod

This comment has been minimized.

centminmod commented Sep 7, 2015

Cheers @kuba no hurry as i think most folks who would opt for this simplefs method might be doing on a per site/vhost path basis unless they're parking domains on top of the main domain too.

@kuba

This comment has been minimized.

Contributor

kuba commented Sep 15, 2015

Anyone could review, please? :)

@schoen

This comment has been minimized.

Contributor

schoen commented Sep 16, 2015

I kind of like @jdkasten's suggestion to merge this into standalone and I'm curious what other people think of this. (The idea would be that if it couldn't bind the port, it could say "As an alternative, if you have permission to create files in this server's web root...") I'm wondering what other people think about this possibility.

@centminmod

This comment has been minimized.

centminmod commented Sep 16, 2015

i don't mind either way as long as we can pass the web root path on the command line and override the manual prompts :)

@kuba

This comment has been minimized.

Contributor

kuba commented Sep 16, 2015

We can psutil before initializing the standalone authenticator and initialize simplefs in case port is taken. Again, I believe that this is topic for a follow-up PR, after I submit the shiny new standalone plugin. Entangling standalone with simplefs directly breaks modularity.

@jdkasten

This comment has been minimized.

Contributor

jdkasten commented Sep 22, 2015

Once again, I think this is great. I think this is really important.... I am still just hesitant to bring it in as is, as we get closer to launch. We have put off a bunch of tasks that still need to be handled and adding to the bunch doesn't seem right.

I think it is more confusing to have this as a separate option and then invoke it from standalone. I think we should keep the number of options to an absolute minimum, Perhaps my problem is I don't feel it has a very descriptive name. SimpleFS means nothing to me. I wouldn't know which option to choose without manually reading the descriptions (which is not supported in text mode)

Would it be possible to rename this to Other Webserver or something that conveys its use without having to read the additional information? I think I would be OK merging if it had a more descriptive name.

@kuba @pde @schoen @bmw Do you guys have any better ideas for a name?

Finally, I am concerned about the lack of compatibility with the rest of the client options. I think not supporting multiple names is a problem. If multiple names are not supported (names with different web roots) the client should at least throw an appropriate error.

@centminmod

This comment has been minimized.

centminmod commented Sep 23, 2015

what about a name of what it is basically ? Web Root Authentication

./venv/bin/letsencrypt -a webroot --webroot-path

for multiple names, maybe allow multiple --webroot-path in an array and then if multiple -d nameXX are specified cycle through the --webroot-path array to check against the -d name XX values ?

@bmw

This comment has been minimized.

Contributor

bmw commented Sep 30, 2015

One problem with this that was pointed out to me by @jsha is that ACME specifies that the Content-Type header in the HTTP response is either absent or application/jose+json. This plugin has no control over the value of this header.

@centminmod

This comment has been minimized.

centminmod commented Oct 1, 2015

@bmw that shouldn't be a problem as you can make note of it in manual/documentation for folks to script their own https vhost prior to running simplefs mode. That's what I did for simplefs tests at https://community.centminmod.com/threads/letsencrypt-ssl-certificate-on-centmin-mod-nginx-http-2.4250/

I added to header for application/jose+json to one my standard nginx vhost include files when i auto generate Nginx http and https vhosts https://github.com/centminmod/centminmod/blob/master/config/nginx/staticfiles.conf#L1-L7

@bmw

This comment has been minimized.

Contributor

bmw commented Oct 1, 2015

While that certainly works @centminmod, it requires the user to manual edit their server's config files which as an automated CA, we'd obviously like to avoid. Additionally, we'd be unable to give much help to the users about how to go about setting the value of that header as it's impossible to include instructions for all web servers. The more technically sophisticated users like you wouldn't have a problem, but others would.

I'm not suggesting that this problem is a complete blocker for this plugin, however, I think it should definitely be discussed. It unfortunately puts a bit of a damper on ideas like #784.

@kuba

This comment has been minimized.

Contributor

kuba commented Oct 1, 2015

Also, see ietf-wg-acme/acme#9, as reported by @jsha.

Update on this PR: it's a bit lower prio than any other work but we've recently established that renaming the plugin to something more sensible (maybe webroot?) and maybe adding more docs ("this plugins requires server running...") should unblock the merge with master.

@centminmod

This comment has been minimized.

centminmod commented Oct 2, 2015

@bmw true requires some edits on server config side would would be a good alternative for folks trying to get it to work with nginx at least which is a growing portion for web servers http://w3techs.com/technologies/history_overview/web_server. Nginx technical how to guides spread online like wildfire so any how tos for this would eventually spread around. Maybe once Letsencrypt goes live, even contact Nginx.com folks and have them post a blog tech category article on how to setup Letsencrypt client and run it on stock Nginx repository installed servers too.

They also have a new Nginx Wiki Resource at https://www.nginx.com/resources/wiki/ - maybe a Letsencrypt entry there too :)

If you want other web hosts and control panels to have Letsencrypt tool integration - simplefs would be the easiest way to get started, as such web hosts and control panels will ultimately be the ones who know the ins and outs of their web server setup and environment the best. So would be alot easier for them to script or code a pre-generated https vhost for passing a web root path to a vhost and setting up the appropriate Content-Type out of the box.

@bmw

This comment has been minimized.

Contributor

bmw commented Oct 2, 2015

For Nginx specifically, I personally think that the better approach in the long term is to get the Nginx plugin working again. A number of developers have invested a lot of time in fully automating the process on Nginx and it seems a shame for that effort to go to waste.

You bring up a good point regarding other web servers. Using this plugin as a base, it should be trivial to write the code to set the Content-Type for a specific web server. Additionally, some people may prefer this option to having to temporarily stop their web server to run the standalone authenticator. I don't think this plugin is a bad idea. I was really excited when I first saw it. I just wanted to point out that a problem with this approach that I don't believe had been considered.

@centminmod

This comment has been minimized.

centminmod commented Oct 2, 2015

A number of developers have invested a lot of time in fully automating the process on Nginx and it seems a shame for that effort to go to waste.

It ain't going to waste if it at least works with debian/ubuntu nginx installs ? just need to extend it to other nginx/OS setups :) Unfortunately, there's so many ways Nginx can be configured - more so than Apache, that anticipating all Nginx configs would be harder.

@chriscroome

This comment has been minimized.

chriscroome commented Oct 2, 2015

Supporting multiple SAN's would be really helpful, I'd be very happy with just one web root per cert as this is a common way of organising web server configurations. For shared servers it could be odd getting certs mixing the domain names from different sites in the same cert, especially when the different sites represent different organisations, where as having domains such as example.org, example.org.uk, example.net etc in the same cert is perfect if you simply want to redirect all port 80 traffic to 443 without anyone getting SSL/TLS warnings.

Documenting the server configuration needed to set the Content-Type or even doing it via conf-available on Debian servers would be fine.

I would very much like to only have to do Apache / Nginx restarts rather than stopping web servers to run another one -- that really isn't viable on a server with dozens of sites.

I'd be happy to do some testing of this plugin on Debian servers.

@chriscroome

This comment has been minimized.

chriscroome commented Oct 3, 2015

The Apache config I have used for setting the Content-Type, create /etc/apache2/conf-available/letsencrypt.conf containing:

<IfModule mod_headers.c>
  <LocationMatch "/.well-known/acme-challenge/*">
    Header set Content-Type "application/jose+json"
  </LocationMatch>
</IfModule>

And then enable it:

a2enmod headers
a2enconf letsencrypt

If -d took a list of names with the first being used as the Common Name and the rest as SAN's that would be ideal.

kuba added some commits Oct 4, 2015

@kuba

This comment has been minimized.

Contributor

kuba commented Oct 4, 2015

@chriscroome, do you mind if I put your snippets in the plugin documentation?

You can already use this plugin for SAN certs as long as all involved domains (vhosts) use the same webroot. The plugin doesn't work only if you want to have SAN cert with different webroots - I will address it in a follow-up PR.

https://github.com/letsencrypt/letsencrypt/pull/757/files#diff-6e5c451d7dd9b8001119507cdba36330R28 is already meeting the documentation requirement I mentioned above. However, I renamed simplefs to webroot and --simplefs-root to --webroot-path. Therefore, it should be now clear to merge. Please remember that we can at any point delete the entry point or hide the plugin using hidden=False, or even move this plugin to a separate letsencrypt-plugins package, in case not enough integration with standalone is made before launch.

@chriscroome

This comment has been minimized.

chriscroome commented Oct 4, 2015

@kuba feel free to use that snippet as you wish!

Regarding SANs, -d example.org -d example.com does work, I had been trying -d example.org,example.com and it didn't work, sorry for not trying that before.

When the command is run subsequent times the sym links in /etc/letsencrypt/live/example.org/ are not updated to point to the latest key and cert, is there another command I should be using to update these symlinks?

What is the situation regarding renewals? I was thinking if the script which is using this webroot method to get key pairs was set to run every 6 weeks or so then there would never be a need for the standalone server to be run for renewals, is that correct?

@chriscroome

This comment has been minimized.

chriscroome commented Oct 4, 2015

I have done some testing and have found that --email example@example.org and --duplicate need adding to the command like to prevent interactive prompts.

But then it fails:

Failed authorization procedure ... 404

It appears that the file under /.well-known/acme-challenge/ isn't being created? Also a config file for the domain in /etc/letsencrypt/configs/ is no longer being created.

@chriscroome

This comment has been minimized.

chriscroome commented Oct 4, 2015

I have solved the 404 problem I had in #757 (comment) by reinstalling the code:

rm -rf /usr/local/letsencrypt
cd /usr/local
git clone -b simplefs https://github.com/kuba/letsencrypt
cd letsencrypt/
virtualenv --no-site-packages -p python2 venv
./venv/bin/pip install -r requirements.txt acme/ . letsencrypt-apache/ 

I still have the issue with the symlinks mentioned in #757 (comment) — when adding additional SANs to a cert the sym links in /etc/letsencrypt/live/example.org/ don't get updated, instead a /etc/letsencrypt/live/example.org-0001/ directory is created with symlinks to the new files — is this to be expected?

I also still have the issues from #757 (comment) — a config file for the domain in /etc/letsencrypt/configs is no longer being created.

@kuba

This comment has been minimized.

Contributor

kuba commented Oct 4, 2015

I would suggest we move the discussion to IRC, not to spam this PR too much, because as far as I can see nothings in the previous comments is relevant (well, apart from the need to reinstall letsencrypt in order for entry points to be updated). Join me #letsencrypt@Freenode.

@chriscroome

This comment has been minimized.

chriscroome commented Oct 4, 2015

@kuba thanks for helping in IRC, just to correct my mistakes above, in case anyone else is trying to follow this, use --renew-by-default not --duplicate and the config is now in /etc/letsencrypt/renewal/. Everything I have tested works :-)

@chriscroome chriscroome referenced this pull request Oct 5, 2015

Closed

Let's encrypt #500

@centminmod

This comment has been minimized.

centminmod commented Oct 5, 2015

@kuba just did latest test with web root authentication on Centmin Mod Nginx server and works wonderfully https://community.centminmod.com/threads/letsencrypt-free-ssl-certificates-with-web-root-authentication-method.4635/ :)

@kuba

This comment has been minimized.

Contributor

kuba commented Oct 12, 2015

Ping, this should be clear to merge.

@bmw?

@pde pde assigned bmw Oct 13, 2015

@pde

This comment has been minimized.

Member

pde commented Oct 13, 2015

Brad will review this; if it needs some help changes in order to not be confusing, those should be in this branch before it lands.

for achall in achalls:
path = self._path_for_achall(achall)
logger.debug("Removing %s", path)
os.remove(path)

This comment has been minimized.

@bmw

bmw Oct 13, 2015

Contributor

Bit of a nit, but perhaps we should delete other folders in challenges.SimpleHTTPResponse.URI_ROOT_PATH if we created them.

This comment has been minimized.

@kuba

kuba Oct 16, 2015

Contributor

Too complicated for this PR. #1003.

@bmw

This comment has been minimized.

Contributor

bmw commented Oct 13, 2015

Other than my one nit, LGTM. Thanks for this @kuba. Very cool approach for solving SimpleHTTP challenges.

@bmw

This comment has been minimized.

Contributor

bmw commented Oct 17, 2015

Agreed to fix nit in another PR, opened #1003.

@bmw bmw merged commit 63c080b into certbot:master Oct 17, 2015

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment