From c8cde704cf353bcde47c97937fdcbe9229d52438 Mon Sep 17 00:00:00 2001 From: shuiyisong <113876041+shuiyisong@users.noreply.github.com> Date: Tue, 15 Aug 2023 18:49:22 +0800 Subject: [PATCH] chore: minor `auth` crate change (#2176) * chore: pub auth_mysql * chore: pub all error * chore: remove back to error * chore: wrap failed permission check result to err * chore: minor change --- src/auth/src/common.rs | 83 ++++++++++++++++++- src/auth/src/error.rs | 4 + src/auth/src/lib.rs | 7 +- src/auth/src/permission.rs | 8 +- src/auth/src/tests.rs | 13 ++- .../src/user_provider/static_user_provider.rs | 81 +----------------- src/auth/tests/mod.rs | 4 +- src/cmd/src/error.rs | 2 +- src/common/error/src/status_code.rs | 8 +- src/frontend/src/error.rs | 2 +- src/servers/src/error.rs | 4 +- src/servers/src/mysql/handler.rs | 3 +- 12 files changed, 117 insertions(+), 102 deletions(-) diff --git a/src/auth/src/common.rs b/src/auth/src/common.rs index 8278ae53dc..d49f4883b7 100644 --- a/src/auth/src/common.rs +++ b/src/auth/src/common.rs @@ -14,10 +14,12 @@ use std::sync::Arc; +use digest::Digest; use secrecy::SecretString; -use snafu::OptionExt; +use sha1::Sha1; +use snafu::{ensure, OptionExt}; -use crate::error::{InvalidConfigSnafu, Result}; +use crate::error::{IllegalParamSnafu, InvalidConfigSnafu, Result, UserPasswordMismatchSnafu}; use crate::user_info::DefaultUserInfo; use crate::user_provider::static_user_provider::{StaticUserProvider, STATIC_USER_PROVIDER}; use crate::{UserInfoRef, UserProviderRef}; @@ -66,3 +68,80 @@ pub enum Password<'a> { MysqlNativePassword(HashedPassword<'a>, Salt<'a>), PgMD5(HashedPassword<'a>, Salt<'a>), } + +pub fn auth_mysql( + auth_data: HashedPassword, + salt: Salt, + username: &str, + save_pwd: &[u8], +) -> Result<()> { + ensure!( + auth_data.len() == 20, + IllegalParamSnafu { + msg: "Illegal mysql password length" + } + ); + // ref: https://github.com/mysql/mysql-server/blob/a246bad76b9271cb4333634e954040a970222e0a/sql/auth/password.cc#L62 + let hash_stage_2 = double_sha1(save_pwd); + let tmp = sha1_two(salt, &hash_stage_2); + // xor auth_data and tmp + let mut xor_result = [0u8; 20]; + for i in 0..20 { + xor_result[i] = auth_data[i] ^ tmp[i]; + } + let candidate_stage_2 = sha1_one(&xor_result); + if candidate_stage_2 == hash_stage_2 { + Ok(()) + } else { + UserPasswordMismatchSnafu { + username: username.to_string(), + } + .fail() + } +} + +fn sha1_two(input_1: &[u8], input_2: &[u8]) -> Vec { + let mut hasher = Sha1::new(); + hasher.update(input_1); + hasher.update(input_2); + hasher.finalize().to_vec() +} + +fn sha1_one(data: &[u8]) -> Vec { + let mut hasher = Sha1::new(); + hasher.update(data); + hasher.finalize().to_vec() +} + +fn double_sha1(data: &[u8]) -> Vec { + sha1_one(&sha1_one(data)) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_sha() { + let sha_1_answer: Vec = vec![ + 124, 74, 141, 9, 202, 55, 98, 175, 97, 229, 149, 32, 148, 61, 194, 100, 148, 248, 148, + 27, + ]; + let sha_1 = sha1_one("123456".as_bytes()); + assert_eq!(sha_1, sha_1_answer); + + let double_sha1_answer: Vec = vec![ + 107, 180, 131, 126, 183, 67, 41, 16, 94, 228, 86, 141, 218, 125, 198, 126, 210, 202, + 42, 217, + ]; + let double_sha1 = double_sha1("123456".as_bytes()); + assert_eq!(double_sha1, double_sha1_answer); + + let sha1_2_answer: Vec = vec![ + 132, 115, 215, 211, 99, 186, 164, 206, 168, 152, 217, 192, 117, 47, 240, 252, 142, 244, + 37, 204, + ]; + let sha1_2 = sha1_two("123456".as_bytes(), "654321".as_bytes()); + assert_eq!(sha1_2, sha1_2_answer); + } +} diff --git a/src/auth/src/error.rs b/src/auth/src/error.rs index 7acfb5726a..f14ffbb190 100644 --- a/src/auth/src/error.rs +++ b/src/auth/src/error.rs @@ -60,6 +60,9 @@ pub enum Error { schema: String, username: String, }, + + #[snafu(display("User is not authorized to perform this action"))] + PermissionDenied { location: Location }, } impl ErrorExt for Error { @@ -75,6 +78,7 @@ impl ErrorExt for Error { Error::UnsupportedPasswordType { .. } => StatusCode::UnsupportedPasswordType, Error::UserPasswordMismatch { .. } => StatusCode::UserPasswordMismatch, Error::AccessDenied { .. } => StatusCode::AccessDenied, + Error::PermissionDenied { .. } => StatusCode::PermissionDenied, } } diff --git a/src/auth/src/lib.rs b/src/auth/src/lib.rs index 8fa919b2a2..dadbdee02c 100644 --- a/src/auth/src/lib.rs +++ b/src/auth/src/lib.rs @@ -13,7 +13,7 @@ // limitations under the License. mod common; -mod error; +pub mod error; mod permission; mod user_info; mod user_provider; @@ -21,8 +21,9 @@ mod user_provider; #[cfg(feature = "testing")] pub mod tests; -pub use common::{user_provider_from_option, userinfo_by_name, HashedPassword, Identity, Password}; -pub use error::{Error, Result}; +pub use common::{ + auth_mysql, user_provider_from_option, userinfo_by_name, HashedPassword, Identity, Password, +}; pub use permission::{PermissionChecker, PermissionReq, PermissionResp}; pub use user_info::UserInfo; pub use user_provider::UserProvider; diff --git a/src/auth/src/permission.rs b/src/auth/src/permission.rs index 64734a3c5a..5ebc0ab20b 100644 --- a/src/auth/src/permission.rs +++ b/src/auth/src/permission.rs @@ -17,7 +17,7 @@ use std::fmt::Debug; use api::v1::greptime_request::Request; use sql::statements::statement::Statement; -use crate::error::Result; +use crate::error::{PermissionDeniedSnafu, Result}; use crate::{PermissionCheckerRef, UserInfoRef}; #[derive(Debug, Clone)] @@ -53,7 +53,11 @@ impl PermissionChecker for Option<&PermissionCheckerRef> { req: PermissionReq, ) -> Result { match self { - Some(checker) => checker.check_permission(user_info, req), + Some(checker) => match checker.check_permission(user_info, req) { + Ok(PermissionResp::Reject) => PermissionDeniedSnafu.fail(), + Ok(PermissionResp::Allow) => Ok(PermissionResp::Allow), + Err(e) => Err(e), + }, None => Ok(PermissionResp::Allow), } } diff --git a/src/auth/src/tests.rs b/src/auth/src/tests.rs index 7cc1b5c7e4..92574541a1 100644 --- a/src/auth/src/tests.rs +++ b/src/auth/src/tests.rs @@ -18,10 +18,7 @@ use crate::error::{ UserPasswordMismatchSnafu, }; use crate::user_info::DefaultUserInfo; -use crate::user_provider::static_user_provider::auth_mysql; -#[allow(unused_imports)] -use crate::Error; -use crate::{Identity, Password, UserInfoRef, UserProvider}; +use crate::{auth_mysql, Identity, Password, UserInfoRef, UserProvider}; pub struct DatabaseAuthInfo<'a> { pub catalog: &'a str, @@ -108,6 +105,8 @@ impl UserProvider for MockUserProvider { #[tokio::test] async fn test_auth_by_plain_text() { + use crate::error; + let user_provider = MockUserProvider::default(); assert_eq!("mock_user_provider", user_provider.name()); @@ -131,7 +130,7 @@ async fn test_auth_by_plain_text() { assert!(auth_result.is_err()); assert!(matches!( auth_result.err().unwrap(), - Error::UnsupportedPasswordType { .. } + error::Error::UnsupportedPasswordType { .. } )); // auth failed, err: user not exist. @@ -144,7 +143,7 @@ async fn test_auth_by_plain_text() { assert!(auth_result.is_err()); assert!(matches!( auth_result.err().unwrap(), - Error::UserNotFound { .. } + error::Error::UserNotFound { .. } )); // auth failed, err: wrong password @@ -157,7 +156,7 @@ async fn test_auth_by_plain_text() { assert!(auth_result.is_err()); assert!(matches!( auth_result.err().unwrap(), - Error::UserPasswordMismatch { .. } + error::Error::UserPasswordMismatch { .. } )) } diff --git a/src/auth/src/user_provider/static_user_provider.rs b/src/auth/src/user_provider/static_user_provider.rs index 5510b34fe0..591a116b92 100644 --- a/src/auth/src/user_provider/static_user_provider.rs +++ b/src/auth/src/user_provider/static_user_provider.rs @@ -19,18 +19,15 @@ use std::io::BufRead; use std::path::Path; use async_trait::async_trait; -use digest::Digest; use secrecy::ExposeSecret; -use sha1::Sha1; use snafu::{ensure, OptionExt, ResultExt}; -use crate::common::Salt; use crate::error::{ Error, IllegalParamSnafu, InvalidConfigSnafu, IoSnafu, Result, UnsupportedPasswordTypeSnafu, UserNotFoundSnafu, UserPasswordMismatchSnafu, }; use crate::user_info::DefaultUserInfo; -use crate::{HashedPassword, Identity, Password, UserInfoRef, UserProvider}; +use crate::{auth_mysql, Identity, Password, UserInfoRef, UserProvider}; pub(crate) const STATIC_USER_PROVIDER: &str = "static_user_provider"; @@ -136,12 +133,6 @@ impl UserProvider for StaticUserProvider { }; } Password::MysqlNativePassword(auth_data, salt) => { - ensure!( - auth_data.len() == 20, - IllegalParamSnafu { - msg: "Illegal MySQL native password format, length != 20" - } - ); auth_mysql(auth_data, salt, username, save_pwd) .map(|_| DefaultUserInfo::with_name(username)) } @@ -165,48 +156,6 @@ impl UserProvider for StaticUserProvider { } } -pub fn auth_mysql( - auth_data: HashedPassword, - salt: Salt, - username: &str, - save_pwd: &[u8], -) -> Result<()> { - // ref: https://github.com/mysql/mysql-server/blob/a246bad76b9271cb4333634e954040a970222e0a/sql/auth/password.cc#L62 - let hash_stage_2 = double_sha1(save_pwd); - let tmp = sha1_two(salt, &hash_stage_2); - // xor auth_data and tmp - let mut xor_result = [0u8; 20]; - for i in 0..20 { - xor_result[i] = auth_data[i] ^ tmp[i]; - } - let candidate_stage_2 = sha1_one(&xor_result); - if candidate_stage_2 == hash_stage_2 { - Ok(()) - } else { - UserPasswordMismatchSnafu { - username: username.to_string(), - } - .fail() - } -} - -fn sha1_two(input_1: &[u8], input_2: &[u8]) -> Vec { - let mut hasher = Sha1::new(); - hasher.update(input_1); - hasher.update(input_2); - hasher.finalize().to_vec() -} - -fn sha1_one(data: &[u8]) -> Vec { - let mut hasher = Sha1::new(); - hasher.update(data); - hasher.finalize().to_vec() -} - -fn double_sha1(data: &[u8]) -> Vec { - sha1_one(&sha1_one(data)) -} - #[cfg(test)] pub mod test { use std::fs::File; @@ -215,36 +164,10 @@ pub mod test { use common_test_util::temp_dir::create_temp_dir; use crate::user_info::DefaultUserInfo; - use crate::user_provider::static_user_provider::{ - double_sha1, sha1_one, sha1_two, StaticUserProvider, - }; + use crate::user_provider::static_user_provider::StaticUserProvider; use crate::user_provider::{Identity, Password}; use crate::UserProvider; - #[test] - fn test_sha() { - let sha_1_answer: Vec = vec![ - 124, 74, 141, 9, 202, 55, 98, 175, 97, 229, 149, 32, 148, 61, 194, 100, 148, 248, 148, - 27, - ]; - let sha_1 = sha1_one("123456".as_bytes()); - assert_eq!(sha_1, sha_1_answer); - - let double_sha1_answer: Vec = vec![ - 107, 180, 131, 126, 183, 67, 41, 16, 94, 228, 86, 141, 218, 125, 198, 126, 210, 202, - 42, 217, - ]; - let double_sha1 = double_sha1("123456".as_bytes()); - assert_eq!(double_sha1, double_sha1_answer); - - let sha1_2_answer: Vec = vec![ - 132, 115, 215, 211, 99, 186, 164, 206, 168, 152, 217, 192, 117, 47, 240, 252, 142, 244, - 37, 204, - ]; - let sha1_2 = sha1_two("123456".as_bytes(), "654321".as_bytes()); - assert_eq!(sha1_2, sha1_2_answer); - } - async fn test_authenticate(provider: &dyn UserProvider, username: &str, password: &str) { let re = provider .authenticate( diff --git a/src/auth/tests/mod.rs b/src/auth/tests/mod.rs index 3cc0cbe2e7..d89b8925d1 100644 --- a/src/auth/tests/mod.rs +++ b/src/auth/tests/mod.rs @@ -17,7 +17,7 @@ use std::assert_matches::assert_matches; use std::sync::Arc; use api::v1::greptime_request::Request; -use auth::Error::InternalState; +use auth::error::Error::InternalState; use auth::{PermissionChecker, PermissionCheckerRef, PermissionReq, PermissionResp, UserInfoRef}; use sql::statements::show::{ShowDatabases, ShowKind}; use sql::statements::statement::Statement; @@ -29,7 +29,7 @@ impl PermissionChecker for DummyPermissionChecker { &self, _user_info: Option, req: PermissionReq, - ) -> auth::Result { + ) -> auth::error::Result { match req { PermissionReq::GrpcRequest(_) => Ok(PermissionResp::Allow), PermissionReq::SqlStatement(_) => Ok(PermissionResp::Reject), diff --git a/src/cmd/src/error.rs b/src/cmd/src/error.rs index 3fe37a96f4..3c55361e40 100644 --- a/src/cmd/src/error.rs +++ b/src/cmd/src/error.rs @@ -80,7 +80,7 @@ pub enum Error { #[snafu(display("Illegal auth config: {}", source))] IllegalAuthConfig { location: Location, - source: auth::Error, + source: auth::error::Error, }, #[snafu(display("Unsupported selector type, {} source: {}", selector_type, source))] diff --git a/src/common/error/src/status_code.rs b/src/common/error/src/status_code.rs index 81f8daa157..92ea598d04 100644 --- a/src/common/error/src/status_code.rs +++ b/src/common/error/src/status_code.rs @@ -84,6 +84,8 @@ pub enum StatusCode { InvalidAuthHeader = 7004, /// Illegal request to connect catalog-schema AccessDenied = 7005, + /// User is not authorized to perform the operation + PermissionDenied = 7006, // ====== End of auth related status code ===== } @@ -120,7 +122,8 @@ impl StatusCode { | StatusCode::UserPasswordMismatch | StatusCode::AuthHeaderNotFound | StatusCode::InvalidAuthHeader - | StatusCode::AccessDenied => false, + | StatusCode::AccessDenied + | StatusCode::PermissionDenied => false, } } @@ -151,7 +154,8 @@ impl StatusCode { | StatusCode::UserPasswordMismatch | StatusCode::AuthHeaderNotFound | StatusCode::InvalidAuthHeader - | StatusCode::AccessDenied => false, + | StatusCode::AccessDenied + | StatusCode::PermissionDenied => false, } } diff --git a/src/frontend/src/error.rs b/src/frontend/src/error.rs index e5c4d03c1a..07c393db32 100644 --- a/src/frontend/src/error.rs +++ b/src/frontend/src/error.rs @@ -593,7 +593,7 @@ pub enum Error { #[snafu(display("Failed to pass permission check, source: {}", source))] Permission { - source: auth::Error, + source: auth::error::Error, location: Location, }, } diff --git a/src/servers/src/error.rs b/src/servers/src/error.rs index 116d6b353a..ed930a0927 100644 --- a/src/servers/src/error.rs +++ b/src/servers/src/error.rs @@ -200,7 +200,7 @@ pub enum Error { #[snafu(display("Failed to get user info, source: {}", source))] Auth { location: Location, - source: auth::Error, + source: auth::error::Error, }, #[snafu(display("Not found http or grpc authorization header"))] @@ -451,7 +451,7 @@ fn status_to_tonic_code(status_code: StatusCode) -> Code { | StatusCode::UserPasswordMismatch | StatusCode::AuthHeaderNotFound | StatusCode::InvalidAuthHeader => Code::Unauthenticated, - StatusCode::AccessDenied => Code::PermissionDenied, + StatusCode::AccessDenied | StatusCode::PermissionDenied => Code::PermissionDenied, } } diff --git a/src/servers/src/mysql/handler.rs b/src/servers/src/mysql/handler.rs index 61668a8e01..0deebc02bb 100644 --- a/src/servers/src/mysql/handler.rs +++ b/src/servers/src/mysql/handler.rs @@ -186,7 +186,8 @@ impl AsyncMysqlShim for MysqlInstanceShi } }; } - let user_info = user_info.unwrap_or_else(|| auth::userinfo_by_name(None)); + let user_info = + user_info.unwrap_or_else(|| auth::userinfo_by_name(Some(username.to_string()))); self.session.set_user_info(user_info);