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

easyrsa gen-req is overwriting --req-cn option with default value #668

Closed
tomdev opened this issue Aug 30, 2022 · 5 comments · Fixed by #682
Closed

easyrsa gen-req is overwriting --req-cn option with default value #668

tomdev opened this issue Aug 30, 2022 · 5 comments · Fixed by #682

Comments

@tomdev
Copy link

tomdev commented Aug 30, 2022

Before easyrsa v3.0.9 it was possible to pass in a custom common name with the --req-cn option flag:

easyrsa --batch --req-cn=myCommonName gen-req fileName nopass

This functionality is broken as of easyrsa v3.0.9, and the common name will now always get set to the passed in "fileName". This is unexpected behaviour.

Bug

This bug was introduced in v3.0.9: https://github.com/OpenVPN/easy-rsa/blob/v3.0.9/easyrsa3/easyrsa#L1097 in commit a52d6c5

Previously, the variable EASYRSA_REQ_CN would only get set to the default value of the fileName when not in batch mode, but that check has been removed. I assume this was erroneously removed, as the code was updated to support EASYRSA_REQ_CN for all operation, and not just batch mode. The code did not make clear the EASYRSA_REQ_CN could have been set by a user already.

Behaviour

Running the following command:

easyrsa --batch --req-cn=myCommonName gen-req fileName nopass

results in a request with common name set to "fileName".

Expected behaviour

Running the following command:

easyrsa --batch --req-cn=myCommonName gen-req fileName nopass

results in a request with common name set to "myCommonName".

Fix

Will link a pull request to this issue that fixes the bug.

@TinCanTech
Copy link
Collaborator

Sure, I can see the issue. The problem then becomes inconsistent behavior between batch mode and normal mode.

I probably went too far trying to resolve related issue #456

@TinCanTech TinCanTech self-assigned this Aug 30, 2022
@TinCanTech TinCanTech added the development Possible changes label Aug 30, 2022
@TinCanTech TinCanTech added this to the v3.1.1-RC1 milestone Aug 30, 2022
@tomdev
Copy link
Author

tomdev commented Aug 30, 2022

Related Pull Request: #669

@tomdev
Copy link
Author

tomdev commented Aug 30, 2022

Sure, I can see the issue. The problem then becomes inconsistent behavior between batch mode and normal mode.

I probably went too far trying to resolve related issue #456

Currently I don't see any different behaviour in the gen_req() function that related to using the EASYRSA_REQ_CN either in batch or regular mode.

Should there be any distincation? Based on the documentation it appears --req-cn was a required option when in batch mode, but the code is not reflecting that anymore.

"EASYRSA_REQ_CN (CLI: --req-cn) - default CN, necessary to set in BATCH mode"
From: https://easy-rsa.readthedocs.io/en/latest/advanced/

Is that something we'd need to implement again? Happy to help out.

@TinCanTech
Copy link
Collaborator

TinCanTech commented Aug 30, 2022

I think that you are correct and my earlier change was based on my misunderstanding.

Also, #669 looks good, thanks! Although, I may have to link the behavior to --batch, as it was before.

@TinCanTech
Copy link
Collaborator

TinCanTech commented Aug 30, 2022

EASYRSA_REQ_CN (CLI: --req-cn) - default CN, necessary to set in BATCH mode"
From: https://easy-rsa.readthedocs.io/en/latest/advanced/

Is that something we'd need to implement again? Happy to help out.

I believe the help-text is factually incorrect.

As opposed to requiring --req-cn in Batch mode, which is inaccurate;
The correct text should read: Use of --req-cn requires Batch mode.

Yet another example of how EasyRSA has abused Batch mode.

At tomdev, you have already helped by submitting this issue, thanks.

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

Successfully merging a pull request may close this issue.

2 participants