-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
feat: set default value of ssl_trusted_certificate
to system
#11993
base: master
Are you sure you want to change the base?
feat: set default value of ssl_trusted_certificate
to system
#11993
Conversation
system
system
ssl_trusted_certificate
to system
58287b9
…p/apisix into revolyssup/system-default-cert
apisix/cli/ops.lua
Outdated
@@ -529,7 +531,7 @@ Please modify "admin_key" in conf/config.yaml . | |||
end | |||
|
|||
local combined_cert_filepath = yaml_conf.apisix.ssl.ssl_trusted_combined_path | |||
or "/usr/local/apisix/conf/ssl_trusted_combined.pem" | |||
or profile.apisix_home .. "/conf/ssl_trusted_combined.pem" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why don't we use the system's CA file directly when it is set to system
, but use the one processed by APISIX itself?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we do use the system CA file. This path is to store the combined cert only.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
okay i got what you were saying. let me make that change
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Specifically, we should hardcode some list of paths based on the default system certificate paths of some of the major Linux distributions (Debian, Redhat) and find and use them at boot time.
As currently designed, this file will be copied and generated in /usr/local/apisix, a place where permissions management is typically more complex than in /etc/ssl/xxx.
This introduces security risks to APISIX that are not otherwise within its administrative purview. This is bad if a malicious user is not authorized to modify /etc and it can modify /usr/local/apisix.
A TLS CA file silently copied by APISIX to verify an authoritative certificate chain could be corrupted by a malicious user, unbeknownst to APISIX itself or the user, jeopardizing the user's system.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As long as the system's own ca bundle is used, we just comply with whatever changes it makes, even though it may be broken as well, and that's not APISIX's problem.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
start with a blank file, add their own self-signed CA to the end of the file, and specify a custom file directory inside the APISIX configuration file
In this case, the usage cost will increase, especially for containerized deployment. Users need to build their own image based on the APISIX container image or write corresponding shell scripts to complete this task before starting the APISIX image. Both methods are somewhat troublesome.
Imagine if the etcd deployed by users has mTLS enabled, in order to use custom CA certificates and system certificates simultaneously, they would need to handle the above steps themselves when deploying APISIX, which becomes inconvenient.
he user will know what they are doing and what the risks are, this cannot be a black box.
We can explain the actual functions of ssl_trusted_certificate
and ssl_trusted_combined_path
in the documentation for config.yaml
. Just as APISIX generates nginx.conf
at startup, we can explain the function of those temporary files.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As currently designed, this file will be copied and generated in /usr/local/apisix, a place where permissions management is typically more complex than in /etc/ssl/xxx.
yes, we should put it in $profile.apisix_home/conf
like nginx.conf
, using /usr/local/apisix
is a mistake. cc @Revolyssup
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In this case, the usage cost will increase, especially for containerized deployment. Users need to build their own image based on the APISIX container image or write corresponding shell scripts to complete this task before starting the APISIX image. Both methods are somewhat troublesome.
Imagine if the etcd deployed by users has mTLS enabled, in order to use custom CA certificates and system certificates simultaneously, they would need to handle the above steps themselves when deploying APISIX, which becomes inconvenient.
I don't agree with the idea that costs will go up, if a user decides to turn on mTLS and the server doesn't use trusted certificates, it should handle any resulting extra work itself, which should follow any debian or redhat guidelines that exist as well, more people will learn about standard packages (ca-certificates) than will learn and understand us weird black box customization mechanisms.
For debian, first make sure the ca-certificates package is installed on your system, then place your own CA certificates in /usr/local/share/ca-certificates (the container will need to be mounted), and then just run sudo update-ca-certificates and those customized certificates will be automatically added to the root certificate package by the script.
For containers, this can be done by custom ENTRYPOINT or CMD, such as
docker run -d apache/apisix:3.11.0-debian sh -c "update-ca-certificates && apisix start"
(just an example), this doesn't require a script. I'm doing this for a couple containers and it's working fine.
The difficulties you mention should be improved by some better docker entrypoint scripts, for example, by providing some directory to place and execute user's custom scripts before startup, as many are doing https://docs.linuxserver.io/general/container-customization/#custom-scripts. this is outside the scope of APISIX, and it's enough for APISIX to ensure that its behavior is safe.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I maintain my viewpoint that, from my perspective, the design of allowing ssl_trusted_certificate
to support multiple files is very intuitive.
Based on this, in order for nginx/openresty's ssl_trusted_certificate
to correctly read multiple files, we have no choice but to merge the files.
This logic is very simple and can be clearly explained in the description of this configuration, so I don't consider it a black-box design.
It is merely an encapsulation and enhancement of nginx/openresty's ssl_trusted_certificate
.
@membphis @moonming @juzhiyuan Need more discussion here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not a rebuttal to the "simple wrapping" statement, it is simple wrapping. The problem is that we shouldn't do it at all.
Any action other than reading the system certificate file (when this value is set to system
) is outside the scope of the APISIX project. Modifying the system certificate chain is a matter for the user's system administrators to deal with, not ours.
In short, we do not endorse or take responsibility for any potential security risks from this behavior.
If the file being copied is controlled by APISIX in terms of content and lifecycle, the responsibility for ensuring its security is APISIX's. With great power comes great responsibility.
Unless the user reads the code, no one will know that we have copied the file and placed it in the unintended unknown, and I think it's fair for me to call it a black box. This is a serious discussion for any security and trust issues.
System administrators are familiar with the way certificates are managed in the Linux distributions their companies use. The ca-certificates
is a well known software package, and there are many documents on how to work with it. And what about APISIX's documentation of this? I don't think there's more than that on the config file.
There is also more than one way to modify the certificate chain, as I've already mentioned.
- Follow the Linux guide and use the
update-ca-certificates
command - Point
ssl_trusted_certificate
to a CA certificate bundle file that you know exactly what it is and want to trust.
It's intuitive, and whether users are more familiar with ca-certificates or nginx they will know what should be done.
…p/apisix into revolyssup/system-default-cert
|
||
yaml_conf.apisix.ssl.ssl_trusted_certificate = combined_cert_filepath | ||
yaml_conf.apisix.ssl.ssl_trusted_certificate = trusted_certs_path | ||
elseif yaml_conf.apisix.ssl.ssl_trusted_certificate ~= nil then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This condition is redundant, the default value for ssl_trusted_certificate
has already been set above.
Description
When testing AI plugins, we found that
ssl_trusted_certificate
should be set tosystem
, otherwise APISIX will continuously report errors when accessing external AI services.Based on the discussion below, this PR also removes support for combining multiple certs
Fixes # (issue)
Checklist