envoy
envoy copied to clipboard
envoyproxy
Add bcrypt hashes support to basic_auth filter
Currently the auth_basic filter only supports SHA hashed passwords.
ExternalSecrets provide a htpasswd templating function from sprig that generates only bcrypt hashes
Supporting brcypt hashes for basic_auth will improve security and simplify the workflow I use -> hashicorp vault>external secret>k8s secret>envoy gateway securitypolicy>envoy basic_auth filter
https://www.envoyproxy.io/docs/envoy/latest/configuration/http/http_filters/basic_auth_filter#configuration https://masterminds.github.io/sprig/crypto.html https://external-secrets.io/latest/guides/templating/
@vixns So openssl doesnt support bcrypt. How do u suggest to proceed with this issue? we would either need to ship the implementation along with code here or use some other library
@jmarantz Could please help me with this
https://httpd.apache.org/docs/2.4/misc/password_encryptions.html Here we can see the docs refer a C implementation
https://github.com/openbsd/src/blob/master/lib/libc/crypt/bcrypt.c I prefer the OpenBSD implementation due to the interface being more user friendly
Hi @Athishpranav2003, the OpenBSD implementation seems fine, thanks for working on this
So @vixns do we indeed ship the openbsd implementation along with this?
I am not sure how does dependencies work in envoy. I can implement it but not sure how to include the dependency
@Athishpranav2003, did you consider using https://github.com/trusch/libbcrypt ?
About dependencies: https://github.com/envoyproxy/envoy/blob/main/DEPENDENCY_POLICY.md
This also seems to solve the problem, except that I didn't know if it's the standard bcrypt implementation. If it's fine for maintainer's I will use it i guess
@vixns so for external dependencies there are many more details that need to be provided and i feel that libbcrypt will not be acceptable by the maintainers. I guess we need someone from maintainer team to step in to approve one of the choices we have here
@jmarantz can u help us proceed with the dependency part? I have the code ready for the PR except that need to accomodate the external dependency
@KBaichoo could you please help me with the dependency part? or redirect me to someone who is apt for this query? I have the PR ready, only thing needed is to include the openBSD dependency
@alyssawilk Idk whom to tag for this. No one came forward to help me resolve the dependency part. Could you please help me
Sorry is the problem that you don't know how to set up the external dependency or that you don't know how to get approval for the external dependency?
@alyssawilk The problem i am not aware of setting up the external dependency in this case since here the dependency is in OpenBSD repo(not a library)
my intuition is you're going to have to find a way to pull the code that isn't pulling all of OpenBSD. I'll poke around internally to see if relevant folks know of a trusted standalone library but if you can't find one I suspect you're going to have to find a way to make an extension point and do this in your own repo rather than upstream
@alyssawilk so the code part for this is relatively stable(doesn't change unless there is a CVE) so I presume if I can properly add the dependency in the losted dependency and directly pull the code part alone should be fine.
But I want a confirmation from you/any maintainer for this.
Plus how do I add the dependency for pulling the code part?
When I want an example PR of adding a dependency to Envoy I usually use the blame view on https://github.com/envoyproxy/envoy/blame/main/bazel/repository_locations.bzl to get example PRs of adding new deps. Again I don't think depending directly on OpenBSD files is plausible in a way that aligns with envoy security guidelines so again I'm not sure that's plausible without converting it to a standalone module/library first.
@alyssawilk thanks for letting me know your opinion.
I am going ahead with adding this repo as a external dependency https://github.com/hilch/Bcrypt.cpp
I hope this works in our case Will make the pr tomorrow
Just to clarify; is the plan to make this an extension? I would assume wouldn't want to add this dependency in core?
@jmarantz this should be an dependency for the basic auth extension Do you suggest any other method?
I don't know the details of how this should work, but I'm guessing that we wouldn't want enabling auth to imply enabling bcrypt; we'd want to use a separate extension point for that feature. I'll defer to Alyssa though.
Technically we store hashed passwords during config and use it to verify it on run The verification will be based on the format of hash(no enabling)
If the hash prefix is SHA then we use SHA, if its $2y$ then its bcrypt.
In this way there is no difference infact(its not affecting existing workflow)
obviously we don't want to change the existing auth defaults. I figure it'd be a configuration option and/or a compile time option depending on how onerous the dependencies ended up being
@alyssawilk I guess this is not changing the defaults but just extends right? Anyway, please check the PR and we can discuss further on the draft
Hi guys
is bcrypt support still planned for envoy (in core or extension?). I see PR was auto closed We would also benefit from using bcrypt instead of sha
@yanavlasov FYI
IMO this should be in 'contrib' or maybe an 'extension' if a suitable maintainer-sponser can be found, but should not be in 'core'. E.g. we probably don't want this in our build internally, and this is straightforward as an extension or contrib.
I know you have your reasons to not have it in core, i am also quite distant from C++ and envoy code base. However, nobody should use sha1 hashing for passwords. Its long considered insecure https://httpd.apache.org/docs/2.4/misc/password_encryptions.html https://www.schneier.com/blog/archives/2005/02/cryptanalysis_o.html https://www.dcode.fr/sha1-hash
So i would even go beyond supporting bcrypt - i would suggest you deprecate usage of sha-1 at all and make bcrypt new default :) This way you will be promoting more secure default algorithm
@jmarantz can you take a look at this PR: https://github.com/envoyproxy/envoy/pull/38666? This change aims to enable users to implement bcrypt in "contrib" and use it in the basic_auth extension.
