spetz commented on code in PR #4615: URL: https://github.com/apache/iggy/pull/4615#discussion_r4340265820
########## foreign/php/src/sensitive_parameters.rs: ########## @@ -0,0 +1,134 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +use std::{ + ffi::{CStr, c_char, c_void}, + mem, + sync::{Mutex, PoisonError}, +}; + +use ext_php_rs::{ + ffi::{HashTable, zend_function, zend_hash_str_find_ptr_lc, zend_string}, + types::ZendStr, + zend::ClassEntry, +}; + +type AddAttribute = unsafe extern "C" fn( + attributes: *mut *mut HashTable, + name: *mut zend_string, + argc: u32, + flags: u32, + offset: u32, + lineno: u32, +) -> *mut c_void; +type GetParameterAttribute = unsafe extern "C" fn( + attributes: *mut HashTable, + name: *const c_char, + length: usize, + offset: u32, +) -> *mut c_void; + +const SENSITIVE_PARAMETER: &str = "SensitiveParameter"; +const SENSITIVE_PARAMETER_LOWERCASE: &str = "sensitiveparameter"; +const ZEND_ATTRIBUTE_PERSISTENT: u32 = 1; +const SUCCESS: i32 = 0; + +/// Class, method and zero-based position of every parameter that carries a secret. +// Every `raw` array is included, since this build cannot tell which unknown keys are secret. +const SENSITIVE_PARAMETERS: &[(&str, &str, u32)] = &[ Review Comment: Added `("Iggy\\Client", "sendBinaryRequest", 1)` at `foreign/php/src/sensitive_parameters.rs:55` and the `raw binary payload` row in `foreign/php/tests/SensitiveParameterTest.php:36`, which fails without the table entry. ########## core/common/src/http/options.rs: ########## @@ -0,0 +1,676 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +use crate::IggyError; +use crate::types::message::{HeaderKey, HeaderKind, HeaderValue}; +use crate::types::options::{ + OptionKey, OptionValue, OptionsPatch, OptionsProvenance, ResourceOptions, erase_options, + key_preview, wire_reset_names, +}; +use crate::wire_conversions::resource_options_to_wire; +use base64::Engine; +use base64::engine::general_purpose::STANDARD; +use iggy_binary_protocol::WireOptions; +use secrecy::zeroize::Zeroize; +use serde::de::{Error as _, MapAccess, Visitor}; +use serde::ser::{Error as _, SerializeMap}; +use serde::{Deserialize, Deserializer, Serialize, Serializer}; +use std::collections::{BTreeMap, BTreeSet}; +use std::fmt::{Debug, Formatter}; +use std::str::FromStr; + +/// The update body member that lists the keys to reset. No option is ever +/// named `reset`. +pub const RESET_FIELD: &str = "reset"; + +/// The largest integer a JSON number carries exactly in every client, 2^53 - 1. +const MAX_EXACT_JSON_INTEGER: u64 = (1 << 53) - 1; + +/// One option value in an HTTP body: a JSON boolean, a JSON integer, or a +/// string in the same form a config file uses (`"7 days"`, `"1 GiB"`). A +/// 64-bit value above 2^53 travels as a decimal string so JavaScript clients +/// keep its precision. +#[derive(Clone, PartialEq, Eq, Serialize, Deserialize)] +#[serde(untagged)] +pub enum OptionInputValue { + Bool(bool), + U64(u64), + String(String), +} + +/// Option values for a create body, keyed by option name. A name repeated in +/// the JSON object is refused rather than silently collapsed. +#[derive(Clone, Default, PartialEq, Eq)] +pub struct OptionInputMap(pub BTreeMap<String, OptionInputValue>); + +/// An HTTP update body: one flat object of option keys plus an optional +/// `reset` array. A value sets the key, an absent key or `null` changes +/// nothing, a key named in `reset` returns to its default. `null` never resets, +/// because many JSON serializers write unset optional fields as `null`. A body +/// that changes nothing, with no keys, only nulls or only zero sentinels such +/// as `"message_expiry": 0`, is refused with `InvalidCommand` on every +/// transport. +#[derive(Clone, Default, PartialEq, Eq)] +pub struct OptionsUpdate { + pub set: BTreeMap<String, OptionInputValue>, + pub reset: BTreeSet<String>, +} + +struct UniqueKeysVisitor; + +struct OptionsUpdateVisitor; + +impl OptionInputMap { + /// Encode the values into a create options block, each in the kind its + /// catalog entry expects where the JSON value fits it. Admission validates + /// the keys and values, so a mismatch fails in the same category as on + /// the binary transports. The map built on the way may hold the stream + /// key, so it is erased once encoded. + /// + /// # Errors + /// + /// `UnsupportedOptionKey` for a key that is not a valid option name. + pub fn to_wire(&self) -> Result<WireOptions, IggyError> { + let options = input_options(&self.0)?; + let wire = resource_options_to_wire(&options, OptionsProvenance::All); + erase_options(options); + wire + } + + #[must_use] + pub fn is_empty(&self) -> bool { + self.0.is_empty() + } +} + +impl OptionsUpdate { + /// Split the body into the keys to set and the keys to reset, in wire + /// form. Admission validates the keys and values. + /// + /// # Errors + /// + /// As [`OptionInputMap::to_wire`], and `UnsupportedOptionKey` for a reset + /// name that is not a valid option name or a reset list longer than the + /// wire carries. + pub fn to_patch(&self) -> Result<OptionsPatch, IggyError> { + let set = input_options(&self.set)?; + let patch = wire_reset_names(self.reset.iter().map(String::as_str)).and_then(|reset| { + Ok(OptionsPatch { + set: resource_options_to_wire(&set, OptionsProvenance::All)?, + reset, + }) + }); + erase_options(set); + patch + } +} + +impl OptionInputValue { + /// Zero text in place, since it may be the base64 stream key. + fn erase(&mut self) { + if let Self::String(text) = self { + text.as_mut_str().zeroize(); + } + } +} + +impl Drop for OptionInputMap { Review Comment: The containers are plain data again. `OptionInputValue::String` holds `Zeroizing<String>` with hand-written serde (`core/common/src/http/options.rs:52`), so moved or destructured values still erase on drop, and the test uses `..update` again. ########## core/server/src/http/extractor.rs: ########## @@ -101,6 +131,87 @@ impl FromRequestParts<HttpState> for Authenticated { } } +impl<T, S> FromRequest<S> for JsonMembers<T> +where + T: DeserializeOwned, + S: Send + Sync, +{ + type Rejection = Response; + + /// Runs axum's own `Json` extraction, so content type checks, body limits + /// and rejections stay what every other route answers. + async fn from_request(request: Request, state: &S) -> Result<Self, Self::Rejection> { + let (parts, body) = request.into_parts(); + let bytes = Bytes::from_request(Request::from_parts(parts.clone(), body), state) Review Comment: The extractor bodies and the follower relay body now go through `erase_buffer` once parsed or relayed (`core/server/src/http/extractor.rs:153`, `core/server/src/http/forward.rs:660`). A body still shared with the connection buffer and the SDK request bytes owned by the transport are documented as not erased. -- 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]
