Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 51caea58 authored by Rahul Arya's avatar Rahul Arya
Browse files

[Private GATT] Refactor AttOpcode handling

Make arbitration logic clearer by explicitly handling Opcode types
as per the spec.

Bug: 255880936
Test: unit
Change-Id: I494e274e393bda1a929e6e2ed8528faa52d3335d
parent 1462b1a7
Loading
Loading
Loading
Loading
+1 −0
Original line number Diff line number Diff line
@@ -7,6 +7,7 @@ pub mod channel;
pub mod ffi;
pub mod ids;
pub mod mocks;
pub mod opcode_types;
pub mod server;

pub use self::callbacks::GattCallbacks;
+23 −11
Original line number Diff line number Diff line
@@ -7,12 +7,14 @@ use log::{error, info, trace};

use crate::{
    do_in_rust_thread,
    packets::{AttOpcode, OwnedAttView, OwnedPacket},
    gatt::server::att_server_bearer::AttServerBearer,
    packets::{OwnedAttView, OwnedPacket},
};

use super::{
    ffi::{InterceptAction, StoreCallbacksFromRust},
    ids::{AdvertiserId, ConnectionId, ServerId, TransportIndex},
    opcode_types::{classify_opcode, OperationType},
};

static ARBITER: Mutex<Option<Arbiter>> = Mutex::new(None);
@@ -91,15 +93,10 @@ impl Arbiter {

        let att = OwnedAttView::try_parse(packet).ok()?;

        match att.view().get_opcode() {
            AttOpcode::FIND_INFORMATION_REQUEST
            | AttOpcode::FIND_BY_TYPE_VALUE_REQUEST
            | AttOpcode::READ_BY_TYPE_REQUEST
            | AttOpcode::READ_REQUEST
            | AttOpcode::READ_BLOB_REQUEST
            | AttOpcode::READ_MULTIPLE_REQUEST
            | AttOpcode::READ_BY_GROUP_TYPE_REQUEST
            | AttOpcode::WRITE_REQUEST => Some((att, conn_id)),
        match classify_opcode(att.view().get_opcode()) {
            OperationType::Command | OperationType::Request | OperationType::Confirmation => {
                Some((att, conn_id))
            }
            _ => None,
        }
    }
@@ -177,7 +174,7 @@ mod test {

    use crate::{
        gatt::ids::AttHandle,
        packets::{AttBuilder, AttReadRequestBuilder, Serializable},
        packets::{AttBuilder, AttOpcode, AttReadRequestBuilder, Serializable},
    };

    const TCB_IDX: TransportIndex = TransportIndex(1);
@@ -317,6 +314,21 @@ mod test {
        assert!(matches!(out, Some((_, CONN_ID))));
    }

    #[test]
    fn test_packet_bypass_when_isolated() {
        let mut arbiter = Arbiter::new();
        arbiter.associate_server_with_advertiser(SERVER_ID, ADVERTISER_ID);
        arbiter.on_le_connect(TCB_IDX, ADVERTISER_ID);
        let packet = AttBuilder {
            opcode: AttOpcode::ERROR_RESPONSE,
            _child_: AttReadRequestBuilder { attribute_handle: AttHandle(1).into() }.into(),
        };

        let out = arbiter.try_parse_att_server_packet(TCB_IDX, packet.to_vec().unwrap().into());

        assert!(out.is_none());
    }

    #[test]
    fn test_packet_bypass_when_not_isolated() {
        let mut arbiter = Arbiter::new();
+62 −0
Original line number Diff line number Diff line
//! This module lets us classify AttOpcodes to determine how to handle them

use crate::packets::AttOpcode;

/// The type of ATT operation performed by the packet
/// (see Core Spec 5.3 Vol 3F 3.3 Attribute PDU for details)
pub enum OperationType {
    /// Client -> server, no response expected
    Command,
    /// Client -> server, response expected
    Request,
    /// Server -> client, response to a request
    Response,
    /// Server -> client, no response expected
    Notification,
    /// Server -> client, response expected
    Indication,
    /// Client -> server, response to an indication
    Confirmation,
}

/// Classify an opcode by its operation type. Note that this could be done using
/// bitmasking, but is done explicitly for clarity.
pub fn classify_opcode(opcode: AttOpcode) -> OperationType {
    match opcode {
        AttOpcode::ERROR_RESPONSE => OperationType::Response,
        AttOpcode::EXCHANGE_MTU_RESPONSE => OperationType::Response,
        AttOpcode::FIND_INFORMATION_RESPONSE => OperationType::Response,
        AttOpcode::FIND_BY_TYPE_VALUE_RESPONSE => OperationType::Response,
        AttOpcode::READ_BY_TYPE_RESPONSE => OperationType::Response,
        AttOpcode::READ_RESPONSE => OperationType::Response,
        AttOpcode::READ_BLOB_RESPONSE => OperationType::Response,
        AttOpcode::READ_MULTIPLE_RESPONSE => OperationType::Response,
        AttOpcode::READ_BY_GROUP_TYPE_RESPONSE => OperationType::Response,
        AttOpcode::WRITE_RESPONSE => OperationType::Response,
        AttOpcode::PREPARE_WRITE_RESPONSE => OperationType::Response,
        AttOpcode::EXECUTE_WRITE_RESPONSE => OperationType::Response,
        AttOpcode::READ_MULTIPLE_VARIABLE_RESPONSE => OperationType::Response,

        AttOpcode::EXCHANGE_MTU_REQUEST => OperationType::Request,
        AttOpcode::FIND_INFORMATION_REQUEST => OperationType::Request,
        AttOpcode::FIND_BY_TYPE_VALUE_REQUEST => OperationType::Request,
        AttOpcode::READ_BY_TYPE_REQUEST => OperationType::Request,
        AttOpcode::READ_REQUEST => OperationType::Request,
        AttOpcode::READ_BLOB_REQUEST => OperationType::Request,
        AttOpcode::READ_MULTIPLE_REQUEST => OperationType::Request,
        AttOpcode::READ_BY_GROUP_TYPE_REQUEST => OperationType::Request,
        AttOpcode::WRITE_REQUEST => OperationType::Request,
        AttOpcode::PREPARE_WRITE_REQUEST => OperationType::Request,
        AttOpcode::EXECUTE_WRITE_REQUEST => OperationType::Request,
        AttOpcode::READ_MULTIPLE_VARIABLE_REQUEST => OperationType::Request,

        AttOpcode::WRITE_COMMAND => OperationType::Command,
        AttOpcode::SIGNED_WRITE_COMMAND => OperationType::Command,

        AttOpcode::HANDLE_VALUE_NOTIFICATION => OperationType::Notification,

        AttOpcode::HANDLE_VALUE_INDICATION => OperationType::Indication,

        AttOpcode::HANDLE_VALUE_CONFIRMATION => OperationType::Confirmation,
    }
}
+21 −1
Original line number Diff line number Diff line
@@ -9,7 +9,10 @@ use tokio::task::spawn_local;

use crate::{
    core::shared_box::WeakBoxRef,
    gatt::ids::AttHandle,
    gatt::{
        ids::AttHandle,
        opcode_types::{classify_opcode, OperationType},
    },
    packets::{
        AttBuilder, AttChild, AttErrorCode, AttErrorResponseBuilder, AttView, Packet,
        SerializeError,
@@ -61,6 +64,23 @@ impl<T: AttDatabase + 'static> WeakBoxRef<'_, AttServerBearer<T>> {
    /// Handle an incoming packet, and send outgoing packets as appropriate
    /// using the owned ATT channel.
    pub fn handle_packet(&self, packet: AttView<'_>) {
        match classify_opcode(packet.get_opcode()) {
            OperationType::Command => {
                error!("dropping ATT command (currently unsupported)");
            }
            OperationType::Request => {
                Self::handle_request(self, packet);
            }
            OperationType::Confirmation => {
                error!("ignoring handle value confirmation (currently unsupported)");
            }
            OperationType::Response | OperationType::Notification | OperationType::Indication => {
                unreachable!("the arbiter should not let us receive these packet types")
            }
        }
    }

    fn handle_request(&self, packet: AttView<'_>) {
        let curr_request = self.curr_request.replace(AttRequestState::Pending(None));
        self.curr_request.replace(match curr_request {
            AttRequestState::Idle(mut request_handler) => {
+17 −0
Original line number Diff line number Diff line
@@ -29,6 +29,23 @@ enum AttOpcode : 8 {

  WRITE_REQUEST = 0x12,
  WRITE_RESPONSE = 0x13,

  WRITE_COMMAND = 0x52,

  PREPARE_WRITE_REQUEST = 0x16,
  PREPARE_WRITE_RESPONSE = 0x17,
  EXECUTE_WRITE_REQUEST = 0x18,
  EXECUTE_WRITE_RESPONSE = 0x19,

  READ_MULTIPLE_VARIABLE_REQUEST = 0x20,
  READ_MULTIPLE_VARIABLE_RESPONSE = 0x21,

  HANDLE_VALUE_NOTIFICATION = 0x1B,

  HANDLE_VALUE_INDICATION = 0x1D,
  HANDLE_VALUE_CONFIRMATION = 0x1E,

  SIGNED_WRITE_COMMAND = 0xD2,
}

packet Att {