Basic_auth: Add bcrypt support #36882

Athishpranav2003 · 2024-10-29T05:48:13Z

Commit Message: Added bcrypt support for basic auth extension
Additional Description: This PR adds support to include bcrypt hashes in basic auth during configuration
Risk Level: N/A
Testing: N/A
Docs Changes: N/A
Release Notes: N/A
Platform Specific Features: N/A
[Optional Runtime guard:]
[Optional Fixes #36278]
[Optional Fixes commit #PR or SHA]
[Optional Deprecated:]
[Optional API Considerations:]

repokitteh-read-only · 2024-10-29T05:48:21Z

CC @envoyproxy/dependency-shepherds: Your approval is needed for changes made to (bazel/.*repos.*\.bzl)|(bazel/dependency_imports\.bzl)|(api/bazel/.*\.bzl)|(.*/requirements\.txt)|(.*\.patch).
envoyproxy/dependency-shepherds assignee is @moderation

🐱

Caused by: #36882 was opened by Athishpranav2003.

see: more, trace.

Athishpranav2003 · 2024-10-29T05:49:40Z

@alyssawilk @jmarantz
I have created the draft addressing the bcrypt issue here,
Lets have the discussions here.
I have made it as a draft since we havent come to any conclusion

I am still not sure of the deps part, roughly added it but cant test it

Signed-off-by: Athish Pranav D <[email protected]>

wbpcode · 2024-10-29T08:05:21Z

Thanks for the contribution. I wonder is there a more popular lib for bcrypt?

Athishpranav2003 · 2024-10-29T08:21:09Z

@wbpcode These are the ones
Apart from this there is code available in openwall and openBSD repos which we can pull and maintain in-house

phlax

@Athishpranav2003 to pull in the dep you will need to set it up in bazel/repositories.bzl

assuming the upstream you use doesnt have bazel setup you will need inject a BUILD file (search for _build_all_content and/or BUILD_ALL_CONTENT in that file)

i would say use the openbsd version as that is likely to be the best maintained but afaict that would involve pulling down the entire openbsd src tarball (~250M) so perhaps not the best option

i had a quick look at others and the ones that i would probably go for dont seem to maintained

phlax · 2024-10-29T08:36:36Z

source/extensions/filters/http/basic_auth/BUILD

@@ -13,7 +13,7 @@ envoy_cc_library(
    name = "basic_auth_lib",
    srcs = ["basic_auth_filter.cc"],
    hdrs = ["basic_auth_filter.h"],
-    external_deps = ["ssl"],
+    external_deps = ["ssl", "bcrypt"],


its not an external dep as defined here - i needs to be referenced as a normal dep using its @name

Thanks @phlax for sharing, will do it

just to add - you will most likely need to use foreign_cc to build it

Please use @com_github_hilch_bcrypt//:bcriypt_lib directly. Maybe you also need to add a build file for the dependency because it doesn't use bazel.

Athishpranav2003 · 2024-10-30T16:14:42Z

Thanks for the contribution. I wonder is there a more popular lib for bcrypt?

@wbpcode Whats your opinion?
which library should i proceed with

wbpcode · 2024-10-31T07:11:05Z

I basically has no strong idea if there are no any widely used choice. May deps shepherds could answer this question @envoyproxy/dependency-shepherds

phlax · 2024-10-31T08:07:35Z

im assuming that all the available libs dont have any recent activity as the code is simple/stable

on that basis, i think any of the first 3 posted in image above seem like a reasonable choice

wbpcode

If the @envoyproxy/dependency-shepherds say this dependency is fine, then the code LGTM overall. Some comments are added.

Thanks for the contribution. :)

wbpcode · 2024-10-31T11:32:41Z

source/extensions/filters/http/basic_auth/basic_auth_filter.cc

+  if (absl::StartsWith(user->second.hash, "{SHA}")) {
+    return user->second.hash == absl::StrCat("{SHA}", computeSHA1(password));
+  }


To avoid unnecessary string catting:

return absl::string_view(user->second.hash).substr(5) == computeSHA1(password);

wbpcode · 2024-10-31T11:36:49Z

bazel/repository_locations.bzl

+        version = "V2.0_NODEBCRYPT",
+        sha256 = "150abb95058a459e58f2bf995fd85af1d616374cc920d1ebb84666a8b2b3c50c",
+        urls = ["https://github.com/hilch/Bcrypt.cpp/archive/refs/tags/{version}.tar.gz"],
+        use_category = ["controlplane", "dataplane_core"],


use_category = ["dataplane_ext"], extensions = ["envoy.filters.http.basic_auth"],

wbpcode · 2024-10-31T11:37:54Z

bazel/repository_locations.bzl

@@ -163,6 +163,17 @@ REPOSITORY_LOCATIONS_SPEC = dict(
        release_date = "2022-06-13",
        cpe = "cpe:2.3:a:google:boringssl:*",
    ),
+    com_github_bcrypt = dict(


com_github_hilch_bcrypt

wbpcode · 2024-10-31T11:39:39Z

source/extensions/filters/http/basic_auth/BUILD

@@ -13,7 +13,7 @@ envoy_cc_library(
    name = "basic_auth_lib",
    srcs = ["basic_auth_filter.cc"],
    hdrs = ["basic_auth_filter.h"],
-    external_deps = ["ssl"],
+    external_deps = ["ssl", "bcrypt"],


Please use @com_github_hilch_bcrypt//:bcriypt_lib directly. Maybe you also need to add a build file for the dependency because it doesn't use bazel.

wbpcode · 2024-10-31T11:40:20Z

source/extensions/filters/http/basic_auth/basic_auth_filter.cc

@@ -8,6 +8,7 @@
 #include "source/common/http/header_utility.h"
 #include "source/common/http/headers.h"
 #include "source/common/http/utility.h"
+#include <bcrypt/BCrypt.hpp>


Prefer "" for non-system/std denpendency.

wbpcode · 2024-10-31T11:40:43Z

/wait

wbpcode · 2024-10-31T11:41:13Z

cc code owner @zhaohuabing

jmarantz · 2024-11-04T13:57:02Z

Please structure this as an extension, rather than changing Envoy to by accept bcrypt by default.

Related: https://x.com/bcrypt/status/1852575080989257893?s=19

moderation · 2024-11-04T20:22:47Z

bazel/repository_locations.bzl

+    com_github_bcrypt = dict(
+        project_name = "bcrypt",
+        project_desc = " C++ wrapper around bcrypt password hashing",
+        project_url = "https://github.com/hilch/Bcrypt.cpp",
+        version = "V2.0_NODEBCRYPT",
+        sha256 = "150abb95058a459e58f2bf995fd85af1d616374cc920d1ebb84666a8b2b3c50c",
+        urls = ["https://github.com/hilch/Bcrypt.cpp/archive/refs/tags/{version}.tar.gz"],
+        use_category = ["controlplane", "dataplane_core"],
+        release_date = "2021-12-24",
+        cpe = "N/A",
+    ),


Poor OSSF Scorecard score:

scorecard --repo github.com/hilch/Bcrypt.cpp RESULTS ------- Aggregate score: 2.9 / 10 Check scores: |---------|------------------------|--------------------------------|-------------------------------------------------------------------------------------------------------------------|-----------------------------------------------------------------------------------------------------------------------| | SCORE | NAME | REASON | DETAILS | DOCUMENTATION/REMEDIATION | |---------|------------------------|--------------------------------|-------------------------------------------------------------------------------------------------------------------|-----------------------------------------------------------------------------------------------------------------------| | 10 / 10 | Binary-Artifacts | no binaries found in the repo | https://github.com/ossf/scorecard/blob/ea7e27ed41b76ab879c862fa0ca4cc9c61764ee4/docs/checks.md#binary-artifacts | |---------|------------------------|--------------------------------|-------------------------------------------------------------------------------------------------------------------|-----------------------------------------------------------------------------------------------------------------------| | 0 / 10 | Branch-Protection | branch protection not enabled | Warn: branch protection not | https://github.com/ossf/scorecard/blob/ea7e27ed41b76ab879c862fa0ca4cc9c61764ee4/docs/checks.md#branch-protection | | | | on development/release | enabled for branch 'master' | | | | | branches | | | |---------|------------------------|--------------------------------|-------------------------------------------------------------------------------------------------------------------|-----------------------------------------------------------------------------------------------------------------------| | 0 / 10 | CI-Tests | 0 out of 2 merged PRs | https://github.com/ossf/scorecard/blob/ea7e27ed41b76ab879c862fa0ca4cc9c61764ee4/docs/checks.md#ci-tests | | | | checked by a CI test -- score | | | | | normalized to 0 | | |---------|------------------------|--------------------------------|-------------------------------------------------------------------------------------------------------------------|-----------------------------------------------------------------------------------------------------------------------| | 0 / 10 | CII-Best-Practices | no effort to earn an OpenSSF | https://github.com/ossf/scorecard/blob/ea7e27ed41b76ab879c862fa0ca4cc9c61764ee4/docs/checks.md#cii-best-practices | | | | best practices badge detected | | |---------|------------------------|--------------------------------|-------------------------------------------------------------------------------------------------------------------|-----------------------------------------------------------------------------------------------------------------------| | 0 / 10 | Code-Review | Found 2/21 approved changesets | https://github.com/ossf/scorecard/blob/ea7e27ed41b76ab879c862fa0ca4cc9c61764ee4/docs/checks.md#code-review | | | | -- score normalized to 0 | | |---------|------------------------|--------------------------------|-------------------------------------------------------------------------------------------------------------------|-----------------------------------------------------------------------------------------------------------------------| | 0 / 10 | Contributors | project has 0 contributing | https://github.com/ossf/scorecard/blob/ea7e27ed41b76ab879c862fa0ca4cc9c61764ee4/docs/checks.md#contributors | | | | companies or organizations -- | | | | | score normalized to 0 | | |---------|------------------------|--------------------------------|-------------------------------------------------------------------------------------------------------------------|-----------------------------------------------------------------------------------------------------------------------| | 10 / 10 | Dangerous-Workflow | no dangerous workflow patterns | https://github.com/ossf/scorecard/blob/ea7e27ed41b76ab879c862fa0ca4cc9c61764ee4/docs/checks.md#dangerous-workflow | | | | detected | | |---------|------------------------|--------------------------------|-------------------------------------------------------------------------------------------------------------------|-----------------------------------------------------------------------------------------------------------------------| | 0 / 10 | Dependency-Update-Tool | no update tool detected | Warn: no dependency update | https://github.com/ossf/scorecard/blob/ea7e27ed41b76ab879c862fa0ca4cc9c61764ee4/docs/checks.md#dependency-update-tool | | | | | tool configurations found | | |---------|------------------------|--------------------------------|-------------------------------------------------------------------------------------------------------------------|-----------------------------------------------------------------------------------------------------------------------| | 0 / 10 | Fuzzing | project is not fuzzed | Warn: no fuzzer integrations | https://github.com/ossf/scorecard/blob/ea7e27ed41b76ab879c862fa0ca4cc9c61764ee4/docs/checks.md#fuzzing | | | | | found | | |---------|------------------------|--------------------------------|-------------------------------------------------------------------------------------------------------------------|-----------------------------------------------------------------------------------------------------------------------| | 9 / 10 | License | license file detected | Info: project has a license | https://github.com/ossf/scorecard/blob/ea7e27ed41b76ab879c862fa0ca4cc9c61764ee4/docs/checks.md#license | | | | | file: LICENSE:0 Warn: project | | | | | | license file does not contain | | | | | | an FSF or OSI license. | | |---------|------------------------|--------------------------------|-------------------------------------------------------------------------------------------------------------------|-----------------------------------------------------------------------------------------------------------------------| | 0 / 10 | Maintained | 0 commit(s) and 0 issue | https://github.com/ossf/scorecard/blob/ea7e27ed41b76ab879c862fa0ca4cc9c61764ee4/docs/checks.md#maintained | | | | activity found in the last 90 | | | | | days -- score normalized to 0 | | |---------|------------------------|--------------------------------|-------------------------------------------------------------------------------------------------------------------|-----------------------------------------------------------------------------------------------------------------------| | ? | Packaging | packaging workflow not | Warn: no GitHub/GitLab | https://github.com/ossf/scorecard/blob/ea7e27ed41b76ab879c862fa0ca4cc9c61764ee4/docs/checks.md#packaging | | | | detected | publishing workflow detected. | | |---------|------------------------|--------------------------------|-------------------------------------------------------------------------------------------------------------------|-----------------------------------------------------------------------------------------------------------------------| | 0 / 10 | Pinned-Dependencies | dependency not pinned by hash | Warn: GitHub-owned GitHubAction not pinned by hash: | https://github.com/ossf/scorecard/blob/ea7e27ed41b76ab879c862fa0ca4cc9c61764ee4/docs/checks.md#pinned-dependencies | | | | detected -- score normalized | .github/workflows/libbcrypt.yml:9: update your workflow using | | | | | to 0 | https://app.stepsecurity.io/secureworkflow/hilch/Bcrypt.cpp/libbcrypt.yml/master?enable=pin | | | | | | Info: 0 out of 1 GitHub-owned GitHubAction dependencies pinned | | |---------|------------------------|--------------------------------|-------------------------------------------------------------------------------------------------------------------|-----------------------------------------------------------------------------------------------------------------------| | 0 / 10 | SAST | SAST tool is not run on all | Warn: 0 commits out of 5 are | https://github.com/ossf/scorecard/blob/ea7e27ed41b76ab879c862fa0ca4cc9c61764ee4/docs/checks.md#sast | | | | commits -- score normalized to | checked with a SAST tool | | | | | 0 | | | |---------|------------------------|--------------------------------|-------------------------------------------------------------------------------------------------------------------|-----------------------------------------------------------------------------------------------------------------------| | 0 / 10 | Security-Policy | security policy file not | Warn: no security policy file | https://github.com/ossf/scorecard/blob/ea7e27ed41b76ab879c862fa0ca4cc9c61764ee4/docs/checks.md#security-policy | | | | detected | detected Warn: no security | | | | | | file to analyze Warn: no | | | | | | security file to analyze Warn: | | | | | | no security file to analyze | | |---------|------------------------|--------------------------------|-------------------------------------------------------------------------------------------------------------------|-----------------------------------------------------------------------------------------------------------------------| | ? | Signed-Releases | no releases found | https://github.com/ossf/scorecard/blob/ea7e27ed41b76ab879c862fa0ca4cc9c61764ee4/docs/checks.md#signed-releases | |---------|------------------------|--------------------------------|-------------------------------------------------------------------------------------------------------------------|-----------------------------------------------------------------------------------------------------------------------| | 0 / 10 | Token-Permissions | detected GitHub workflow | Warn: no topLevel | https://github.com/ossf/scorecard/blob/ea7e27ed41b76ab879c862fa0ca4cc9c61764ee4/docs/checks.md#token-permissions | | | | tokens with excessive | permission defined: | | | | | permissions | .github/workflows/libbcrypt.yml:1 | | | | | | Info: no jobLevel write | | | | | | permissions found | | |---------|------------------------|--------------------------------|-------------------------------------------------------------------------------------------------------------------|-----------------------------------------------------------------------------------------------------------------------| | 10 / 10 | Vulnerabilities | 0 existing vulnerabilities | https://github.com/ossf/scorecard/blob/ea7e27ed41b76ab879c862fa0ca4cc9c61764ee4/docs/checks.md#vulnerabilities | | | | detected | | |---------|------------------------|--------------------------------|-------------------------------------------------------------------------------------------------------------------|-----------------------------------------------------------------------------------------------------------------------|

Athishpranav2003 requested a review from wbpcode as a code owner October 29, 2024 05:48

repokitteh-read-only bot added the deps Approval required for changes to Envoy's external dependencies label Oct 29, 2024

repokitteh-read-only bot assigned moderation Oct 29, 2024

Athishpranav2003 marked this pull request as draft October 29, 2024 05:48

Added bcrypt support

f0e6849

Signed-off-by: Athish Pranav D <[email protected]>

phlax reviewed Oct 29, 2024

View reviewed changes

wbpcode reviewed Oct 31, 2024

View reviewed changes

repokitteh-read-only bot added the waiting label Oct 31, 2024

wbpcode self-assigned this Oct 31, 2024

moderation reviewed Nov 4, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Basic_auth: Add bcrypt support #36882

Basic_auth: Add bcrypt support #36882

Athishpranav2003 commented Oct 29, 2024

repokitteh-read-only bot commented Oct 29, 2024

Athishpranav2003 commented Oct 29, 2024

wbpcode commented Oct 29, 2024

Athishpranav2003 commented Oct 29, 2024

phlax left a comment

phlax Oct 29, 2024

Athishpranav2003 Oct 29, 2024

phlax Oct 29, 2024

wbpcode Oct 31, 2024

Athishpranav2003 commented Oct 30, 2024

wbpcode commented Oct 31, 2024 •

edited

Loading

phlax commented Oct 31, 2024

wbpcode left a comment

wbpcode Oct 31, 2024

wbpcode Oct 31, 2024

wbpcode Oct 31, 2024

wbpcode Oct 31, 2024

wbpcode Oct 31, 2024

wbpcode commented Oct 31, 2024

wbpcode commented Oct 31, 2024

jmarantz commented Nov 4, 2024

moderation Nov 4, 2024

Basic_auth: Add bcrypt support #36882

Are you sure you want to change the base?

Basic_auth: Add bcrypt support #36882

Conversation

Athishpranav2003 commented Oct 29, 2024

repokitteh-read-only bot commented Oct 29, 2024

Athishpranav2003 commented Oct 29, 2024

wbpcode commented Oct 29, 2024

Athishpranav2003 commented Oct 29, 2024

phlax left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Athishpranav2003 commented Oct 30, 2024

wbpcode commented Oct 31, 2024 • edited Loading

phlax commented Oct 31, 2024

wbpcode left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wbpcode commented Oct 31, 2024

wbpcode commented Oct 31, 2024

jmarantz commented Nov 4, 2024

Choose a reason for hiding this comment

wbpcode commented Oct 31, 2024 •

edited

Loading