tustvold commented on code in PR #462:
URL: 
https://github.com/apache/arrow-rs-object-store/pull/462#discussion_r2267618476


##########
src/crypto.rs:
##########
@@ -0,0 +1,97 @@
+use std::fmt::{Debug, Display};
+
+type DynError = Box<dyn std::error::Error + Send + Sync>;
+
+#[derive(Debug)]
+pub struct Error {

Review Comment:
   I think it'd be better to just use object_store::Error, whilst it contains 
variants that aren't relevant, I don't think this matters



##########
src/crypto.rs:
##########
@@ -0,0 +1,97 @@
+use std::fmt::{Debug, Display};
+
+type DynError = Box<dyn std::error::Error + Send + Sync>;
+
+#[derive(Debug)]
+pub struct Error {
+    inner: DynError,
+}
+
+impl Display for Error {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        write!(f, "Crypto error: {}", self.inner)
+    }
+}
+
+impl std::error::Error for Error {}
+
+impl From<DynError> for Error {
+    fn from(err: DynError) -> Self {
+        Error { inner: err }
+    }
+}
+
+/// TODO(jakedern): Docs
+pub trait CryptoProvider: Send + Sync + Debug + 'static {
+    fn digest_sha256(bytes: &[u8]) -> Result<impl AsRef<[u8]>, Error>;
+    fn hmac_sha256(secret: &[u8], bytes: &[u8]) -> Result<impl AsRef<[u8]>, 
Error>;
+    fn hex_digest(bytes: &[u8]) -> Result<String, Error> {

Review Comment:
   Probably don't need this to be here unless you expect people to override it



##########
src/crypto.rs:
##########
@@ -0,0 +1,97 @@
+use std::fmt::{Debug, Display};
+
+type DynError = Box<dyn std::error::Error + Send + Sync>;
+
+#[derive(Debug)]
+pub struct Error {
+    inner: DynError,
+}
+
+impl Display for Error {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        write!(f, "Crypto error: {}", self.inner)
+    }
+}
+
+impl std::error::Error for Error {}
+
+impl From<DynError> for Error {
+    fn from(err: DynError) -> Self {
+        Error { inner: err }
+    }
+}
+
+/// TODO(jakedern): Docs
+pub trait CryptoProvider: Send + Sync + Debug + 'static {
+    fn digest_sha256(bytes: &[u8]) -> Result<impl AsRef<[u8]>, Error>;
+    fn hmac_sha256(secret: &[u8], bytes: &[u8]) -> Result<impl AsRef<[u8]>, 
Error>;
+    fn hex_digest(bytes: &[u8]) -> Result<String, Error> {
+        let digest = Self::digest_sha256(bytes)?;
+        Ok(hex_encode(digest.as_ref()))
+    }
+}
+
+pub(crate) fn hex_encode(bytes: &[u8]) -> String {
+    use std::fmt::Write;
+    let mut out = String::with_capacity(bytes.len() * 2);
+    for byte in bytes {
+        let _ = write!(out, "{byte:02x}");
+    }
+    out
+}
+
+/// TODO(jakedern): Docs
+#[cfg(feature = "ring")]
+pub mod ring_crypto {
+    use super::{CryptoProvider, Error};
+
+    #[derive(Debug, Clone, Copy)]
+    pub struct RingProvider;
+
+    impl CryptoProvider for RingProvider {
+        fn digest_sha256(bytes: &[u8]) -> Result<impl AsRef<[u8]>, Error> {
+            let digest = ring::digest::digest(&ring::digest::SHA256, bytes);
+            Ok(digest)
+        }
+
+        fn hmac_sha256(secret: &[u8], bytes: &[u8]) -> Result<impl 
AsRef<[u8]>, Error> {
+            let key = ring::hmac::Key::new(ring::hmac::HMAC_SHA256, secret);
+            let tag = ring::hmac::sign(&key, bytes);
+            Ok(tag)
+        }
+    }
+
+    impl Default for RingProvider {
+        fn default() -> Self {
+            RingProvider
+        }
+    }
+}
+
+pub mod openssl_crypto {

Review Comment:
   I would personally prefer people to add additional providers out of tree if 
they need them. I'm not massively excited about adding a dependency on openssl, 
optional or not.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to