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

Expose elasticsearch.ssl.certificateAuthorities to plugins #116626

Closed
joshdover opened this issue Oct 28, 2021 · 10 comments
Closed

Expose elasticsearch.ssl.certificateAuthorities to plugins #116626

joshdover opened this issue Oct 28, 2021 · 10 comments
Assignees
Labels
bug Fixes for quality problems that affect the customer experience impact:low Addressing this issue will have a low level of impact on the quality/strength of our product. loe:medium Medium Level of Effort NeededFor:Fleet Needed for Fleet Team Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc

Comments

@joshdover
Copy link
Contributor

joshdover commented Oct 28, 2021

Blocks #116620

In order to support security on by default, Fleet needs to provide the fingerprint for Elasticsearch's self-signed CA to Fleet Server and Elastic Agents. To do this, our plugin will need to access the ssl.certificateAuthorities field from the Elasticsearch configuration.

Currently, the config.legacy.globalConfig$ API only provides a few values:

elasticsearch: ['shardTimeout', 'requestTimeout', 'pingTimeout'] as const,

It'd be nice if we could be sure to get the fully resolved value from the ElasticsearchConfig class rather than the field provided by the config schema which may or may not be an array, etc.

@joshdover joshdover added Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc NeededFor:Fleet Needed for Fleet Team labels Oct 28, 2021
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-core (Team:Core)

@pgayvallet
Copy link
Contributor

cc @elastic/kibana-security can you just confirm it would be fine to expose the elasticsearch.ssl.certificateAuthorities (processed) config property to plugins?

@jportner
Copy link
Contributor

jportner commented Oct 28, 2021

cc @elastic/kibana-security can you just confirm it would be fine to expose the elasticsearch.ssl.certificateAuthorities (processed) config property to plugins?

Yes, this is OK, these PEM strings are not secret or sensitive in any way (well the SAN and/or CN values can contain IP addresses or FQDNs for Elasticsearch, so we should not expose these PEM strings to the client -- but fingerprints are fine to expose to the client)

It'd be nice if we could be sure to get the fully resolved value from the ElasticsearchConfig class rather than the field provided by the config schema which may or may not be an array, etc.

Yes, we need to provide the fully resolved/parsed value, because this array is derived from all of elasticsearch.ssl.certificateAuthorities, elastic.ssl.keystore, and elastic.ssl.truststore:

const readKeyAndCerts = (rawConfig: ElasticsearchConfigType) => {
let key: string | undefined;
let keyPassphrase: string | undefined;
let certificate: string | undefined;
let certificateAuthorities: string[] | undefined;
const addCAs = (ca: string[] | undefined) => {
if (ca && ca.length) {
certificateAuthorities = [...(certificateAuthorities || []), ...ca];
}
};
if (rawConfig.ssl.keystore?.path) {
const keystore = readPkcs12Keystore(
rawConfig.ssl.keystore.path,
rawConfig.ssl.keystore.password
);
if (!keystore.key) {
throw new Error(`Did not find key in Elasticsearch keystore.`);
} else if (!keystore.cert) {
throw new Error(`Did not find certificate in Elasticsearch keystore.`);
}
key = keystore.key;
certificate = keystore.cert;
addCAs(keystore.ca);
} else {
if (rawConfig.ssl.key) {
key = readFile(rawConfig.ssl.key);
keyPassphrase = rawConfig.ssl.keyPassphrase;
}
if (rawConfig.ssl.certificate) {
certificate = readFile(rawConfig.ssl.certificate);
}
}
if (rawConfig.ssl.truststore?.path) {
const ca = readPkcs12Truststore(
rawConfig.ssl.truststore.path,
rawConfig.ssl.truststore.password
);
addCAs(ca);
}
const ca = rawConfig.ssl.certificateAuthorities;
if (ca) {
const parsed: string[] = [];
const paths = Array.isArray(ca) ? ca : [ca];
if (paths.length > 0) {
for (const path of paths) {
parsed.push(readFile(path));
}
addCAs(parsed);
}
}
return {
key,
keyPassphrase,
certificate,
certificateAuthorities,
};
};

@joshdover
Copy link
Contributor Author

Yes, this is OK, these PEM strings are not secret or sensitive in any way (well the SAN and/or CN values can contain IP addresses or FQDNs for Elasticsearch, so we should not expose these PEM strings to the client -- but fingerprints are fine to expose to the client)

If we'd like to be on the safe side, we could only expose the CA fingerprints from Core and not the entire CA.

@jportner
Copy link
Contributor

If we'd like to be on the safe side, we could only expose the CA fingerprints from Core and not the entire CA.

Yeah, if that's all we need right now, that might be best.

@joshdover
Copy link
Contributor Author

@planadecu @lukeelmers This is something we'd like to leverage for fixing the onboarding flow for on-prem users in 8.0 (where Elasticsearch configured self-signed certs by default). Is this something we could get in the next few weeks? If not, would you accept a PR to Core from our team to expose this?

@lukeelmers
Copy link
Member

Is this something we could get in the next few weeks? If not, would you accept a PR to Core from our team to expose this?

We are planning our next sprint tomorrow and can get back to you about our capacity then. But I'd be fine to accept a PR to Core if y'all wanted to pick this up.

Based on my understanding of #116620, I think we could consider this a bugfix, and therefore I don't have concerns about adding it even though we have passed the 8.0 Stack FF.

@lukeelmers lukeelmers added the bug Fixes for quality problems that affect the customer experience label Nov 1, 2021
@exalate-issue-sync exalate-issue-sync bot added impact:low Addressing this issue will have a low level of impact on the quality/strength of our product. loe:small Small Level of Effort loe:medium Medium Level of Effort and removed loe:small Small Level of Effort labels Nov 1, 2021
@lukeelmers
Copy link
Member

@joshdover In our planning today we ended up being pretty overloaded, so we aren't going to be able to pick this one up in the next 3 weeks. Still open to accepting a PR if you'd like to do it though!

(We can also earmark it for next sprint, but I don't want to hold y'all up since that is getting closer to solutions FF)

@joshdover
Copy link
Contributor Author

@lukeelmers Thanks for letting me know. I plan to submit a PR in the next week or so.

@joshdover
Copy link
Contributor Author

Going with a different approach, no longer necessary: #120276

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Fixes for quality problems that affect the customer experience impact:low Addressing this issue will have a low level of impact on the quality/strength of our product. loe:medium Medium Level of Effort NeededFor:Fleet Needed for Fleet Team Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants