Skip to content

Support stored authentication headers prior to version 6.7#92221

Merged
elasticsearchmachine merged 4 commits intoelastic:mainfrom
ywangd:support-v6-stored-authc-header
Dec 12, 2022
Merged

Support stored authentication headers prior to version 6.7#92221
elasticsearchmachine merged 4 commits intoelastic:mainfrom
ywangd:support-v6-stored-authc-header

Conversation

@ywangd
Copy link
Copy Markdown
Member

@ywangd ywangd commented Dec 8, 2022

Officially Elasticsearch is compatible with last major at data level. Therefore v8 is not compatible with v6. However we don't have a guided migration path for stored authentication headers, e.g. upgrade assistant does not do anything for them. Therefore it is more helpful and user friendly for v8 to support v6 stored authentication headers.

This PR adds back the version conditional logic removed in #41185 along with tests.

@ywangd
Support stored authentication headers prior to version 6.7
c3815cf 
Officially Elasticsearch is compatible with last major at data level.
Therefore v8 is not compatible with v6. However we don't have a guided
migration path for stored authentication headers, e.g. upgrade assistant
does not do anything for them. Therefore it is more helpful and user
friendly for v8 to support v6 stored authentication headers.

This PR adds back the version conditional logic removed in elastic#41185 along
with tests.
@ywangd ywangd added >bug :Security/Authentication Logging in, Usernames/passwords, Realms (Native/LDAP/AD/SAML/PKI/etc) v8.6.0 v8.7.0 labels Dec 8, 2022
@ywangd ywangd requested a review from tvernum December 8, 2022 03:43
@elasticsearchmachine elasticsearchmachine added the Team:Security Meta label for security team label Dec 8, 2022
@elasticsearchmachine
Copy link
Copy Markdown
Collaborator

elasticsearchmachine commented Dec 8, 2022

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

@elasticsearchmachine
Copy link
Copy Markdown
Collaborator

elasticsearchmachine commented Dec 8, 2022

Hi @ywangd, I've created a changelog YAML for you.

@ywangd ywangd requested a review from jakelandis December 8, 2022 08:15
droberts195 added a commit to droberts195/elasticsearch that referenced this pull request Dec 12, 2022
@droberts195
[ML] Adding a known issue for the ML datafeed auth header problem
2f9102a 
Due to elastic#41185 datafeeds created prior to 7.0 and not updated since
then have unparseable authorization headers in 8.x. In 8.0-8.3 this
could easily be a non-issue, as such datafeeds were likely forgotten
leftovers and never run. Even if it was a problem, only the datafeed
of interest would need updating with any urgency.

Due to elastic#87884 datafeeds with authorization headers older than 7.0
prevent _all_ datafeeds being listed in 8.4 and 8.5. This in turn
breaks the anomaly detection job management section of the ML UI.

The problem is alleviated by elastic#92168 and fixed by elastic#92221, but we
should warn users that the problem exists in 8.4.0-8.5.3 inclusive.
@droberts195 droberts195 mentioned this pull request Dec 12, 2022
Merged
ywangd
ywangd commented Dec 12, 2022
View reviewed changes
...ck/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authc/Authentication.java
Comment on lines +130 to +136
if (version.onOrAfter(VERSION_AUTHENTICATION_TYPE)) {
type = AuthenticationType.values()[in.readVInt()];
metadata = in.readMap();
} else {
type = AuthenticationType.REALM;
metadata = Map.of();
}
Copy link
Copy Markdown
Member Author

@ywangd ywangd Dec 12, 2022

Choose a reason for hiding this comment

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

We should have corresponding logic on the writeTo side. But I could not decide whether it should be:

  1. Simply throwing an error if version is 6.x
  2. Drop type and metadata if version is 6.x
  3. A combination of the two, ie. drop type and metadata if type is REALM and metadata is empty, otherwise throw.

I think the answer might be 3?

Copy link
Copy Markdown
Member Author

@ywangd ywangd Dec 12, 2022

Choose a reason for hiding this comment

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

I pushed a commit for option 2. Because option 1 is not viable as it prevents authentication to be sent across wire. Option 3 is likely overkill and hard to test (need to hack an invalid 6.x header). Please consider it as a strawman implementation. It is easier to discuss with something concrete.

