Skip to content

Add support for automatic self-signec cert generation#89

Merged
stilliard merged 1 commit intostilliard:masterfrom
lafriks:feat/autocert
Nov 14, 2018
Merged

Add support for automatic self-signec cert generation#89
stilliard merged 1 commit intostilliard:masterfrom
lafriks:feat/autocert

Conversation

@lafriks
Copy link
Contributor

@lafriks lafriks commented Nov 13, 2018

I'm still testing it but wanted to know what do you think

@stilliard
Copy link
Owner

Hey Lauris,
This looks really useful! Nice work on this 👍

I know it'd help a lot saving people from generating these as it's a pain and i never remember the commands, for testing especially.

I think it'd be worth setting up a test for this too if possible.
The project uses Travis CI to run a test of a normal use but it'd be beneficial to have a ssl test running with this as well or have the default test use this?
What do you think?

@lafriks
Copy link
Contributor Author

lafriks commented Nov 13, 2018

@stilliard test could be added but it would be long one as generating openssl dhparam takes a lot of time

@stilliard
Copy link
Owner

That's true, I'd be ok with adding a minute or so though if you think it can run in that to get coverage for this.
The tests already run pretty slowly, about 5 minutes at times due to building pureftpd from source lol.

@lafriks lafriks changed the title WIP: Add support for automatic self-signec cert generation Add support for automatic self-signec cert generation Nov 13, 2018
@lafriks
Copy link
Contributor Author

lafriks commented Nov 13, 2018

@stilliard I tested and it works as expected. As for tests I could work on that in other PR this weekend

@stilliard
Copy link
Owner

Hey @lafriks
I've reviewed this and it works great! Thanks for the contribution.
While testing it myself i've written a test, I'll get that out soon so the CI version will hopefully use this by default.

@stilliard stilliard merged commit 168e181 into stilliard:master Nov 14, 2018
@lafriks lafriks deleted the feat/autocert branch November 14, 2018 16:00
@stilliard
Copy link
Owner

Hi @lafriks , I'm thinking of changing the openssl dhparam call to use the -dsaparam flag for faster generation. This let's us test this much faster and should be just as good. Would this impact your use case?

Here's the test so far: https://github.com/stilliard/docker-pure-ftpd/compare/ftp-tls-test
Let me know what you think.

@lafriks
Copy link
Contributor Author

lafriks commented Nov 14, 2018

It does lowers security but not in my current use case as I use this to spin up testing instances. For production this probably would not be recommended. Best options would probably add additional env variable that if set than dhparam is called with -dsparam. Something like:

if  [[ "$TLS_USE_DSAPRAM" == "true" ]]; then
    openssl dhparam -dsaparam -out /etc/ssl/private/pure-ftpd-dhparams.pem 2048
else
    openssl dhparam -out /etc/ssl/private/pure-ftpd-dhparams.pem 2048
fi

so for tests you could set env TLS_USE_DSAPRAM to true

@stilliard
Copy link
Owner

Good call, I've added in this flag thanks.
Running final tests now before merging.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants