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

[bug] ChatQnA Security Assessment (It is not a Security Audit) #1220

Open
4 of 6 tasks
dehatideep opened this issue Dec 2, 2024 · 12 comments
Open
4 of 6 tasks

[bug] ChatQnA Security Assessment (It is not a Security Audit) #1220

dehatideep opened this issue Dec 2, 2024 · 12 comments
Assignees
Labels
bug Something isn't working

Comments

@dehatideep
Copy link

Priority

Undecided

OS type

Other (Please let us know in description)

Hardware type

CPU-other (Please let us know in description)

Installation method

  • Pull docker images from hub.docker.com
  • Build docker images from source

Deploy method

  • Docker compose
  • Docker
  • Kubernetes
  • Helm

Running nodes

Single Node

What's the version?

2b2c7ee

Description

ChatQnA security Assessment:
https://docs.google.com/document/d/1df20UOmqJ_30VW5i6MajxXbJn3KhwVHfxYo3oGt2W5o/edit?usp=sharing

Reproduce steps

See the security assessment details. These are based upon code reading.

Raw log

See the security assessment details. These are based upon code reading.
@dehatideep dehatideep added the bug Something isn't working label Dec 2, 2024
@dehatideep
Copy link
Author

@arun-gupta
#1220

@feng-intel
Copy link
Collaborator

@arun-gupta
Do you have comments?

@arun-gupta
Copy link
Contributor

I was having a discussion with folks in the CNCF AI/ML working group. @dehatideep offered to help out with the security review of the OPEA samples. This is a result of that. That is all the context that I can provide.

It would be useful to look at these recommendations and see how we can improve the security of ChatQnA, and possibly other samples.

@xiguiw xiguiw self-assigned this Dec 17, 2024
@xiguiw
Copy link
Collaborator

xiguiw commented Dec 17, 2024

@dehatideep

The assessment is really helpful!
Thank you @dehatideep!

We will review it carefully and take some actions.

BR,
Xigui

@xiguiw
Copy link
Collaborator

xiguiw commented Dec 17, 2024

I was having a discussion with folks in the CNCF AI/ML working group. @dehatideep offered to help out with the security review of the OPEA samples. This is a result of that. That is all the context that I can provide.

It would be useful to look at these recommendations and see how we can improve the security of ChatQnA, and possibly other samples.

Yes, definitely we'll review these recommendations and improve GenAI examples, I believe not only ChatQnA, but also other examples.

@xiguiw
Copy link
Collaborator

xiguiw commented Jan 6, 2025

@dehatideep

We are starting to fix problems pointed out by you.
Here are something I am not sured.

Would you please help for some details in ChatQnA security Assessment:
https://docs.google.com/document/d/1df20UOmqJ_30VW5i6MajxXbJn3KhwVHfxYo3oGt2W5o/edit?usp=sharing?

ChatQnA/docker_compose/install_docker.sh
/install_docker.sh

Is the intention of keeping it readable to all by design? If not it should be restricted to 600, despite the fact that it is a public trusted key.

I did not get your point. Would you please give some more details. Thanks!

  1. ChatQnA/benchmark/accuracy/eval_crud.py

default="http://localhost:8888/v1/chatqna"

Default URL being http suggests non-default may also be http, instead of https? Clients should always know and verify the server’s identity, irrespective of whether the server is local or external.

Do you mean non-default should be https?

@dehatideep
Copy link
Author

@dehatideep

We are starting to fix problems pointed out by you. Here are something I am not sured.

Would you please help for some details in ChatQnA security Assessment: https://docs.google.com/document/d/1df20UOmqJ_30VW5i6MajxXbJn3KhwVHfxYo3oGt2W5o/edit?usp=sharing?

ChatQnA/docker_compose/install_docker.sh /install_docker.sh

Is the intention of keeping it readable to all by design? If not it should be restricted to 600, despite the fact that it is a public trusted key.

I did not get your point. Would you please give some more details. Thanks!
[Deep] You are bringing Docker public GPG key, so that Docker server could be verified and trusted when you add docker repo to source list. Later down the line you add user to 'docker' group. Note that all of it is being done as 'sudo', which suggests that access to key is needed by only root and some privileged user. This suggests that key permission could be 600 or 660 (if a particular group need to access it as well). This restricts the attack surface because any random user bringing sth from docker will fail server verification. THIS IS MERELY DEFENSIVE. Nothing is broken per se here because this key itself is public.
-------- steps in the code ---------

Install prerequisites

sudo apt-get -y install ca-certificates curl

Create the directory for the Docker GPG key

sudo install -m 0755 -d /etc/apt/keyrings

Add Docker's official GPG key

sudo curl -fsSL https://download.docker.com/linux/ubuntu/gpg -o /etc/apt/keyrings/docker.asc

Set permissions for the GPG key

sudo chmod a+r /etc/apt/keyrings/docker.asc

  1. ChatQnA/benchmark/accuracy/eval_crud.py

default="http://localhost:8888/v1/chatqna"

Default URL being http suggests non-default may also be http, instead of https? Clients should always know and verify the server’s identity, irrespective of whether the server is local or external.

Do you mean non-default should be https?

[Deep] If you have a rule (say an iptable firewall, it is not there though) where this port 8888 is accessible only internally (localhost/127.0.0.1), then it is fine but as it exists today, an external host, say 10.11.12.13 may call it like: http://10.11.12.13:8888/v1/chatqna and this opens the door for spoofing/MITM attack because connection is insecure. It should all be https, particularly if this port can be accessed by external machines. Basically you have to create an https server instead of http and that would require a TLS certificates, which you can generate locally. This would be a self-signed cert but at least transport pipe would be opaque.

@xiguiw
Copy link
Collaborator

xiguiw commented Jan 8, 2025

@dehatideep

Thank you for your explanation in detail!

I checked the installation of docker. It seems this script copied from docker website

Just keep it unchanged if there is no potential issue.
Think twice, install docker script should not be part of the OPEA, I would like to remove it.

image

For the https interfaces. Yes, we agree with you. The problem is we cannot sign certificates.

OPEA is a reference design. We plan to put the https interface implementation to the customized implementation stage.
Do you have any suggestions, comments for this?
Thanks!

@dehatideep
Copy link
Author

#1220 (comment)
Docker GPG Key a+r permission is fine. Recommendation was related to reducing the attack surface, given after Docker installation, no one should need to change/update the installation, except root and/or privileged user. This is fine though, given it is only read permission.

Coming to http vs https pipe, your ref design must have https IMO. You can always generate a self-signed certificate for the server with the advice that customized implementation must install their CA Signed cert installed on the server and make Signer cert available to users, just the way Docker public key is brought in the first case above. When server is running default self-signed server, clients can skip cert verification. Note that self-signed cert is also a problem, given it cant be verified, so client doesn't know if connection is spoofed, however it ensures that pipe is opaque to everyone else. I have no opinion if you want to defer it to customized implementation, just add comments somewhere, else people simply clone the repo and the issue remains buried there.

@xiguiw
Copy link
Collaborator

xiguiw commented Jan 13, 2025

@dehatideep
Thank you for your suggestions!

I'll add some comments in README to keep everyone aware and be noticed about https secure connection here.

@arun-gupta
Copy link
Contributor

Tagging @kdruckman for awareness

@xiguiw
Copy link
Collaborator

xiguiw commented Jan 17, 2025

@arun-gupta

GenAIExamples/ChatQnA/docker_compose/install_docker.sh
This bash script installs docker on linux, it's copied from docker website.

I would like to remove it. Any comment?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants