Skip to content

feat: data encryption support more plugins#8487

Merged
spacewander merged 8 commits into
apache:masterfrom
tzssangglass:gde2
Dec 19, 2022
Merged

feat: data encryption support more plugins#8487
spacewander merged 8 commits into
apache:masterfrom
tzssangglass:gde2

Conversation

@tzssangglass

Copy link
Copy Markdown
Member

Description

Fixes #8407

Checklist

  • I have explained the need for this PR and the problem it solves
  • I have explained the changes or the new features added to this PR
  • I have added tests corresponding to this change
  • I have updated the documentation to reflect this change
  • I have verified that this change is backward compatible (If not, please discuss on the APISIX mailing list first)

@tzssangglass
tzssangglass marked this pull request as ready for review December 9, 2022 02:32
@soulbird

soulbird commented Dec 9, 2022

Copy link
Copy Markdown
Contributor

It is best to update the documentation of each plugin to indicate the fields that will be encrypted.

@tzssangglass

Copy link
Copy Markdown
Member Author

It is best to update the documentation of each plugin to indicate the fields that will be encrypted.

done

Comment thread apisix/plugin.lua Outdated
Comment thread docs/en/latest/plugin-develop.md Outdated
Comment thread docs/zh/latest/plugins/openid-connect.md Outdated
Comment thread docs/en/latest/plugins/openid-connect.md Outdated
Comment thread apisix/plugin.lua
return
end

-- we only support two levels

@juststillthinking juststillthinking Dec 13, 2022

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

too many limitations, maybe we should optimize this feature later when we have time

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why add this limitation, can we parse the conf recursively

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I'd like to optimize when the schema must be expressed in more than two levels.
For now, I think two levels are enough(In fact, I hate nesting more than two levels in a schema, it's unreadable.)

| expires | number | False | `7200` | Expiration time in seconds of the CSRF cookie. Set to `0` to skip checking expiration time. |
| key | string | True | | Secret key used to encrypt the cookie. |

NOTE: `encrypt_fields = {"key"}` is also defined in the schema, which means that the field will be stored encrypted in etcd. See [encrypted storage fields](../plugin-develop.md#encrypted-storage-fields).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The field will be encrypted and stored in etcd ?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

yes, what is stored in etcd is the encrypted

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is the sentence better The field will be encrypted and stored in etcd

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I guess there is no difference? For this field, the only encrypted state is in etcd. They are decrypted when APISIX is running and when fetched via the admin API.
I think the way I wrote it better conveys the idea that it is only encrypted in etcd.

soulbird
soulbird previously approved these changes Dec 14, 2022
Comment thread apisix/plugin.lua Outdated
Comment thread docs/en/latest/plugins/key-auth.md
Comment thread docs/zh/latest/plugins/key-auth.md
Comment thread docs/zh/latest/plugins/key-auth.md Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Proposal: APISIX Supports Global Data Encryption

4 participants