tustvold commented on code in PR #585:
URL:
https://github.com/apache/arrow-rs-object-store/pull/585#discussion_r2637300633
##########
src/gcp/credential.rs:
##########
@@ -123,44 +104,43 @@ pub struct GcpSigningCredential {
}
/// A private RSA key for a service account
-#[derive(Debug)]
-pub struct ServiceAccountKey(RsaKeyPair);
+pub struct ServiceAccountKey(Box<dyn Signer>);
+
+impl std::fmt::Debug for ServiceAccountKey {
+ fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+ f.debug_tuple("ServiceAccountKey").finish_non_exhaustive()
+ }
+}
impl ServiceAccountKey {
+ /// Creates a [`ServiceAccountKey`] from the provided [`Signer`]
+ pub fn new(signer: Box<dyn Signer>) -> Self {
+ Self(signer)
+ }
+
/// Parses a pem-encoded RSA key
- pub fn from_pem(encoded: &[u8]) -> Result<Self> {
- use rustls_pki_types::PrivateKeyDer;
- use rustls_pki_types::pem::PemObject;
-
- match PrivateKeyDer::from_pem_slice(encoded) {
- Ok(PrivateKeyDer::Pkcs8(key)) =>
Self::from_pkcs8(key.secret_pkcs8_der()),
- Ok(PrivateKeyDer::Pkcs1(key)) =>
Self::from_der(key.secret_pkcs1_der()),
- Ok(_) => Err(Error::MissingKey),
- Err(source) => Err(Error::ReadPem { source }),
- }
+ #[cfg(feature = "ring")]
+ pub fn from_pem(encoded: &[u8]) -> crate::Result<Self> {
Review Comment:
This is technically a breaking change, however, the previous error type was
not public and therefore there wasn't any way to actually name it. I think we
can therefore get away with this change
##########
Cargo.toml:
##########
@@ -71,12 +71,24 @@ wasm-bindgen-futures = "0.4.18"
[features]
default = ["fs"]
-cloud = ["serde", "serde_json", "quick-xml", "hyper", "reqwest",
"reqwest/stream", "chrono/serde", "base64", "rand", "ring", "http-body-util",
"form_urlencoded", "serde_urlencoded"]
-azure = ["cloud", "httparse"]
+cloud-no-crypto = ["serde", "serde_json", "quick-xml", "hyper", "reqwest",
"reqwest/stream", "chrono/serde", "base64", "rand","http-body-util",
"form_urlencoded", "serde_urlencoded"]
Review Comment:
I'm not a massive fan of the feature explosion, but this was the only way to
avoid needing to cut a breaking release.
##########
src/client/crypto.rs:
##########
@@ -0,0 +1,235 @@
+use crate::Result;
+
+/// Algorithm for computing digests
+#[derive(Debug, Ord, PartialOrd, Eq, PartialEq)]
+#[non_exhaustive]
+pub enum DigestAlgorithm {
+ /// SHA-256
+ Sha256,
+}
+
+/// Algorithm for signing payloads
+#[derive(Debug, Ord, PartialOrd, Eq, PartialEq)]
+#[non_exhaustive]
+pub enum SigningAlgorithm {
+ /// RSASSA-PKCS1-v1_5 using SHA-256
+ RS256,
+}
+
+/// Provides cryptographic primitives
+pub trait CryptoProvider: std::fmt::Debug + Send + Sync {
+ /// Compute a digest
+ fn digest(&self, algorithm: DigestAlgorithm) -> Result<Box<dyn
DigestContext>>;
+
+ /// Compute an HMAC with the provided `secret`
+ fn hmac(&self, algorithm: DigestAlgorithm, secret: &[u8]) ->
Result<Box<dyn HmacContext>>;
+
+ /// Sign a payload with the provided PEM-encoded secret
+ fn sign(&self, algorithm: SigningAlgorithm, pem: &[u8]) -> Result<Box<dyn
Signer>>;
+}
+
+/// Incrementally compute a digest, see [`CryptoProvider::digest`]
+pub trait DigestContext: Send {
+ ///Updates the digest with all the data in data.
+ ///
+ /// It is implementation-defined behaviour to call this after calling
[`Self::finish`]
+ fn update(&mut self, data: &[u8]);
+
+ /// Finalizes the digest calculation and returns the digest value.
+ ///
+ /// It is implementation-defined behaviour to call this after calling
[`Self::finish`]
+ fn finish(&mut self) -> Result<&[u8]>;
+}
+
+/// Incrementally compute a HMAC, see [`CryptoProvider::hmac`]
+pub trait HmacContext: Send {
+ /// Updates the HMAC with all the data in data.
+ ///
+ /// It is implementation-defined behaviour to call this after calling
[`Self::finish`]
+ fn update(&mut self, data: &[u8]);
+
+ /// Finalizes the HMAC calculation and returns the HMAC value.
+ ///
+ /// It is implementation-defined behaviour to call this after calling
[`Self::finish`]
+ fn finish(&mut self) -> Result<&[u8]>;
+}
+
+/// Sign a payload, see [`CryptoProvider::sign`]
+pub trait Signer: Send + Sync {
+ /// Sign the provided payload
+ fn sign(&self, string_to_sign: &[u8]) -> Result<Vec<u8>>;
+}
+
+/// Attempts to find a [`CryptoProvider`]
+///
+/// If `custom` is `Some(v)` returns `v` otherwise returns the compile-time
default
+#[cfg(feature = "ring")]
+#[inline]
+pub(crate) fn crypto_provider(custom: Option<&dyn CryptoProvider>) ->
Result<&dyn CryptoProvider> {
Review Comment:
This formulation is a bit funky, but was the best I could come up with that
would:
* Avoid needing breaking changes to AwsAuthorizer and AzureAuthorizer
* Allow not specifying a crypto provider if skip_signature is enabled
##########
src/client/crypto.rs:
##########
@@ -0,0 +1,235 @@
+use crate::Result;
+
+/// Algorithm for computing digests
+#[derive(Debug, Ord, PartialOrd, Eq, PartialEq)]
+#[non_exhaustive]
+pub enum DigestAlgorithm {
+ /// SHA-256
+ Sha256,
+}
+
+/// Algorithm for signing payloads
+#[derive(Debug, Ord, PartialOrd, Eq, PartialEq)]
+#[non_exhaustive]
+pub enum SigningAlgorithm {
+ /// RSASSA-PKCS1-v1_5 using SHA-256
+ RS256,
+}
+
+/// Provides cryptographic primitives
+pub trait CryptoProvider: std::fmt::Debug + Send + Sync {
+ /// Compute a digest
+ fn digest(&self, algorithm: DigestAlgorithm) -> Result<Box<dyn
DigestContext>>;
+
+ /// Compute an HMAC with the provided `secret`
+ fn hmac(&self, algorithm: DigestAlgorithm, secret: &[u8]) ->
Result<Box<dyn HmacContext>>;
+
+ /// Sign a payload with the provided PEM-encoded secret
+ fn sign(&self, algorithm: SigningAlgorithm, pem: &[u8]) -> Result<Box<dyn
Signer>>;
+}
+
+/// Incrementally compute a digest, see [`CryptoProvider::digest`]
+pub trait DigestContext: Send {
+ ///Updates the digest with all the data in data.
+ ///
+ /// It is implementation-defined behaviour to call this after calling
[`Self::finish`]
+ fn update(&mut self, data: &[u8]);
Review Comment:
I debated making this method fallible, but decided any error can be returned
by `finish`
##########
src/client/crypto.rs:
##########
@@ -0,0 +1,235 @@
+use crate::Result;
+
+/// Algorithm for computing digests
+#[derive(Debug, Ord, PartialOrd, Eq, PartialEq)]
+#[non_exhaustive]
+pub enum DigestAlgorithm {
+ /// SHA-256
+ Sha256,
+}
+
+/// Algorithm for signing payloads
+#[derive(Debug, Ord, PartialOrd, Eq, PartialEq)]
+#[non_exhaustive]
+pub enum SigningAlgorithm {
+ /// RSASSA-PKCS1-v1_5 using SHA-256
+ RS256,
+}
+
+/// Provides cryptographic primitives
+pub trait CryptoProvider: std::fmt::Debug + Send + Sync {
+ /// Compute a digest
+ fn digest(&self, algorithm: DigestAlgorithm) -> Result<Box<dyn
DigestContext>>;
+
+ /// Compute an HMAC with the provided `secret`
+ fn hmac(&self, algorithm: DigestAlgorithm, secret: &[u8]) ->
Result<Box<dyn HmacContext>>;
+
+ /// Sign a payload with the provided PEM-encoded secret
+ fn sign(&self, algorithm: SigningAlgorithm, pem: &[u8]) -> Result<Box<dyn
Signer>>;
+}
+
+/// Incrementally compute a digest, see [`CryptoProvider::digest`]
Review Comment:
I opted to allow for incremental computation to somewhat future-proof this
API
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
