Password auth #32131

DAlperin · 2025-04-08T18:06:24Z

This is the bones of self managed password auth. This is missing integration tests and documentation which will come next. You can test this locally like so:

#this starts materialize with auth enabled on the external port but not the internal port
$ bin/environmentd --bazel --reset -- --all-features --unsafe-mode --enable-self-hosted-auth

#start a mz_system session, turn on sql support and create a user
$ psql -U mz_system -h localhost -p 6877 materialize
NOTICE:  connected to Materialize v0.139.0-dev.0
  Org ID: 1bd2c405-c638-44cc-b917-6d05dfb832ac
  Region: local/az1
  User: mz_system
  Cluster: mz_system
  Database: materialize
  Schema: public
  Session UUID: fe6d2dbe-3a94-430d-82e1-5e79cb14b91f

Issue a SQL query to get started. Need help?
  View documentation: https://materialize.com/s/docs
  Join our Slack community: https://materialize.com/s/chat
    
psql (14.17 (Homebrew), server 9.5.0)
Type "help" for help.

materialize=> alter system set enable_self_managed_auth=true;
NOTICE:  variable "enable_self_managed_auth" was updated for the system, this will have no effect on the current session
ALTER SYSTEM
materialize=> create role foo with superuser password 'bar';
CREATE ROLE

# Now connect over the port with auth enabled
$ psql -U foo -h localhost -p 6875 materialize
Password for user foo:

This begins implementing #32005

Motivation

Tips for reviewer

Checklist

This PR has adequate test coverage / QA involvement has been duly considered. (trigger-ci for additional test/nightly runs)
This PR has an associated up-to-date design doc, is a design doc (template), or is sufficiently small to not require a design.
If this PR evolves an existing $T ⇔ Proto$T mapping (possibly in a backwards-incompatible way), then it is tagged with a T-proto label.
If this PR will require changes to cloud orchestration or tests, there is a companion cloud PR to account for those changes that is tagged with the release-blocker label (example).
If this PR includes major user-facing behavior changes, I have pinged the relevant PM to schedule a changelog post.

…cratch

src/auth/src/hash.rs

DAlperin · 2025-04-08T20:11:16Z

src/adapter/src/coord/command_handler.rs

