Skip to content
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

virtualmin does not know how to concatenate certificate #971

Closed
aqueos opened this issue Nov 28, 2024 · 7 comments
Closed

virtualmin does not know how to concatenate certificate #971

aqueos opened this issue Nov 28, 2024 · 7 comments

Comments

@aqueos
Copy link

aqueos commented Nov 28, 2024

hi there,

yesterday i had a user changing a certificate and he put a intermediate CA without a "\n" at the end.

When virtualmin created the ssl.combined it ends up as

-----END CERTIFICATE----------BEGIN CERTIFICATE-----
vs
-----END CERTIFICATE-----
-----BEGIN CERTIFICATE-----

and so apache crash (because apache is stupid it put the error not in the global error log but in the website log...).

Following this i have 2 suggestions:

1/ add a \n for cert/key/int ca so this does not happen or have a way to correct this

i dont think that ssl lib mind extra blanks line so easy way to prevent this issue.
If not a simple regex can do it. Here we use haproxy in front so internaly we use

sed -i 's/\-\-\-\-\-\-\-\-\-\-/-----\n-----/g' "/etc/haproxy/ssl/${VIRTUALSERVER_ID}.pem";

to prevent this. A similar perl regex should take care of this .

2/ i dont know why intermediate certificate CA is separated in a specific tab from cert/key. in virtualmin.

When you renew a cert the intermediate CA is allways comming with the key and the certificate.
You have to put the 3 at the same time, each time.
Therefor having it separated means you have to change the CA, then change the cert and key.
This will need 2 actions and 2 restarts of the services and, also, there is a period when the certificate is invalid as the chain is wrong.

It doesn't seems to make sense to have it separated did it ?
Should it not be a third text in the form with the same option as the key/cert to update it or not instead of a separate tab ?

best regards,
Ghislain ADNET.

ps: and yes the title is for clickbait ;p

@iliajie
Copy link
Collaborator

iliajie commented Nov 28, 2024

Hello,

Thank you for reaching out to us!

yesterday i had a user changing a certificate and he put a intermediate CA without a "\n" at the end.

Sorry, I can’t reproduce it. Could you provide the exact steps to replicate the issue?

@jcameron
Copy link
Collaborator

Yeah it's hard to see how this could happen, as we always add a newline when combining the cert and CA, even if it's not needed : https://github.com/virtualmin/virtualmin-gpl/blob/master/feature-ssl.pl#L3214

@aqueos
Copy link
Author

aqueos commented Nov 29, 2024

humm your right it seems that i cannot reproduce this myself. I dont know how the user did it.
he use vscode to copy paste i was wondering if it was not a mac/windows encoding of newline issue there

Dam i got too quick on this one but he only used the panel and the result is clearly a broken combined cert.....
He said he did not change the intermediate CA, just the cert but if you allways added the newline then it cannot be that an old one was broken as you add the line at the creation of the combined not in the file itself... I dont know how it happened.

sorry for the noise then i dont find a way to reproduce i should have before reporting this, too much trust on this user sorry this is bad policy i should allways do the test myself. :(

What do you think about the second remark on the fact that the intermediate could be regrouped with cert/key and not apart on another tab to prevent 2 restart and a small but existing downtime on the ssl validity chain, any thought on this one ?

best regards,
Ghislain.

@jcameron
Copy link
Collaborator

What do you think about the second remark on the fact that the intermediate could be regrouped with cert/key and not apart on another tab to prevent 2 restart and a small but existing downtime on the ssl validity chain, any thought on this one ?

That's a good point! Is the specific case you're running into when you are uploading a cert and key from a new SSL CA, and so also need to update the CA cert at the same time?

@aqueos
Copy link
Author

aqueos commented Dec 1, 2024

When you order a certificate you have the key you created and the CA send

  • the cert

  • the intermediate CA

  • the CA root which is not needed as it is in the browser

    To have a complete TLS chain that is valid you need the 3 but now in virtualmin you cannot update them at the same time so you must:

  • update the intermediate CA, breaking the current chain if the renewed cert is not the same CA or the CA intermediate is out of validity and the new one use another ( or a security issue made them change it).

  • then update the cert/key

    I dont know a case when you dont have to do this.

    Of course if you take the new cert on the same CA the intermediate COULD not change for a few years but the time needed to check that is a lot more than to simlply put the one they gave you directly so nobody do that :)

best regards,
Ghislain.

@jcameron
Copy link
Collaborator

jcameron commented Dec 1, 2024

Ok I'll take a look into improving this part of the UI, likely by combining the cert/key and CA forms into one

@jcameron
Copy link
Collaborator

Ok in the next release, the cert, key and CA cert will all be settable on a single page.

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

No branches or pull requests

3 participants