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

Build Safe SSL config at correct stage #954

Merged
merged 4 commits into from May 6, 2023

Conversation

TinCanTech
Copy link
Collaborator

@TinCanTech TinCanTech commented May 6, 2023

This patch set fixes three inter-linked problems.

Problem-1:

When using EC algorithms, it was found that EASYRSA_REQ_CN was ignored by easyrsa because the value in the config took priority.

The config built was built too soon, before EASYRSA_REQ_CN had been assigned, which meant that building a CA would result in a CA CommonName of ChangeMe, instead of Easy-RSA CA.

This is partially resolved by not using easyrsa_openssl() meta-wrapper when calling verify_algo_params() but calling EASYRSA_OPENSSL directly.

This also, resolves an issue that if incorrect algorithm settings were chosen then easyrsa_openssl() would die with a misleading error, instead of verify_algo_params() exiting with the correct error message.

Problem-2:

Diagnosing Problem-1 exposed an issue with the SSL config file. When EASYRSA_ALGO was not set to rsa then EASYRSA_KEY_SIZE was also not set. This resulted in an error in the config file for empty vaue for default_bits.

This is resolved by ALWAYS assigning a value to EASYRSA_KEY_SIZE, regardless of algorithm.

Problem-3:

The first time the Safe SSL config is created then working_safe_ssl_conf is set and further invocations of easyrsa_openssl() will use the same SSL config file. A new Safe SSL config file is not required once it has been built.

The assignment of working_safe_ssl_conf was set too late and resulted in it being set even if the Safe SSL config file had not been built.

This is resolved by moving the assignment to the correct place.

A secondary check is also now in place, at the end of verify_working_env(), to ensure that working_safe_ssl_conf remains unset until the issued command is executed. eg. build_ca()`.

Additional:

Move the use of escape_hazard(), use the same controlling code as easyrsa_rewrite_ssl_config().

EASYRSA_KEY_SIZE is present in the SSL config file, therefore,
it MUST always be set, regardless of EASYRSA_ALGO in use.

Signed-off-by: Richard T Bonhomme <tincantech@protonmail.com>
verify_algo_params() expects errors when settings are not corrrect.
Therefore, is must not use easyrsa_openssl() meta-wrapper, which would
error out with a misleading error message.

Fixing this also ensures that the SAFE SSL config is not built prior
to EASYRSA_REQ_CN being set.

Signed-off-by: Richard T Bonhomme <tincantech@protonmail.com>
Saving the name of the fully expanded Safe SSL config means that this
config file only has to be built once.

The assignment of working_safe_ssl_conf, which signifies that a Safe
SSL config has already been created, was set too late, which caused
it to be set even if the Safe SSL config had not been created.

Also, include a final check in verify_working_env() to ensure that
working_safe_ssl_conf has not been set prior to executing the issued
command, eg. build-ca.

Also, improve verbose messages and comments.

Signed-off-by: Richard T Bonhomme <tincantech@protonmail.com>
Move escape_hazard() to use the same control as easyrsa_rewrite_ssl_config().

Signed-off-by: Richard T Bonhomme <tincantech@protonmail.com>
@TinCanTech TinCanTech merged commit 7d310e4 into OpenVPN:master May 6, 2023
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
1 participant