@albertzaharovits albertzaharovits self-requested a review December 12, 2022 11:41
@ywangd ywangd force-pushed the support-v6-stored-authc-header branch from 4eb9cdd to 9b3a82c Compare December 12, 2022 12:33
albertzaharovits
albertzaharovits approved these changes Dec 12, 2022
View reviewed changes
Copy link
Copy Markdown
Contributor

@albertzaharovits albertzaharovits left a comment

Choose a reason for hiding this comment

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

LGTM

@ywangd ywangd added auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) auto-backport-and-merge labels Dec 12, 2022
@ywangd
Copy link
Copy Markdown
Member Author

ywangd commented Dec 12, 2022

@elasticmachine run elasticsearch-ci/docs

@elasticsearchmachine elasticsearchmachine merged commit 265f32a into elastic:main Dec 12, 2022
@ywangd ywangd deleted the support-v6-stored-authc-header branch December 12, 2022 14:23
@ywangd ywangd mentioned this pull request Dec 12, 2022
Merged
ywangd added a commit to ywangd/elasticsearch that referenced this pull request Dec 12, 2022
@ywangd
Support stored authentication headers prior to version 6.7 (elastic#9…
766a13b 
…2221)

Officially Elasticsearch is compatible with last major at data level.
Therefore v8 is not compatible with v6. However we don't have a guided
migration path for stored authentication headers, e.g. upgrade assistant
does not do anything for them. Therefore it is more helpful and user
friendly for v8 to support v6 stored authentication headers.

This PR adds back the version conditional logic removed in elastic#41185 along
with tests.
elasticsearchmachine pushed a commit that referenced this pull request Dec 12, 2022
@ywangd
Support stored authentication headers prior to version 6.7 (#92221) (#…
24b2485 
…92295)

Officially Elasticsearch is compatible with last major at data level.
Therefore v8 is not compatible with v6. However we don't have a guided
migration path for stored authentication headers, e.g. upgrade assistant
does not do anything for them. Therefore it is more helpful and user
friendly for v8 to support v6 stored authentication headers.

This PR adds back the version conditional logic removed in #41185 along
with tests.
droberts195 added a commit that referenced this pull request Dec 13, 2022
@droberts195
[ML] Adding a known issue for the ML datafeed auth header problem (#9…
9fe9020 
…2274)

Due to #41185 datafeeds created prior to 7.0 and not updated since
then have unparseable authorization headers in 8.x. In 8.0-8.3 this
could easily be a non-issue, as such datafeeds were likely forgotten
leftovers and never run. Even if it was a problem, only the datafeed
of interest would need updating with any urgency.

Due to #87884 datafeeds with authorization headers older than 7.0
prevent _all_ datafeeds being listed in 8.4 and 8.5. This in turn
breaks the anomaly detection job management section of the ML UI.

