-
Notifications
You must be signed in to change notification settings - Fork 464
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
[Meta] Migration to secret variables #8610
Comments
Do we need to raise the minimum |
Yes there are a few known issues prior to 8.11.2 that have been fixed or are actively being backported:
It's not unlikely other issues will shake out with more integrations and teams using secrets heavily as part of this effort. If that happens we will provide guidance on Kibana versions with high impact fixes. The absolute minimum version for secrets to function prior to any of these fixes is 8.10.0, but we likely want the best experience possible with these fixes and UX improvements so 8.11.2 is a better target as of right now. |
@kpollich Shouldn't older versions of Kibana (pre-secrets) still accept packages that have fields annotated with |
Yes, packages will still work in older versions of Kibana. Let me add a note about this in the description. |
Yes the secret flag will simply be ignored in older versions of Kibana. Newer stack versions will enjoy the security benefits of better secrets storage, but older stack versions will continue to work as they do now. If a user chooses to upgrade their stack, their policy secrets will be migrated to the new storage scheme whenever they perform an edit/upgrade operation on their package policy for a given package. |
am i understanding correctly? secrets are ignored completely in kibana versions < 8.10.0. also, is there some script or something for identifying potential secret fields? there quite a lot of are the migrations of the packages listed above the responsibility of the teams listed in the headings, or will there be some automation to open PRs for packages/fields identified here? if PRs were opened for all the above, the PR review process would deal with the question of whether they are actually secrets or not, as opposed to having to edit the tables here, then make the changes. thanks |
Correct.
Good point, we will review the list. We are doing it with a different script.
Yes, migration of packages needs to be done by the owners of each package, and there is no available automation. We want each team to review each case, as the decision on considering a variable to be a secret or not is sensitive, and there are some consequences of the migration to secrets.
Yeah, as PRs start to appear we can collectively keep the table updated. |
There are indeed some secret candidates in AWS, description updated with more findings. |
Hi all, we've identified and fixed another bug with secrets that will land in 8.12: elastic/kibana#172061 Thus, I've updated the advisory for the Kibana version constraint in the overview doc to 8.12.0. I'll send this same note out to the email thread as well. |
Updated the table with Cloud Security Posture migrations. The ones that won't be migrated are explained on the linked PR |
Another secrets edge case fixed in 8.12.0: elastic/kibana#173048 + elastic/kibana#173115 |
We are rolling it back , because of elastic/kibana#173718 which is blocking any integration creation on 8.13.0-SNAPSHOT at the moment |
The SEI integrations are currently blocked by elastic/kibana#173718 as well. I didn't test |
Looks like we have a regression in kibana/fleet/agent with rendering secret values. @ebeahan just ran into this (see his comment on the SEI issue here)
I also observed this in practice with the system test in the
|
I also see the same rendering issue in 8.11.3, so it may be an unhandled use case instead of a regression. |
Thanks @taylor-swanson and @ebeahan for raising this. This looks like a bug in Fleet Server's secrets rendering logic when multiple secret values appear in a multiline string. @juliaElastic @michel-laterman would you be able to take a look at this? I'll also start investigating and see what I can come up with. |
@juliaElastic @michel-laterman would also be great to come up with some unit test for this to avoid any further problem. Does it sound good to you? |
I can take a look at this today. |
Reproduced the issue locally and it was really a problem with multiple secret refs in one variable. |
Hey @romulets, I see you marked some fields as |
Hi @jsoriano 👋 Does that mean it's preferred to have |
@jsoriano : Do these validators apply to the fields you have listed above for the Integrations, or to all the fields other than secret fields in the manifest file? |
A brief update on the secrets migration for o11y integration: While we have migrated the large number of integrations to enable secrets, there are handful packages where password is embedded as part of the host URI. For such cases, it would require changes, to ensure that the password field is passed independently and then marked as secret. This may not be a small change (We also need to ensure we do these changes in a non-breaking way), we are working it on case-by-case basis here. |
Regarding Synthetics, here is a list of all the secret fields we have, if they should correspond to the integration. |
Hey folks FYI we have merged some UX improvements to smooth the onboarding experience with secrets on long-lived clusters that have been upgraded across stack versions: elastic/kibana#178045. This will land in 8.14, but but changes don't necessitate changing our target version for secrets. |
@bturquet can we get an update on aws, azure and synthetics? |
Added this issue for synthetics changes: https://github.com/elastic/synthetics-dev/issues/355 |
Problem statement Nearly every package that supports TLS includes a YAML textarea to specify arbitrary settings, some of which are sensitive. Fleet added the ability to mark variables as secrets, and after saving this makes it impossible to read the values back for editing. This is a bad user experience. Solutions
Solution 1 would be best for users but is there some team that would want to add a new UI component for the TLS config options? Solution 3 could be implemented immediately by package developers and would not cause any breaking changes for users. But the UX and DX are inferior compared to solution 1. For this packages would add new secret manifest variables for:
The handlebar templates would keep the existing {{if ssl_certificate_key}}
ssl.key: {{escape_string ssl_certificate_key}}
{{end}}
{{if ssl_key_password}}
ssl.key_password: {{escape_string ssl_key_password}}
{{end}}
{{if ssl}}
ssl:
{{ssl}}
{{end}} |
I think this is the best option from UX perspective to be sure. We can create a new I would definitely be curious to see if there are teams who don't want this, and who value the existing UX around TLS settings. Or, if there are packages that have slight variations on the TLS fields that wouldn't be able to migrate to the new workflow. |
* update format_version to 3.0.2 Align with suggestion from #8610 * set secrets: true Add secret: true to secrets field in manifest * add changelog; bump to 8.14.0 * provide owner.type See https://github.com/elastic/package-spec/blob/ee5775911d6764804548f1f47b479877fbd88d7e/spec/integration/manifest.spec.yml#L285
* update format_version to 3.0.2 Align with suggestion from #8610 * set secrets: true Add secret: true to secrets field in manifest * add changelog; bump to 8.14.0 * provide owner.type See https://github.com/elastic/package-spec/blob/ee5775911d6764804548f1f47b479877fbd88d7e/spec/integration/manifest.spec.yml#L285
I raised this question during the monthly Integration Development Office Hours meeting. There were no objections from the attendees.
AFAIK there are two types of TLS settings usages -- the client context and server context. There are some small variations in the meaning of some fields. For example, in a client context the CA certs indicate which server's to trust, but in the server context this indicates which mTLS clients to trust. The server's CA certs need to be bundled into the |
@jsoriano I see that on Feb 9th you updated that the @elastic/obs-ds-intake-services team has updated the secrets; this is not actually the case. |
The main intention of this issue was to ensure that teams review the secret candidates of the packages they own. Secret candidates are marked as "Migrated" here if the team has reviewed them, by setting I understand this can be confusing in cases like this one, maybe we should stop updating the status with the script and do it manually? The truth is that I haven't run the script in some time. From what I can see, all the fields owned by the Intake Services team have been properly reviewed, the APM fields have |
I think we have reached a satisfactory level of adoption here, and remaining secrets adoption work can be handled on a case-by-case basis. I am closing this issue. Thanks to all the integration teams for their efforts here! |
We are driving an effort to encourage the declaration of secret variables in packages configurations. The use of secret variables highly decreases the risk of leaking secrets in log and configuration files.
Find below a list of variables that we consider secret candidates, sorted by owner.
For these variables, we ask the teams to evaluate if they are actually secrets. If they are, mark them with
secret: true
, if they aren't, mark them withsecret: false
.For packages reviewed, we recommend to update to
format_version: "3.0.2"
to avoid regressions on these fields. Starting on this version the use ofsecret
is required for variables that look like secrets.Secrets are supported since Kibana 8.10.0, but there are known issues till 8.11.2. Packages using
secret: true
will also work in older versions, but secrets won't be used.Due to the known issues, it is recommended to use
kibana.version: ^8.11.2
when using secrets, but older versions can still be used for packages that work in a broad range of versions.As a reminder, these are the consequences of enabling secrets:
We will use this issue to keep track of the progress on this migration.
Issues uncovered during migration efforts
We'll track notable issues uncovered as part of the migration efforts here.
Version target recommendation
All fixes for the above known issues are present in Kibana version 8.12.0. So, the recommended target version constraint for integrations using secrets is
^8.12.0
.Codeowner: @elastic/cloud-security-posture
Secret candidates:
Codeowner: @elastic/security-service-integrations
Secret candidates:
Codeowner: @elastic/obs-ds-hosted-services
Secret candidates:
Codeowner: @elastic/obs-ds-intake-services
Secret candidates:
Codeowner: @elastic/obs-infraobs-integrations
Secret candidates:
Codeowner: @elastic/obs-ux-infra_services-team
https://github.com/elastic/synthetics-dev/issues/355
Secret candidates:
Codeowner: @elastic/sec-deployment-and-devices
Secret candidates:
Codeowner: @elastic/security-service-integrations
Secret candidates:
Codeowner: @elastic/stack-monitoring
Secret candidates:
Codeowner: @elastic/profiling
Secret candidates:
cc @elastic/ecosystem @kpollich @jillguyonnet
The text was updated successfully, but these errors were encountered: