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

Basic_auth: Add bcrypt support #36882

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions bazel/repository_locations.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,17 @@ REPOSITORY_LOCATIONS_SPEC = dict(
release_date = "2022-06-13",
cpe = "cpe:2.3:a:google:boringssl:*",
),
com_github_bcrypt = dict(
Copy link
Member

Choose a reason for hiding this comment

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

com_github_hilch_bcrypt

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"],
Copy link
Member

Choose a reason for hiding this comment

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

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

release_date = "2021-12-24",
cpe = "N/A",
),
Comment on lines +166 to +176
Copy link
Contributor

Choose a reason for hiding this comment

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

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                       |                                                                                                                   |
|---------|------------------------|--------------------------------|-------------------------------------------------------------------------------------------------------------------|-----------------------------------------------------------------------------------------------------------------------|

aspect_bazel_lib = dict(
project_name = "Aspect Bazel helpers",
project_desc = "Base Starlark libraries and basic Bazel rules which are useful for constructing rulesets and BUILD files",
Expand Down
2 changes: 1 addition & 1 deletion source/extensions/filters/http/basic_auth/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -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"],
Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks @phlax for sharing, will do it

Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Member

Choose a reason for hiding this comment

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

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.

deps = [
"//envoy/server:filter_config_interface",
"//source/common/common:base64_lib",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Copy link
Member

Choose a reason for hiding this comment

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

Prefer "" for non-system/std denpendency.


namespace Envoy {
namespace Extensions {
Expand Down Expand Up @@ -101,7 +102,12 @@ bool BasicAuthFilter::validateUser(const UserMap& users, absl::string_view usern
return false;
}

return computeSHA1(password) == user->second.hash;
if (absl::StartsWith(user->second.hash, "{SHA}")) {
return user->second.hash == absl::StrCat("{SHA}", computeSHA1(password));
}
Comment on lines +105 to +107
Copy link
Member

Choose a reason for hiding this comment

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

To avoid unnecessary string catting:

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


std::string password_string{password.data(), password.length()};
return BCrypt::validatePassword(password_string, user->second.hash);
}

Http::FilterHeadersStatus BasicAuthFilter::onDenied(absl::string_view body,
Expand Down
19 changes: 13 additions & 6 deletions source/extensions/filters/http/basic_auth/config.cc
Original file line number Diff line number Diff line change
Expand Up @@ -43,14 +43,21 @@ UserMap readHtpasswd(const std::string& htpasswd) {
throw EnvoyException("basic auth: duplicate users");
}

if (!absl::StartsWith(hash, "{SHA}")) {
throw EnvoyException("basic auth: unsupported htpasswd format: please use {SHA}");
if (absl::StartsWith(hash, "{SHA}")) {
// The base64 encoded SHA1 hash is 28 bytes long and prefix is 5
if (hash.length() != 5+28) {
throw EnvoyException("basic auth: invalid htpasswd format, invalid SHA hash length");
}
}

hash = hash.substr(5);
// The base64 encoded SHA1 hash is 28 bytes long
if (hash.length() != 28) {
throw EnvoyException("basic auth: invalid htpasswd format, invalid SHA hash length");
else if (absl::StartsWith(hash, "$2b$") || absl::StartsWith(hash, "$2y$") || absl::StartsWith(hash, "$2x$") || absl::StartsWith(hash, "$2a$")) {
if (hash.length() != 60) {
throw EnvoyException("basic auth: invalid htpasswd format, invalid bcrypt hash length");
}
}

else {
throw EnvoyException("basic auth: unsupported htpasswd format: invalid hash type");
}

users.insert({name, {name, hash}});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ name: envoy.filters.http.basic_auth
inline_string: |-
user1:{SHA}tESsBmE/yNY3lb6a0L6vVQEZNqw=
user2:{SHA}EJ9LPFDXsN9ynSmbxvjp75Bmlx8=
user3:$2y$05$br5Mb9Dvx3tjWSd9imKvfOBTP9EycYIRTzpDkrMMuZS7y8G1P7p8O
forward_username_header: x-username
)EOF";

Expand Down Expand Up @@ -89,7 +90,7 @@ INSTANTIATE_TEST_SUITE_P(
testing::ValuesIn(HttpProtocolIntegrationTest::getProtocolTestParamsWithoutHTTP3()),
HttpProtocolIntegrationTest::protocolTestParamsToString);

// Request with valid credential
// Request with valid credential with SHA1 hash
TEST_P(BasicAuthIntegrationTestAllProtocols, ValidCredential) {
initializeFilter();
codec_client_ = makeHttpConnection(lookupPort("http"));
Expand All @@ -114,6 +115,31 @@ TEST_P(BasicAuthIntegrationTestAllProtocols, ValidCredential) {
EXPECT_EQ("200", response->headers().getStatusValue());
}

// Request with valid credential with bcrypt hash
TEST_P(BasicAuthIntegrationTestAllProtocols, ValidCredential) {
initializeFilter();
codec_client_ = makeHttpConnection(lookupPort("http"));

auto response = codec_client_->makeHeaderOnlyRequest(Http::TestRequestHeaderMapImpl{
{":method", "GET"},
{":path", "/"},
{":scheme", "http"},
{":authority", "host"},
{"Authorization", "Basic dXNlcjM6dGVzdDM="}, // user3, test3
});

waitForNextUpstreamRequest();

const auto username_entry = upstream_request_->headers().get(Http::LowerCaseString("x-username"));
EXPECT_FALSE(username_entry.empty());
EXPECT_EQ(username_entry[0]->value().getStringView(), "user3");

upstream_request_->encodeHeaders(Http::TestResponseHeaderMapImpl{{":status", "200"}}, true);
ASSERT_TRUE(response->waitForEndStream());
ASSERT_TRUE(response->complete());
EXPECT_EQ("200", response->headers().getStatusValue());
}

// Request without credential
TEST_P(BasicAuthIntegrationTestAllProtocols, NoCredential) {
initializeFilter();
Expand Down Expand Up @@ -167,7 +193,7 @@ TEST_P(BasicAuthIntegrationTestAllProtocols, NoneExistedUser) {
{":path", "/"},
{":scheme", "http"},
{":authority", "host"},
{"Authorization", "Basic dXNlcjM6dGVzdDI="}, // user3, test2
{"Authorization", "Basic dXNlcjQ6dGVzdDI="}, // user4, test2
});

ASSERT_TRUE(response->waitForEndStream());
Expand Down