The problem is alleviated by #92168 and fixed by #92221, but we
should warn users that the problem exists in 8.4.0-8.5.3 inclusive.
droberts195 added a commit to droberts195/elasticsearch that referenced this pull request Dec 13, 2022
@droberts195
[ML] Adding a known issue for the ML datafeed auth header problem (el…
0645121 
…astic#92274)

Due to elastic#41185 datafeeds created prior to 7.0 and not updated since
then have unparseable authorization headers in 8.x. In 8.0-8.3 this
could easily be a non-issue, as such datafeeds were likely forgotten
leftovers and never run. Even if it was a problem, only the datafeed
of interest would need updating with any urgency.

Due to elastic#87884 datafeeds with authorization headers older than 7.0
prevent _all_ datafeeds being listed in 8.4 and 8.5. This in turn
breaks the anomaly detection job management section of the ML UI.

The problem is alleviated by elastic#92168 and fixed by elastic#92221, but we
should warn users that the problem exists in 8.4.0-8.5.3 inclusive.
elasticsearchmachine pushed a commit that referenced this pull request Dec 13, 2022
@droberts195
[ML] Adding a known issue for the ML datafeed auth header problem (#9…
5d47733 
…2274) (#92318)

Due to #41185 datafeeds created prior to 7.0 and not updated since
then have unparseable authorization headers in 8.x. In 8.0-8.3 this
could easily be a non-issue, as such datafeeds were likely forgotten
leftovers and never run. Even if it was a problem, only the datafeed
of interest would need updating with any urgency.

Due to #87884 datafeeds with authorization headers older than 7.0
prevent _all_ datafeeds being listed in 8.4 and 8.5. This in turn
breaks the anomaly detection job management section of the ML UI.

The problem is alleviated by #92168 and fixed by #92221, but we
should warn users that the problem exists in 8.4.0-8.5.3 inclusive.
@droberts195 droberts195 mentioned this pull request Dec 13, 2022
Merged
droberts195 added a commit to droberts195/elasticsearch that referenced this pull request Dec 13, 2022
@droberts195
[8.5] [ML] Adding a known issue for the ML datafeed auth header problem
38cae4d 
Due to elastic#41185 datafeeds created prior to 7.0 and not updated since
then have unparseable authorization headers in 8.x. In 8.0-8.3 this
could easily be a non-issue, as such datafeeds were likely forgotten
leftovers and never run. Even if it was a problem, only the datafeed
of interest would need updating with any urgency.

Due to elastic#87884 datafeeds with authorization headers older than 7.0
prevent _all_ datafeeds being listed in 8.4 and 8.5. This in turn
breaks the anomaly detection job management section of the ML UI.

The problem is alleviated by elastic#92168 and fixed by elastic#92221, but we
should warn users that the problem exists in 8.4.0-8.5.3 inclusive.

Backport of elastic#92274
droberts195 added a commit to droberts195/elasticsearch that referenced this pull request Dec 13, 2022
@droberts195
[8.4] [ML] Adding a known issue for the ML datafeed auth header problem
37499f6 
Due to elastic#41185 datafeeds created prior to 7.0 and not updated since
then have unparseable authorization headers in 8.x. In 8.0-8.3 this
could easily be a non-issue, as such datafeeds were likely forgotten
leftovers and never run. Even if it was a problem, only the datafeed
of interest would need updating with any urgency.

Due to elastic#87884 datafeeds with authorization headers older than 7.0
prevent _all_ datafeeds being listed in 8.4 and 8.5. This in turn
breaks the anomaly detection job management section of the ML UI.

The problem is alleviated by elastic#92168 and fixed by elastic#92221, but we
should warn users that the problem exists in 8.4.0-8.5.3 inclusive.

Backport of elastic#92274
@droberts195 droberts195 mentioned this pull request Dec 13, 2022
Merged
elasticsearchmachine pushed a commit that referenced this pull request Dec 13, 2022
@droberts195
[8.5] [ML] Adding a known issue for the ML datafeed auth header probl…
a9a3c27 
…em (#92323)

Due to #41185 datafeeds created prior to 7.0 and not updated since
then have unparseable authorization headers in 8.x. In 8.0-8.3 this
could easily be a non-issue, as such datafeeds were likely forgotten
leftovers and never run. Even if it was a problem, only the datafeed
of interest would need updating with any urgency.

Due to #87884 datafeeds with authorization headers older than 7.0
prevent _all_ datafeeds being listed in 8.4 and 8.5. This in turn
breaks the anomaly detection job management section of the ML UI.

The problem is alleviated by #92168 and fixed by #92221, but we
should warn users that the problem exists in 8.4.0-8.5.3 inclusive.

Backport of #92274
droberts195 added a commit that referenced this pull request Dec 13, 2022
@droberts195
[8.4] [ML] Adding a known issue for the ML datafeed auth header probl…
e0a7765 
…em (#92324)

Due to #41185 datafeeds created prior to 7.0 and not updated since
then have unparseable authorization headers in 8.x. In 8.0-8.3 this
could easily be a non-issue, as such datafeeds were likely forgotten
leftovers and never run. Even if it was a problem, only the datafeed
of interest would need updating with any urgency.

Due to #87884 datafeeds with authorization headers older than 7.0
prevent _all_ datafeeds being listed in 8.4 and 8.5. This in turn
breaks the anomaly detection job management section of the ML UI.

The problem is alleviated by #92168 and fixed by #92221, but we
should warn users that the problem exists in 8.4.0-8.5.3 inclusive.

Backport of #92274
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@albertzaharovits albertzaharovits albertzaharovits approved these changes

@tvernum tvernum Awaiting requested review from tvernum

@jakelandis jakelandis Awaiting requested review from jakelandis

Assignees

No one assigned

Labels

auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) >bug :Security/Authentication Logging in, Usernames/passwords, Realms (Native/LDAP/AD/SAML/PKI/etc) Team:Security Meta label for security team v8.6.0 v8.7.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@ywangd @elasticsearchmachine @albertzaharovits