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

Add certutil http command #49827

Merged
merged 4 commits into from
Jan 13, 2020
Merged

Add certutil http command #49827

merged 4 commits into from
Jan 13, 2020

Conversation

tvernum
Copy link
Contributor

@tvernum tvernum commented Dec 4, 2019

This adds a new "http" sub-command to the certutil CLI tool.

The http command generates certificates/CSRs for use on the http
interface of an elasticsearch node/cluster.
It is designed to be a guided tool that provides explanations and
sugestions for each of the configuration options. The generated zip
file output includes extensive "readme" documentation and sample
configuration files for core Elastic products.

This adds a new "http" sub-command to the certutil CLI tool.

The http command generates certificates/CSRs for use on the http
interface of an elasticsearch node/cluster.
It is designed to be a guided tool that provides explanations and
sugestions for each of the configuration options. The generated zip
file output includes extensive "readme" documentation and sample
configuration files for core Elastic products.
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-security (:Security/Network)

@tvernum
Copy link
Contributor Author

tvernum commented Dec 4, 2019

This currently only supports elasticsearch/ and kibana/ output.
I will add beats, logstash & clients at a later date, but I wanted to get the core done first, and then tweak it.

Copy link
Member

@jkakavas jkakavas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did a first round today, I want to look at this once more, from a user's perspective this time as in actually using the tool, and I'll put in additional comments. Impressive amount of user-friendliness here @tvernum :)

Copy link
Member

@azasypkin azasypkin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Kibana related pieces look good to me and everything seems to be in order (apart from minor path related issue). I assume PKI auth between Kibana and Elasticsearch is out of scope of this PR. Is there anything specific you want us to look at @tvernum?

Impressive amount of user-friendliness here @tvernum :)

I want to second this, the guidance is super clear and helpful!

}

static FileType guessFileType(Path path, Terminal terminal) {
// trust the extension for some file-types rather then inspecting the contents
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// trust the extension for some file-types rather then inspecting the contents
// trust the extension for some file-types rather than inspecting the contents

return types.get(0);
default:
if (types.contains(FileType.PEM_KEY)) {
// A Key and something else. Could be a cert + key paired, but we don't support that
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// A Key and something else. Could be a cert + key paired, but we don't support that
// A Key and something else. Could be a cert + key pair, but we don't support that

@tvernum
Copy link
Contributor Author

tvernum commented Dec 23, 2019

I assume PKI auth between Kibana and Elasticsearch is out of scope of this PR.

It is. I'd like to include it down the track, but I suspect that most people who decide to implement mutual TLS have an existing tool set that they're comfortable with, so I'll be prioritising the config other component (beats, language clients, etc) first.

Is there anything specific you want us to look at @tvernum?

If the Kibana setup makes sense (except for my path problem) then the main feedback I'm after is any suggestions for improvements in the text (or other UI feedback such as changing the order of the questions, etc).
I'm sure the current choice words have a lot of my viewpoint built in to them (e.g. using Australian terminology/idioms, or technical blind spots from years of TLS-wrangling), so just telling me if something sounds weird/confusing to you is helpful.

private CertificateTool.CAInfo findExistingCA(Terminal terminal, Environment env) throws UserException {
printHeader("What is the path to your CA?", terminal);

terminal.println("Please enter the full pathname to the Certificate Authority that you wish to");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The path is ( can be ) relevant to the ES_PATH_CONF of the elasticsearch installation. I am not sure if this is worth pointing out but, if users don't enter a full path ( starting with /) the error message can be somewhat confusing.

printHeader("What password do you want for your private keys?", terminal);
char[] password;
if (csr) {
terminal.println("Your private keys will be stored as a PEM formatted file.");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
terminal.println("Your private keys will be stored as a PEM formatted file.");
terminal.println("Your private key(s) will be stored as PEM formatted file(s).");

@tvernum tvernum requested a review from jkakavas January 13, 2020 07:50
@tvernum
Copy link
Contributor Author

tvernum commented Jan 13, 2020

@jkakavas I've addressed most of your comments. If I missed any that you still think are important, let me know and we'll work out a solution.

@jkakavas
Copy link
Member

@jkakavas I've addressed most of your comments. If I missed any that you still think are important, let me know and we'll work out a solution.

Thanks for the iterations Tim. I have no more blocking feedback, I'd be very happy to see this merged

Copy link
Member

@jkakavas jkakavas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@tvernum
Copy link
Contributor Author

tvernum commented Jan 13, 2020

Thanks @jkakavas
I'm going to merge this as is, and work on docs and packaging tests in a followup PR.

@tvernum tvernum merged commit 39a5d75 into elastic:master Jan 13, 2020
tvernum added a commit to tvernum/elasticsearch that referenced this pull request Jan 14, 2020
This adds a new "http" sub-command to the certutil CLI tool.

The http command generates certificates/CSRs for use on the http
interface of an elasticsearch node/cluster.
It is designed to be a guided tool that provides explanations and
sugestions for each of the configuration options. The generated zip
file output includes extensive "readme" documentation and sample
configuration files for core Elastic products.

Backport of: elastic#49827
tvernum added a commit that referenced this pull request Jan 14, 2020
This adds a new "http" sub-command to the certutil CLI tool.

The http command generates certificates/CSRs for use on the http
interface of an elasticsearch node/cluster.
It is designed to be a guided tool that provides explanations and
sugestions for each of the configuration options. The generated zip
file output includes extensive "readme" documentation and sample
configuration files for core Elastic products.

Backport of: #49827
SivagurunathanV pushed a commit to SivagurunathanV/elasticsearch that referenced this pull request Jan 23, 2020
This adds a new "http" sub-command to the certutil CLI tool.

The http command generates certificates/CSRs for use on the http
interface of an elasticsearch node/cluster.
It is designed to be a guided tool that provides explanations and
sugestions for each of the configuration options. The generated zip
file output includes extensive "readme" documentation and sample
configuration files for core Elastic products.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants