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

Add Python Attribute Accessors and Cross-Library Compatibility for PkEncryption #15

Open
wants to merge 14 commits into
base: main
Choose a base branch
from
Open
58 changes: 55 additions & 3 deletions src/pk_encryption.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,12 @@
use pyo3::{
pyclass, pymethods,
types::{PyBytes, PyType},
Bound, Py, Python,
pyclass,
pymethods,
types::{PyBytes, PyString, PyType},
Bound,
IntoPyObject,
Py,
PyResult,
Python,
};

use crate::{
Expand Down Expand Up @@ -42,6 +47,53 @@ impl Message {
fn new(ciphertext: Vec<u8>, mac: Vec<u8>, ephemeral_key: Vec<u8>) -> Self {
Message { ciphertext, mac, ephemeral_key }
}

/// Create a new Message object from unpadded Base64-encoded components.
///
/// This function decodes the given Base64 strings and returns a `Message`
/// with the resulting byte vectors.
///
/// # Arguments
/// * `ciphertext` - Unpadded Base64-encoded ciphertext
/// * `mac` - Unpadded Base64-encoded message authentication code
/// * `ephemeral_key` - Unpadded Base64-encoded ephemeral key
#[classmethod]
fn from_base64(
_cls: &Bound<'_, PyType>,
ciphertext: &str,
mac: &str,
ephemeral_key: &str,
) -> Self {
let decoded_ciphertext =
vodozemac::base64_decode(ciphertext).expect("Failed to decode ciphertext");
Copy link
Collaborator

Choose a reason for hiding this comment

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

Oh no, we can't just expect() here. We should return a proper error. In other words this method needs to return a Result<Self, SomeError> type.

You can probably just extend the PK error type like such:

diff --git a/src/error.rs b/src/error.rs
index c91c256..b26716e 100644
--- a/src/error.rs
+++ b/src/error.rs
@@ -136,6 +136,8 @@ pub enum PkEncryptionError {
     InvalidKeySize(usize),
     #[error(transparent)]
     Decode(#[from] vodozemac::pk_encryption::Error),
+    #[error(transparent)]
+    Mac(#[from] vodozemac::Base64DecodeError),
 }
 
 pyo3::create_exception!(module, PkInvalidKeySizeException, pyo3::exceptions::PyValueError);
@@ -148,6 +150,7 @@ impl From<PkEncryptionError> for PyErr {
                 PkInvalidKeySizeException::new_err(e.to_string())
             }
             PkEncryptionError::Decode(_) => PkDecodeException::new_err(e.to_string()),
+            PkEncryptionError::Mac(_) => PkDecodeException::new_err(e.to_string()),
         }
     }
 }

Copy link
Author

Choose a reason for hiding this comment

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

Ok, i see - yes.
Please double-check this, if this fits the purpose. 
I also added a test for that exception.

let decoded_mac = vodozemac::base64_decode(mac).expect("Failed to decode mac");
let decoded_ephemeral_key =
vodozemac::base64_decode(ephemeral_key).expect("Failed to decode ephemeral_key");

Self {
ciphertext: decoded_ciphertext,
mac: decoded_mac,
ephemeral_key: decoded_ephemeral_key,
}
}

/// Convert the message components to unpadded Base64-encoded strings.
///
/// Returns a tuple of (ciphertext, mac, ephemeral_key) as unpadded Base64 strings.
fn to_base64<'py>(
poljar marked this conversation as resolved.
Show resolved Hide resolved
&self,
py: Python<'py>,
) -> PyResult<(Bound<'py, PyString>, Bound<'py, PyString>, Bound<'py, PyString>)> {
let ciphertext_b64 = vodozemac::base64_encode(&self.ciphertext);
let mac_b64 = vodozemac::base64_encode(&self.mac);
let ephemeral_key_b64 = vodozemac::base64_encode(&self.ephemeral_key);

Ok((
ephemeral_key_b64.into_pyobject(py)?,
mac_b64.into_pyobject(py)?,
ciphertext_b64.into_pyobject(py)?,
))
}
}

/// ☣️ Compat support for libolm's PkDecryption.
Expand Down