+            return;
+        }
+        let password = password.expect("a password, we don't support other auth modes for now");
+        if let Some(role) = self.catalog().try_get_role_by_name(role_name.as_str()) {


holy nesting, batman!

You could reduce some of the nesting by using let else with an early return.

DAlperin · 2025-04-08T20:13:35Z

src/auth/src/hash.rs

+
+fn hash_password_inner(opts: &HashOpts, password: &[u8]) -> [u8; SHA256_OUTPUT_LEN] {
+    let mut salted_password = [0u8; SHA256_OUTPUT_LEN];
+    pbkdf2::derive(


Not technically constant time, but I don't think we are really subject to timing attacks here

is there another document where we outline the set of attacks we are defending against? I can't see anything in the design

src/catalog/src/durable/upgrade/v73_to_v74.rs

src/pgwire/src/protocol.rs

src/sql-parser/src/parser.rs

src/auth/Cargo.toml

src/adapter/src/command.rs

src/adapter/src/coord/command_handler.rs

ptravers · 2025-04-10T18:11:16Z

src/auth/src/hash.rs

+use ring::hmac::{self, Key, HMAC_SHA256};
+use ring::pbkdf2::{self, PBKDF2_HMAC_SHA256 as SHA256};
+use ring::rand::SecureRandom;


we are using BoringSSL here or a repackaging of it[1]. BoringSSL is not FIPS certified[2]. in past positions we have declined to use ring/BoringSSL. it's worth noting that neither ring nor BoringSSL recommend the usage of their code. in past positions we have declined to use both these libraries. what security guarantees are we planning to give to our customers?

[1] https://crates.io/crates/ring
[2] https://boringssl.googlesource.com/boringssl/+/master/crypto/fipsmodule/FIPS.md

I retract part of this as it looks like the algorithms in boringssl are FIPS certified. so I guess this is fine?

see: https://boringssl.googlesource.com/boringssl/+/master/crypto/fipsmodule/FIPS.md

spoke offline with @DAlperin we are migrating to aws-lc-rs which has passed FIPs certification with an approved lab at least for a specific branch of the repo.

it has not been approved by NIST as of 04/10/2025 for current status, see: https://csrc.nist.gov/Projects/cryptographic-module-validation-program/modules-in-process/Modules-In-Process-List

having said that it seems to be far and away the best option. good find @DAlperin.

…cratch

ptravers · 2025-04-10T19:43:45Z

src/auth/src/hash.rs

+
+/// The default iteration count as suggested by
+/// https://cheatsheetseries.owasp.org/cheatsheets/Password_Storage_Cheat_Sheet.html
+const DEFAULT_ITERATIONS: NonZeroU32 = NonZeroU32::new(600_000).unwrap();


if we're going for 600,000 then we should update the technical design to reflect the new value as last I checked we were planning to use 400,000. there should also be a test showing that we meet that bench criteria of 0.1 seconds per hash.

see: https://github.com/MaterializeInc/materialize/pull/32005/files#r2025880535

I'm still reading through NIST and references in that website to try to understand why they picked that value.

src/auth/src/hash.rs

…cratch

ParkMyCar

Overall looks good to me! Don't forget to "Squash and Merge" :)

A few comments that would be nice to address but aren't blocking. The only thing we need to do is remove "OpenSSL" from our list of approved licenses, once I see that I'll approve!!

ParkMyCar · 2025-04-14T17:44:04Z

deny.toml

@@ -296,6 +297,7 @@ allow = [
    "Zlib",
    "Unicode-3.0",
    "Unicode-DFS-2016",
+    "OpenSSL",


Before merging this we need legal's approval. Have you reached out to #operations? If so, can you copy the Slack thread here?

I'm curious where this additional license requirement comes from?

ParkMyCar · 2025-04-14T17:45:15Z

src/adapter/src/error.rs

@@ -175,6 +175,11 @@ pub enum AdapterError {
    Unstructured(anyhow::Error),
    /// The named feature is not supported and will (probably) not be.
    Unsupported(&'static str),
+    /// Some


Left behind comment?

ParkMyCar · 2025-04-14T17:46:48Z

src/adapter/src/error.rs

+    /// Authentication error.
+    AuthenticationError,


Can you include some more context, like what kind of authentication error? Is this from Frontegg or something else?

If you're new to the code and trying to track down why this error is being returned, what would be helpful for you to see here?

ParkMyCar · 2025-04-14T17:48:40Z

src/auth/src/password.rs

+#[derive(Clone, PartialEq, Eq, Serialize, Deserialize, PartialOrd, Ord, Arbitrary)]
+pub struct Password(String);
+
+assert_not_impl_all!(Password: Display);


Probably want to assert it doesn't impl Debug as well?

It does impl Debug, but we implement it manually so it always shows Password(****). This is due to the fact that Password gets carried around in lots of structs with #[derive(Debug)]

ParkMyCar · 2025-04-14T17:48:48Z

src/auth/src/password.rs

+
+///Password is a String wrapper type that does not implement Display or Debug
+#[derive(Clone, PartialEq, Eq, Serialize, Deserialize, PartialOrd, Ord, Arbitrary)]
+pub struct Password(String);


Nice!! Thanks for adding this!

ParkMyCar · 2025-04-14T17:56:02Z

src/environmentd/src/environmentd/main.rs

+    /// Self hosted auth
+    #[clap(long, env = "ENABLE_SELF_HOSTED_AUTH")]
+    enable_self_hosted_auth: bool,
+    /// Self hosted auth over internal port
+    #[clap(long, env = "ENABLE_SELF_HOSTED_AUTH_INTERNAL")]
+    enable_self_hosted_auth_internal: bool,


Mind tossing a TODO(auth) here so we can track the follow-up?

src/pgwire/src/protocol.rs

ParkMyCar · 2025-04-14T17:57:47Z

src/pgwire/src/protocol.rs

+        // No frontegg check, so auth session lasts indefinitely.
+        let auth_session = pending().right_future();


Thanks for checking!

ParkMyCar · 2025-04-14T17:59:10Z

test/sqllogictest/role.slt

@@ -47,11 +47,14 @@ COMPLETE 0
 statement error non inherit roles not yet supported
 CREATE ROLE foo NOINHERIT

-statement error LOGIN attribute is not supported, for more information consult the documentation at
+statement error db error: ERROR: SUPERUSER, PASSWORD, and LOGIN attributes is not supported in this environment. For more information consult the documentation at https://materialize.com/docs/sql/create-role/#details


Very slick! 💯

ParkMyCar · 2025-04-14T17:59:38Z

test/sqllogictest/role_create.slt

+# You might think to yourself "why is creating a role with PASSWORD NULL
+# allowed? Especially when self hosted auth isn't enabled?"
+# The answer is unsatisfying: it's a legacy behavior from Postgres.
+# Creating a role with a null password is the same as not specifying a password at all.
+# So, uh, sure...


Honestly I was wondering, so thanks for adding this comment!

Follow-up to MaterializeInc#32131 Failures seen in https://buildkite.com/materialize/nightly/builds/11825

DAlperin added 11 commits March 27, 2025 10:18

Start parsing the new role options

285f43a

catalog boilerplate

e1ac835

Add a first pass at auth

bb90724

add hashing code

e75023d

plumb hashing through

31f1cc3

Increase salt size

41b06eb

Superuser support

73db029

Add a flag to enable self hosted auth in environmentd/pgwire

7508711

Add a dyncfg for self managed auth

b76e54c

Add seperate config flag for internal port

e8478be

Merge remote-tracking branch 'upstream/main' into dov/password-auth-s…

39260a0

…cratch

DAlperin force-pushed the dov/password-auth-scratch branch from a80a26b to 39260a0 Compare April 8, 2025 18:14

DAlperin added 3 commits April 8, 2025 15:27

lint fixes

fdf36fd

fix catalog-default

8304edb

remove superflous reference

80dff8b

DAlperin commented Apr 8, 2025

View reviewed changes

src/auth/src/hash.rs Outdated Show resolved Hide resolved

DAlperin commented Apr 8, 2025

View reviewed changes

src/catalog/src/durable/upgrade/v73_to_v74.rs Outdated Show resolved Hide resolved

DAlperin commented Apr 8, 2025

View reviewed changes

src/pgwire/src/protocol.rs Outdated Show resolved Hide resolved

DAlperin commented Apr 8, 2025

View reviewed changes

src/sql-parser/src/parser.rs Outdated Show resolved Hide resolved

DAlperin added 2 commits April 8, 2025 16:42

This is suboptimal but likely fine

472967e

fix clippy

3ac78c2

alex-hunt-materialize reviewed Apr 9, 2025

View reviewed changes

src/auth/Cargo.toml Outdated Show resolved Hide resolved

DAlperin added 6 commits April 9, 2025 10:52

fix a bunch of tests

cb4c0df

fix tests and errant comments

ea34d34

rust tests for self managed auth

01d9d62

Fix slt

9e6877d

fix snapshot

b0051bc

clippy

11db523

ptravers self-requested a review April 10, 2025 15:35

ptravers reviewed Apr 10, 2025

View reviewed changes

src/adapter/src/command.rs Outdated Show resolved Hide resolved

ptravers reviewed Apr 10, 2025

View reviewed changes

src/adapter/src/coord/command_handler.rs Outdated Show resolved Hide resolved

DAlperin added 3 commits April 10, 2025 12:04

fix snapshot

7353864

Enforce+test rbac and consistency

2df6d81

Use custom password type

27d6974

ptravers reviewed Apr 10, 2025

View reviewed changes

DAlperin added 6 commits April 10, 2025 14:32

password header + rule allows

060b91a

Merge remote-tracking branch 'upstream/main' into dov/password-auth-s…

002efd7

…cratch

adress feedback

df1638e

switch to aws_lc_rs

cb235ef

fix snapshot

c2ad82c

update bazel files

d9e3656

ptravers reviewed Apr 10, 2025

View reviewed changes

fix cargo deny

516c6a9

ptravers reviewed Apr 10, 2025

View reviewed changes

src/auth/src/hash.rs Outdated Show resolved Hide resolved

ptravers reviewed Apr 10, 2025

View reviewed changes

src/auth/src/hash.rs Outdated Show resolved Hide resolved

ptravers reviewed Apr 10, 2025

View reviewed changes

src/auth/src/hash.rs Outdated Show resolved Hide resolved

ptravers reviewed Apr 10, 2025

View reviewed changes

src/auth/src/hash.rs Outdated Show resolved Hide resolved

DAlperin added 4 commits April 14, 2025 10:32

Swap out aws-lc-rs for openssl

2058f50

fix tests

2315dd3

Merge remote-tracking branch 'upstream/main' into dov/password-auth-s…

ffc4d59

…cratch

fix clippy hopefully

dc35cac

ParkMyCar reviewed Apr 14, 2025

View reviewed changes

fix last few notes

0e9174f

ParkMyCar approved these changes Apr 14, 2025

View reviewed changes

DAlperin enabled auto-merge (squash) April 14, 2025 18:25

DAlperin merged commit d37c5be into MaterializeInc:main Apr 14, 2025
83 checks passed

def- added a commit to def-/materialize that referenced this pull request Apr 15, 2025

Fix nightly failures from auth

030aeec

Follow-up to MaterializeInc#32131 Failures seen in https://buildkite.com/materialize/nightly/builds/11825

def- mentioned this pull request Apr 15, 2025

Fix nightly failures from auth #32204

Merged

5 tasks

		// No frontegg check, so auth session lasts indefinitely.
		let auth_session = pending().right_future();

Password auth #32131

Password auth #32131

Uh oh!

Conversation

DAlperin commented Apr 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation

Tips for reviewer

Checklist

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ptravers Apr 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ptravers Apr 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ptravers Apr 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ParkMyCar left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

DAlperin commented Apr 8, 2025 •

edited

Loading

ptravers Apr 10, 2025 •

edited

Loading

ptravers Apr 10, 2025 •

edited

Loading

ptravers Apr 10, 2025 •

edited

Loading