Copilot commented on code in PR #2225:
URL: https://github.com/apache/nifi-minifi-cpp/pull/2225#discussion_r3923799365
##########
minifi_rust/extensions/minifi_pgp/src/processors/decrypt_content.rs:
##########
@@ -0,0 +1,356 @@
+mod processor_definition;
+
+use processor_definition::*;
+
+use crate::controller_services::private_key_service::PGPPrivateKeyService;
+
+use minifi_native::macros::{ComponentIdentifier, PropertyType};
+use minifi_native::{
+ FlowFileStreamTransform, GetControllerService, GetProperty, InputStream,
Logger, MinifiError,
+ OutputStream, ProcessError, RouteErrorExt, Schedule, TransformStreamResult,
+};
+use pgp::composed::{Message, TheRing};
+use std::fmt::Debug;
+use strum_macros::{Display, EnumString, IntoStaticStr, VariantNames};
+
+#[derive(
+ Debug, Clone, Copy, PartialEq, Display, EnumString, VariantNames,
IntoStaticStr, PropertyType,
+)]
+#[strum(serialize_all = "UPPERCASE", const_into_str)]
+enum DecryptionStrategy {
+ Decrypted,
+ Packaged,
+}
+
+#[derive(Debug, ComponentIdentifier)]
+pub(crate) struct DecryptContentPGP {
+ decompress_data: bool,
+ symmetric_password: Option<pgp::types::Password>,
+}
+
+impl Schedule for DecryptContentPGP {
+ fn schedule<P: GetProperty, L>(context: &P, _logger: &L) -> Result<Self,
MinifiError>
+ where
+ Self: Sized,
+ L: Logger,
+ {
+ let decryption_strategy = context.get_property(&DECRYPTION_STRATEGY)?;
+
+ let symmetric_password = context.get_property(&SYMMETRIC_PASSWORD)?;
+ let has_context_service =
context.get_raw_property(&PRIVATE_KEY_SERVICE)?.is_some();
+ if !has_context_service && symmetric_password.is_none() {
+ Err(MinifiError::validation(
+ "Either Symmetric Password or Private Key Service must be set",
+ ))
+ } else {
+ Ok(DecryptContentPGP {
+ decompress_data: decryption_strategy ==
DecryptionStrategy::Decrypted,
+ symmetric_password,
+ })
+ }
+ }
+}
+
+impl DecryptContentPGP {
+ fn decrypt_msg<'a>(
+ &'a self,
+ msg: Message<'a>,
+ private_key_service: Option<&'a PGPPrivateKeyService>,
+ ) -> pgp::errors::Result<Message<'a>> {
+ let mut ring = if let Some(pks) = private_key_service {
+ pks.get_the_ring()
+ } else {
+ TheRing::default()
+ };
+
+ ring.decrypt_options = ring.decrypt_options.enable_gnupg_aead();
+
+ if let Some(sym_passwd) = &self.symmetric_password {
+ ring.message_password.push(sym_passwd);
+ }
+ let (decrypted_msg, _ring_result) = msg.decrypt_the_ring(ring, false)?;
+ Ok(decrypted_msg)
+ }
+
+ fn extract_attributes_from_decrypted_message(
+ decrypted_msg: &Message,
+ ) -> Vec<(&'static str, String)> {
+ let mut res = Vec::new();
+ if let Some(literal_data_header) = decrypted_msg.literal_data_header()
{
+ if let Ok(file_name) =
str::from_utf8(literal_data_header.file_name()) {
+ res.push((LITERAL_DATA_FILENAME.name, file_name.to_string()));
+ }
+ // NiFi uses ms timestamp
+ res.push((
+ LITERAL_DATA_MODIFIED.name,
+ (1000u64 * literal_data_header.created().as_secs() as
u64).to_string(),
+ ));
+ }
+ res
+ }
+}
+
+impl FlowFileStreamTransform for DecryptContentPGP {
+ fn transform<Ctx: GetProperty + GetControllerService, LoggerImpl: Logger>(
+ &self,
+ context: &Ctx,
+ input_stream: &mut dyn InputStream,
+ output_stream: &mut dyn OutputStream,
+ _logger: &LoggerImpl,
+ ) -> Result<TransformStreamResult, ProcessError> {
+ let private_key_service =
context.get_controller_service(&PRIVATE_KEY_SERVICE)?;
+
+ let msg = Message::from_reader(input_stream)
+ .map(|(msg, _header)| msg)
+ .route_err_to_failure()?;
+
+ let mut decrypted_msg = self
+ .decrypt_msg(msg, private_key_service)
+ .route_err_to_failure()?;
+
+ if self.decompress_data && decrypted_msg.is_compressed() {
+ decrypted_msg = decrypted_msg
+ .decompress()
+ .map_err(MinifiError::other)
+ .route_err_to_failure()?
+ };
+
+ let attributes_to_add =
Self::extract_attributes_from_decrypted_message(&decrypted_msg);
+ let _written_bytes =
+ std::io::copy(&mut decrypted_msg.into_inner(),
output_stream).route_err_to_failure()?;
Review Comment:
The `PACKAGED` path leaves `decrypted_msg` as a parsed
`Message::Compressed`, then `into_inner()` returns the remaining packet body
rather than a complete OpenPGP packet (the packet header has already been
consumed). The resulting output cannot reliably be parsed by a subsequent
signature-verification processor, contrary to this strategy's contract.
Preserve or re-serialize the complete decrypted packet stream for `PACKAGED`;
only unwrap to literal payload for `DECRYPTED`.
##########
minifi_rust/extensions/minifi_pgp/src/utils.rs:
##########
@@ -0,0 +1,16 @@
+use minifi_native::{MinifiError, PropertyConstraints, PropertySchema,
PropertyType};
+
+pub(crate) struct Password {}
+
+impl PropertySchema for Password {
+ const CONSTRAINT: Option<PropertyConstraints> = None;
Review Comment:
This accepts an explicitly configured empty password. Because
`Option<Password>` turns `""` into `Some(empty)`, the encryption schedule check
passes and creates password-based encryption with no secret. Apply the
non-blank validator used for credential properties; users of unencrypted
private keys can leave the optional passphrase property unset.
##########
minifi_rust/extensions/minifi_pgp/src/processors/encrypt_content.rs:
##########
@@ -0,0 +1,286 @@
+use minifi_native::{
+ FlowFileStreamTransform, GetAttribute, GetControllerService, GetId,
GetProperty, InputStream,
+ Logger, MinifiError, OutputStream, ProcessError, RouteErrorExt, Schedule,
+ TransformStreamResult,
+};
+use pgp::composed::{ArmorOptions, MessageBuilder, SignedPublicKey};
+use pgp::types::{Password, StringToKey};
+
+mod processor_definition;
+
+use processor_definition::*;
+
+use minifi_native::macros::{ComponentIdentifier, PropertyType};
+use strum_macros::{Display, EnumString, IntoStaticStr, VariantNames};
+
+#[derive(
+ Debug, Clone, Copy, PartialEq, Display, EnumString, VariantNames,
IntoStaticStr, PropertyType,
+)]
+#[strum(serialize_all = "UPPERCASE", const_into_str)]
+enum FileEncoding {
+ Ascii,
+ Binary,
+}
+
+#[derive(Debug, ComponentIdentifier)]
+pub(crate) struct EncryptContentPGP {
+ file_encoding: FileEncoding,
+ symmetric_password: Option<Password>,
+}
+
+#[cfg(not(test))]
+fn string_to_key() -> StringToKey {
+ StringToKey::new_argon2(rand::thread_rng(), 3, 4, 16) // 64 MiB with
rpgp's recommended parameter choice
+}
+
+#[cfg(test)]
+fn string_to_key() -> StringToKey {
+ StringToKey::new_argon2(rand::thread_rng(), 1, 1, 10) // fast for unit
tests
+}
+
+impl EncryptContentPGP {
+ fn encrypt_bytes(
+ &self,
+ input_stream: &mut dyn InputStream,
+ output_stream: &mut dyn OutputStream,
+ pub_key: Option<&SignedPublicKey>,
+ file_name: String,
+ ) -> Result<(), MinifiError> {
+ if pub_key.is_none() && self.symmetric_password.is_none() {
+ return Err(MinifiError::custom(
+ "No password or public key to encrypt with",
+ ));
+ }
+
+ let mut builder = MessageBuilder::from_reader(file_name,
input_stream).seipd_v1(
+ rand::thread_rng(),
+ pgp::crypto::sym::SymmetricKeyAlgorithm::AES256,
+ );
+
+ if let Some(pub_key) = pub_key {
+ builder
+ .encrypt_to_key(rand::thread_rng(), pub_key)
+ .map_err(MinifiError::other)?;
+ }
+
+ if let Some(password) = &self.symmetric_password {
+ builder
+ .encrypt_with_password(string_to_key(), password)
+ .map_err(MinifiError::other)?;
+ }
+
+ match self.file_encoding {
+ FileEncoding::Ascii => builder
+ .to_armored_writer(rand::thread_rng(),
ArmorOptions::default(), output_stream)
+ .map_err(MinifiError::other),
+ FileEncoding::Binary => builder
+ .to_writer(rand::thread_rng(), output_stream)
+ .map_err(MinifiError::other),
+ }
+ }
+
+ fn check_validity(password: &Option<Password>, has_pub_key: bool) ->
Result<(), MinifiError> {
+ if password.is_none() && !has_pub_key {
+ Err(MinifiError::custom(
+ "Either a password or Public Key Service with Public Key
Search should be configured to encrypt files",
+ ))
+ } else {
+ Ok(())
+ }
+ }
+}
+
+impl Schedule for EncryptContentPGP {
+ fn schedule<P: GetProperty, L: Logger>(context: &P, _logger: &L) ->
Result<Self, MinifiError>
+ where
+ Self: Sized,
+ {
+ let file_encoding =
context.get_property::<FileEncoding>(&FILE_ENCODING)?;
+ let symmetric_password = context.get_property(&PASSWORD)?;
+
+ let has_public_key =
context.get_raw_property(&PUBLIC_KEY_SERVICE)?.is_some()
+ && context.get_property(&PUBLIC_KEY_SEARCH)?.is_some();
+
+ Self::check_validity(&symmetric_password, has_public_key)?;
+ Ok(EncryptContentPGP {
+ file_encoding,
+ symmetric_password,
+ })
+ }
+}
+
+impl EncryptContentPGP {
+ fn get_public_key<Ctx: GetProperty + GetControllerService>(
+ context: &Ctx,
+ ) -> Result<Option<&SignedPublicKey>, MinifiError> {
+ if let (Some(pub_key_search), Some(public_key_service)) = (
+ context.get_property(&PUBLIC_KEY_SEARCH)?,
+ context.get_controller_service(&PUBLIC_KEY_SERVICE)?,
+ ) {
+ Ok(public_key_service.get(&pub_key_search))
+ } else {
+ Ok(None)
+ }
Review Comment:
When a public-key search is configured but has no match, this returns
`None`. If a symmetric password is also configured, `encrypt_bytes` then
succeeds with password-only encryption, silently dropping the requested
public-key recipient. Treat a failed configured lookup as an error so
encryption routes to failure rather than weakening the configured protection.
##########
minifi_rust/extensions/minifi_pgp/src/controller_services/private_key_service/controller_service_definition.rs:
##########
@@ -0,0 +1,31 @@
+use super::PGPPrivateKeyService;
+use crate::controller_services::key_file_property::SecretKeyFile;
+use crate::controller_services::key_property::SecretKey;
+use crate::utils;
+use minifi_native::{
+ ControllerServiceDefinition, Property, PropertyDefinition,
ProvidedInterface,
+ property_definitions,
+};
+
+pub(super) const KEY_FILE: Property<Option<SecretKeyFile>> = Property::new(
+ "Key File",
+ "File path to PGP Secret Key encoded in binary or ASCII Armor",
+)
+.supports_expression_language();
Review Comment:
This advertises FlowFile-attribute expression language for a
controller-service property, but controller services are enabled without a
FlowFile and their context API has no FlowFile argument. Such expressions
cannot be evaluated as advertised. Remove this flag unless the Rust property
API gains the environment-only expression scope intended for controller-service
paths.
##########
minifi_rust/extensions/minifi_pgp/src/controller_services/public_key_service/controller_service_definition.rs:
##########
@@ -0,0 +1,26 @@
+use super::PGPPublicKeyService;
+use crate::controller_services::key_file_property::PublicKeyFile;
+use crate::controller_services::key_property::PublicKey;
+use minifi_native::{
+ ControllerServiceDefinition, Property, PropertyDefinition,
ProvidedInterface,
+ property_definitions,
+};
+
+pub(crate) const KEYRING_FILE: Property<Option<PublicKeyFile>> = Property::new(
+ "Keyring File",
+ "File path to PGP Keyring or Secret Key encoded in binary or ASCII Armor",
+)
+.supports_expression_language();
Review Comment:
This advertises FlowFile-attribute expression language for a
controller-service property, but controller services are enabled without a
FlowFile and their context API has no FlowFile argument. Such expressions
cannot be evaluated as advertised. Remove this flag unless the Rust property
API gains the environment-only expression scope intended for controller-service
paths.
##########
minifi_rust/extensions/minifi_pgp/src/processors/encrypt_content.rs:
##########
@@ -0,0 +1,286 @@
+use minifi_native::{
+ FlowFileStreamTransform, GetAttribute, GetControllerService, GetId,
GetProperty, InputStream,
+ Logger, MinifiError, OutputStream, ProcessError, RouteErrorExt, Schedule,
+ TransformStreamResult,
+};
+use pgp::composed::{ArmorOptions, MessageBuilder, SignedPublicKey};
+use pgp::types::{Password, StringToKey};
+
+mod processor_definition;
+
+use processor_definition::*;
+
+use minifi_native::macros::{ComponentIdentifier, PropertyType};
+use strum_macros::{Display, EnumString, IntoStaticStr, VariantNames};
+
+#[derive(
+ Debug, Clone, Copy, PartialEq, Display, EnumString, VariantNames,
IntoStaticStr, PropertyType,
+)]
+#[strum(serialize_all = "UPPERCASE", const_into_str)]
+enum FileEncoding {
+ Ascii,
+ Binary,
+}
+
+#[derive(Debug, ComponentIdentifier)]
+pub(crate) struct EncryptContentPGP {
+ file_encoding: FileEncoding,
+ symmetric_password: Option<Password>,
+}
+
+#[cfg(not(test))]
+fn string_to_key() -> StringToKey {
+ StringToKey::new_argon2(rand::thread_rng(), 3, 4, 16) // 64 MiB with
rpgp's recommended parameter choice
+}
+
+#[cfg(test)]
+fn string_to_key() -> StringToKey {
+ StringToKey::new_argon2(rand::thread_rng(), 1, 1, 10) // fast for unit
tests
+}
+
+impl EncryptContentPGP {
+ fn encrypt_bytes(
+ &self,
+ input_stream: &mut dyn InputStream,
+ output_stream: &mut dyn OutputStream,
+ pub_key: Option<&SignedPublicKey>,
+ file_name: String,
+ ) -> Result<(), MinifiError> {
+ if pub_key.is_none() && self.symmetric_password.is_none() {
+ return Err(MinifiError::custom(
+ "No password or public key to encrypt with",
+ ));
+ }
+
+ let mut builder = MessageBuilder::from_reader(file_name,
input_stream).seipd_v1(
+ rand::thread_rng(),
+ pgp::crypto::sym::SymmetricKeyAlgorithm::AES256,
+ );
+
+ if let Some(pub_key) = pub_key {
+ builder
+ .encrypt_to_key(rand::thread_rng(), pub_key)
+ .map_err(MinifiError::other)?;
+ }
+
+ if let Some(password) = &self.symmetric_password {
+ builder
+ .encrypt_with_password(string_to_key(), password)
+ .map_err(MinifiError::other)?;
+ }
+
+ match self.file_encoding {
+ FileEncoding::Ascii => builder
+ .to_armored_writer(rand::thread_rng(),
ArmorOptions::default(), output_stream)
+ .map_err(MinifiError::other),
+ FileEncoding::Binary => builder
+ .to_writer(rand::thread_rng(), output_stream)
+ .map_err(MinifiError::other),
+ }
+ }
+
+ fn check_validity(password: &Option<Password>, has_pub_key: bool) ->
Result<(), MinifiError> {
+ if password.is_none() && !has_pub_key {
+ Err(MinifiError::custom(
+ "Either a password or Public Key Service with Public Key
Search should be configured to encrypt files",
+ ))
+ } else {
+ Ok(())
+ }
+ }
+}
+
+impl Schedule for EncryptContentPGP {
+ fn schedule<P: GetProperty, L: Logger>(context: &P, _logger: &L) ->
Result<Self, MinifiError>
+ where
+ Self: Sized,
+ {
+ let file_encoding =
context.get_property::<FileEncoding>(&FILE_ENCODING)?;
+ let symmetric_password = context.get_property(&PASSWORD)?;
+
+ let has_public_key =
context.get_raw_property(&PUBLIC_KEY_SERVICE)?.is_some()
+ && context.get_property(&PUBLIC_KEY_SEARCH)?.is_some();
Review Comment:
`PUBLIC_KEY_SEARCH` supports FlowFile expression language, but this
schedule-time `get_property` call has no FlowFile and therefore evaluates the
expression without its attributes. A valid configuration such as `${recipient}`
can evaluate empty and prevent scheduling. Check the raw configured value here
and defer evaluation to `transform`, where the FlowFile is available.
##########
minifi_rust/extensions/minifi_pgp/src/processors/decrypt_content.rs:
##########
@@ -0,0 +1,356 @@
+mod processor_definition;
+
+use processor_definition::*;
+
+use crate::controller_services::private_key_service::PGPPrivateKeyService;
+
+use minifi_native::macros::{ComponentIdentifier, PropertyType};
+use minifi_native::{
+ FlowFileStreamTransform, GetControllerService, GetProperty, InputStream,
Logger, MinifiError,
+ OutputStream, ProcessError, RouteErrorExt, Schedule, TransformStreamResult,
+};
+use pgp::composed::{Message, TheRing};
+use std::fmt::Debug;
+use strum_macros::{Display, EnumString, IntoStaticStr, VariantNames};
+
+#[derive(
+ Debug, Clone, Copy, PartialEq, Display, EnumString, VariantNames,
IntoStaticStr, PropertyType,
+)]
+#[strum(serialize_all = "UPPERCASE", const_into_str)]
+enum DecryptionStrategy {
+ Decrypted,
+ Packaged,
+}
+
+#[derive(Debug, ComponentIdentifier)]
+pub(crate) struct DecryptContentPGP {
+ decompress_data: bool,
+ symmetric_password: Option<pgp::types::Password>,
+}
+
+impl Schedule for DecryptContentPGP {
+ fn schedule<P: GetProperty, L>(context: &P, _logger: &L) -> Result<Self,
MinifiError>
+ where
+ Self: Sized,
+ L: Logger,
+ {
+ let decryption_strategy = context.get_property(&DECRYPTION_STRATEGY)?;
+
+ let symmetric_password = context.get_property(&SYMMETRIC_PASSWORD)?;
+ let has_context_service =
context.get_raw_property(&PRIVATE_KEY_SERVICE)?.is_some();
+ if !has_context_service && symmetric_password.is_none() {
+ Err(MinifiError::validation(
+ "Either Symmetric Password or Private Key Service must be set",
+ ))
+ } else {
+ Ok(DecryptContentPGP {
+ decompress_data: decryption_strategy ==
DecryptionStrategy::Decrypted,
+ symmetric_password,
+ })
+ }
+ }
+}
+
+impl DecryptContentPGP {
+ fn decrypt_msg<'a>(
+ &'a self,
+ msg: Message<'a>,
+ private_key_service: Option<&'a PGPPrivateKeyService>,
+ ) -> pgp::errors::Result<Message<'a>> {
+ let mut ring = if let Some(pks) = private_key_service {
+ pks.get_the_ring()
+ } else {
+ TheRing::default()
+ };
+
+ ring.decrypt_options = ring.decrypt_options.enable_gnupg_aead();
+
+ if let Some(sym_passwd) = &self.symmetric_password {
+ ring.message_password.push(sym_passwd);
+ }
+ let (decrypted_msg, _ring_result) = msg.decrypt_the_ring(ring, false)?;
+ Ok(decrypted_msg)
+ }
+
+ fn extract_attributes_from_decrypted_message(
+ decrypted_msg: &Message,
+ ) -> Vec<(&'static str, String)> {
+ let mut res = Vec::new();
+ if let Some(literal_data_header) = decrypted_msg.literal_data_header()
{
+ if let Ok(file_name) =
str::from_utf8(literal_data_header.file_name()) {
+ res.push((LITERAL_DATA_FILENAME.name, file_name.to_string()));
+ }
+ // NiFi uses ms timestamp
+ res.push((
+ LITERAL_DATA_MODIFIED.name,
+ (1000u64 * literal_data_header.created().as_secs() as
u64).to_string(),
+ ));
+ }
+ res
+ }
+}
+
+impl FlowFileStreamTransform for DecryptContentPGP {
+ fn transform<Ctx: GetProperty + GetControllerService, LoggerImpl: Logger>(
+ &self,
+ context: &Ctx,
+ input_stream: &mut dyn InputStream,
+ output_stream: &mut dyn OutputStream,
+ _logger: &LoggerImpl,
+ ) -> Result<TransformStreamResult, ProcessError> {
+ let private_key_service =
context.get_controller_service(&PRIVATE_KEY_SERVICE)?;
+
+ let msg = Message::from_reader(input_stream)
+ .map(|(msg, _header)| msg)
+ .route_err_to_failure()?;
+
+ let mut decrypted_msg = self
+ .decrypt_msg(msg, private_key_service)
+ .route_err_to_failure()?;
+
+ if self.decompress_data && decrypted_msg.is_compressed() {
+ decrypted_msg = decrypted_msg
+ .decompress()
+ .map_err(MinifiError::other)
+ .route_err_to_failure()?
+ };
+
+ let attributes_to_add =
Self::extract_attributes_from_decrypted_message(&decrypted_msg);
+ let _written_bytes =
+ std::io::copy(&mut decrypted_msg.into_inner(),
output_stream).route_err_to_failure()?;
+
+
Ok(TransformStreamResult::new(&SUCCESS).with_attributes(attributes_to_add))
+ }
+}
+
+#[cfg(test)]
+mod tests {
+ use super::*;
+ use crate::test_utils;
+ use crate::test_utils::get_test_message;
+ use minifi_native::{
+ ComponentIdentifier, EnableControllerService, IoState,
MockControllerServiceContext,
+ MockLogger, MockProcessContext, test,
+ };
+ #[test]
+ fn test_ids() {
+ assert_eq!(
+ DecryptContentPGP::CLASS_NAME,
+ "minifi_pgp::processors::decrypt_content::DecryptContentPGP"
+ );
+ assert_eq!(DecryptContentPGP::GROUP_NAME, "minifi_pgp");
+ assert_eq!(DecryptContentPGP::VERSION, "1.0.0");
+ }
+
+ #[test]
+ fn fails_to_schedule_by_default() {
+ let decrypt_content =
+ DecryptContentPGP::schedule(&MockProcessContext::new(),
&MockLogger::new());
+ assert!(decrypt_content.is_err());
+ }
+
+ #[test]
+ fn schedules_with_password() {
+ let mut context = MockProcessContext::new();
+ context
+ .properties
+ .insert(SYMMETRIC_PASSWORD.name(),
"my_secret_password".to_string());
+ let decrypt_content = DecryptContentPGP::schedule(&context,
&MockLogger::new());
+ assert!(decrypt_content.is_ok());
+ }
+
+ #[test]
+ fn schedules_with_controller() {
+ let mut context = MockProcessContext::new();
+ context.properties.insert(
+ PRIVATE_KEY_SERVICE.name(),
+ "my_private_key_service".to_string(),
+ );
+ let decrypt_content = DecryptContentPGP::schedule(&context,
&MockLogger::new());
+ assert!(decrypt_content.is_ok());
+ }
+
+ #[test]
+ fn schedule_rejects_invalid_strategy_without_panicking() {
+ let mut context = MockProcessContext::new();
+ context
+ .properties
+ .insert(DECRYPTION_STRATEGY.name(), "NOT_A_STRATEGY".to_string());
+ context
+ .properties
+ .insert(SYMMETRIC_PASSWORD.name(),
"my_secret_password".to_string());
+ // Must return Err, not panic.
+ let result = DecryptContentPGP::schedule(&context, &MockLogger::new());
+ assert!(result.is_err(), "expected schedule to fail on bad strategy");
+ }
+
+ #[derive(Copy, Clone)]
+ struct PrivateKeyData {
+ key_filename: &'static str,
+ passphrase: Option<&'static str>,
+ }
+
+ impl PrivateKeyData {
+ fn into_controller(self) -> PGPPrivateKeyService {
+ let mut context = MockControllerServiceContext::new();
+ context
+ .properties
+ .insert("Key File",
test_utils::get_test_key_path(self.key_filename));
+
+ if let Some(passphrase) = self.passphrase {
+ context.properties.insert("Key Passphrase", passphrase);
+ }
+
+ PGPPrivateKeyService::enable(&context,
&MockLogger::new()).expect("should enable")
+ }
+ }
+
+ fn test_decryption(
+ message_file_name: &str,
+ private_key_data: Option<PrivateKeyData>,
+ symmetric_password: Option<&'static str>,
+ expected_result: Result<&[u8], ()>,
+ ) {
+ let mut processor_context = MockProcessContext::new();
+ if let Some(private_key) = private_key_data {
+ processor_context.controller_services.insert(
+ "my_private_key_service".to_string(),
+ Box::new(private_key.into_controller()),
+ );
+ processor_context.properties.insert(
+ PRIVATE_KEY_SERVICE.name(),
+ "my_private_key_service".to_string(),
+ );
+ }
+ if let Some(symmetric_password) = symmetric_password {
+ processor_context
+ .properties
+ .insert(SYMMETRIC_PASSWORD.name(),
symmetric_password.to_string());
+ }
+
+ let decrypt_content = DecryptContentPGP::schedule(&processor_context,
&MockLogger::new())
+ .expect("Should schedule with the configured properties");
+ let mut output: Vec<u8> = Vec::new();
+ let mut flow_file_stream =
std::io::Cursor::new(get_test_message(message_file_name));
+ let res = decrypt_content.transform(
+ &processor_context,
+ &mut flow_file_stream,
+ &mut output,
+ &MockLogger::new(),
+ );
+
+ match expected_result {
+ Ok(_result_bytes) => {
+ let res = res.expect("Should be able to transform");
+ assert_eq!(res.target_relationship_name(), SUCCESS.name);
+ assert_eq!(res.write_status(), IoState::Ok);
+ let data_modified = res
+ .get_attribute(LITERAL_DATA_MODIFIED.name)
+ .unwrap()
+ .parse::<u64>()
+ .expect("Should be u64");
+ assert!(data_modified > 1770000000000);
+ assert!(data_modified < 1780000000000);
+
assert!(res.get_attribute(LITERAL_DATA_FILENAME.name).is_some());
+ }
Review Comment:
The successful-decryption cases never compare `output` with the supplied
expected bytes, so they pass even when decryption emits corrupted or packaged
bytes instead of `foo\n`. Assert the output here; this would also exercise the
actual distinction between decrypting and merely routing successfully.
##########
minifi_rust/extensions/minifi_pgp/src/controller_services/public_key_service/controller_service_definition.rs:
##########
@@ -0,0 +1,26 @@
+use super::PGPPublicKeyService;
+use crate::controller_services::key_file_property::PublicKeyFile;
+use crate::controller_services::key_property::PublicKey;
+use minifi_native::{
+ ControllerServiceDefinition, Property, PropertyDefinition,
ProvidedInterface,
+ property_definitions,
+};
+
+pub(crate) const KEYRING_FILE: Property<Option<PublicKeyFile>> = Property::new(
+ "Keyring File",
+ "File path to PGP Keyring or Secret Key encoded in binary or ASCII Armor",
Review Comment:
The description says secret keys are accepted, but `PublicKeyFile::parse`
only parses `SignedPublicKey` and the added tests explicitly reject
`alice_private.*`. Describe this as a public key/keyring, or add actual
secret-key extraction support.
##########
minifi_rust/extensions/minifi_pgp/src/controller_services/key_file_property.rs:
##########
@@ -0,0 +1,124 @@
+use minifi_native::{MinifiError, PropertyConstraints, PropertySchema,
PropertyType};
+use pgp::composed::{Deserializable, SignedPublicKey, SignedSecretKey};
+
+pub(crate) struct SecretKeyFile {}
+
+impl PropertySchema for SecretKeyFile {
+ const CONSTRAINT: Option<PropertyConstraints> = None;
+ const IS_REQUIRED: bool = false;
+}
+
+impl PropertyType for SecretKeyFile {
+ type Output = Vec<SignedSecretKey>;
+
+ fn parse(s: &str) -> Result<Self::Output, MinifiError> {
+ let mut result: Vec<SignedSecretKey> = Vec::new();
+ if let Ok((keys, _headers)) = SignedSecretKey::from_armor_file_many(s)
{
+ result.extend(keys.filter_map(Result::ok));
+ } else if let Ok(keys) = SignedSecretKey::from_file_many(s) {
+ result.extend(keys.filter_map(Result::ok));
+ }
+ if result.is_empty() {
+ Err(MinifiError::validation(
+ "Couldnt load any valid secret keys",
Review Comment:
Correct the contraction in this user-facing validation message.
This issue also appears on line 49 of the same file.
##########
minifi_rust/extensions/minifi_pgp/src/controller_services/key_property.rs:
##########
@@ -0,0 +1,49 @@
+use minifi_native::{MinifiError, PropertyConstraints, PropertySchema,
PropertyType};
+use pgp::composed::{Deserializable, SignedPublicKey, SignedSecretKey};
+
+pub(crate) struct SecretKey {}
+
+impl PropertySchema for SecretKey {
+ const CONSTRAINT: Option<PropertyConstraints> = None;
+ const IS_REQUIRED: bool = false;
+}
+
+impl PropertyType for SecretKey {
+ type Output = Vec<SignedSecretKey>;
+
+ fn parse(s: &str) -> Result<Self::Output, MinifiError> {
+ let mut secret_keys: Vec<SignedSecretKey> = Vec::new();
+ if let Ok((keys, _headers)) =
SignedSecretKey::from_armor_many(s.as_bytes()) {
+ secret_keys.extend(keys.filter_map(Result::ok));
+ }
+ if secret_keys.is_empty() {
+ return Err(MinifiError::validation(
+ "Couldnt load any valid secrey keys",
Review Comment:
Correct the user-facing validation message spelling.
This issue also appears on line 44 of the same file.
##########
minifi_rust/extensions/minifi_pgp/src/controller_services/public_key_service/controller_service_definition.rs:
##########
@@ -0,0 +1,26 @@
+use super::PGPPublicKeyService;
+use crate::controller_services::key_file_property::PublicKeyFile;
+use crate::controller_services::key_property::PublicKey;
+use minifi_native::{
+ ControllerServiceDefinition, Property, PropertyDefinition,
ProvidedInterface,
+ property_definitions,
+};
+
+pub(crate) const KEYRING_FILE: Property<Option<PublicKeyFile>> = Property::new(
+ "Keyring File",
+ "File path to PGP Keyring or Secret Key encoded in binary or ASCII Armor",
+)
+.supports_expression_language();
+
+pub(crate) const KEYRING: Property<Option<PublicKey>> = Property::new(
+ "Keyring",
+ "PGP Keyring or Secret Key encoded in ASCII Armor",
Review Comment:
The inline property is documented as accepting a secret key, while
`PublicKey::parse` accepts only armored `SignedPublicKey` values and the tests
reject private-key input. Correct the advertised input type or implement the
documented support.
##########
minifi_rust/extensions/minifi_pgp/src/lib.rs:
##########
@@ -0,0 +1,24 @@
+mod controller_services;
Review Comment:
The new `minifi_pgp` Rust source files omit the ASF license header used by
every existing Rust module in this extension area (for example,
`minifi_rust/extensions/minifi_rs_playground/src/lib.rs:1-16`). Add the
standard header to all newly added `.rs` files.
--
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]
