0
0
mirror of https://github.com/signalapp/libsignal.git synced 2024-09-20 03:52:17 +02:00

Fix Java error handling for CDSI lookup

CDSI error handling code would attempt to instantiate a nonexistent Java class. 
Add the missing class and split up the handling for CDSI lookup errors to reuse 
existing error types.
This commit is contained in:
Alex Konradi 2024-02-09 15:31:35 -05:00 committed by GitHub
parent 45d513a548
commit 100ce19945
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
13 changed files with 120 additions and 35 deletions

View File

@ -0,0 +1,13 @@
//
// Copyright 2024 Signal Messenger, LLC.
// SPDX-License-Identifier: AGPL-3.0-only
//
package org.signal.libsignal.net;
/** Error thrown by a failed CDSI lookup operation. */
public class CdsiLookupException extends Exception {
public CdsiLookupException(String message) {
super(message);
}
}

View File

@ -0,0 +1,13 @@
//
// Copyright 2024 Signal Messenger, LLC.
// SPDX-License-Identifier: AGPL-3.0-only
//
package org.signal.libsignal.net;
/** Error thrown by a network failure. */
public class NetworkException extends Exception {
public NetworkException(String message) {
super(message);
}
}

View File

@ -38,4 +38,9 @@ public class CdsiLookupResponseTest {
assertEquals(expected, actual);
}
@Test
public void cdsiLookupErrorConvert() {
assertThrows(java.lang.Exception.class, () -> Native.TESTING_CdsiLookupErrorConvert());
}
}

View File

@ -577,6 +577,7 @@ public final class Native {
public static native long Svr2Client_New(byte[] mrenclave, byte[] attestationMsg, long currentTimestamp);
public static native void TESTING_CdsiLookupErrorConvert();
public static native Map TESTING_CdsiLookupResponseConvert();
public static native void TESTING_ErrorOnBorrowAsync(Object input);
public static native CompletableFuture TESTING_ErrorOnBorrowIo(long asyncRuntime, Object input);

1
node/Native.d.ts vendored
View File

@ -428,6 +428,7 @@ export function SignedPreKeyRecord_GetSignature(obj: Wrapper<SignedPreKeyRecord>
export function SignedPreKeyRecord_GetTimestamp(obj: Wrapper<SignedPreKeyRecord>): Timestamp;
export function SignedPreKeyRecord_New(id: number, timestamp: Timestamp, pubKey: Wrapper<PublicKey>, privKey: Wrapper<PrivateKey>, signature: Buffer): SignedPreKeyRecord;
export function SignedPreKeyRecord_Serialize(obj: Wrapper<SignedPreKeyRecord>): Buffer;
export function TESTING_CdsiLookupErrorConvert(): void;
export function TESTING_CdsiLookupResponseConvert(): LookupResponse;
export function TESTING_ErrorOnBorrowAsync(_input: null): Promise<void>;
export function TESTING_ErrorOnBorrowIo(asyncRuntime: Wrapper<NonSuspendingBackgroundThreadRuntime>, _input: null): Promise<void>;

View File

@ -7,6 +7,7 @@ import { config, expect } from 'chai';
import * as util from './util';
import { Aci, Pni } from '../Address';
import * as Native from '../../Native';
import { ErrorCode, LibSignalErrorBase } from '../Errors';
util.initLogger();
config.truncateThreshold = 0;
@ -37,5 +38,11 @@ describe('cdsi lookup', () => {
expect(Native.TESTING_CdsiLookupResponseConvert()).deep.equals(expected);
});
it('converts errors to native', () => {
expect(() => Native.TESTING_CdsiLookupErrorConvert())
.throws(LibSignalErrorBase)
.with.property('code', ErrorCode.IoError);
});
});
});

View File

@ -2,14 +2,16 @@
// Copyright 2020-2021 Signal Messenger, LLC.
// SPDX-License-Identifier: AGPL-3.0-only
//
use jni::objects::{GlobalRef, JObject, JString, JThrowable};
use jni::{JNIEnv, JavaVM};
use std::fmt;
use std::io::{Error as IoError, ErrorKind as IoErrorKind};
use jni::objects::{GlobalRef, JObject, JString, JThrowable};
use jni::{JNIEnv, JavaVM};
use libsignal_net::infra::errors::NetError;
use attest::hsm_enclave::Error as HsmEnclaveError;
use device_transfer::Error as DeviceTransferError;
use libsignal_net::cdsi::CdsiError;
use libsignal_protocol::*;
use signal_crypto::Error as SignalCryptoError;
use signal_pin::Error as PinError;
@ -39,7 +41,8 @@ pub enum SignalJniError {
Mp4SanitizeParse(signal_media::sanitize::mp4::ParseErrorReport),
#[cfg(feature = "signal-media")]
WebpSanitizeParse(signal_media::sanitize::webp::ParseErrorReport),
Cdsi(libsignal_net::cdsi::Error),
Cdsi(CdsiError),
Net(NetError),
Bridge(BridgeLayerError),
}
@ -80,6 +83,7 @@ impl fmt::Display for SignalJniError {
#[cfg(feature = "signal-media")]
SignalJniError::WebpSanitizeParse(e) => write!(f, "{}", e),
SignalJniError::Cdsi(e) => write!(f, "{}", e),
SignalJniError::Net(e) => write!(f, "{}", e),
SignalJniError::Bridge(e) => write!(f, "{}", e),
}
}
@ -212,9 +216,23 @@ impl From<signal_media::sanitize::webp::Error> for SignalJniError {
}
}
impl From<libsignal_net::cdsi::Error> for SignalJniError {
fn from(e: libsignal_net::cdsi::Error) -> SignalJniError {
SignalJniError::Cdsi(e)
impl From<libsignal_net::cdsi::LookupError> for SignalJniError {
fn from(e: libsignal_net::cdsi::LookupError) -> SignalJniError {
use libsignal_net::cdsi::LookupError;
SignalJniError::Cdsi(match e {
LookupError::AttestationError(e) => return e.into(),
LookupError::Net(e) => return e.into(),
LookupError::InvalidResponse => CdsiError::InvalidResponse,
LookupError::Protocol => CdsiError::Protocol,
LookupError::RateLimited { retry_after } => CdsiError::RateLimited { retry_after },
LookupError::ParseError => CdsiError::ParseError,
})
}
}
impl From<NetError> for SignalJniError {
fn from(e: NetError) -> SignalJniError {
Self::Net(e)
}
}

View File

@ -530,7 +530,8 @@ where
jni_class_name!(org.signal.libsignal.media.ParseException)
}
SignalJniError::Cdsi(_) => jni_class_name!(org.signal.libsignal.cds2.CdsiLookupException),
SignalJniError::Cdsi(_) => jni_class_name!(org.signal.libsignal.net.CdsiLookupException),
SignalJniError::Net(_) => jni_class_name!(org.signal.libsignal.net.NetworkException),
};
let throwable = env

View File

@ -167,14 +167,14 @@ async fn CdsiLookup_new(
password: String,
request: &LookupRequest,
timeout_millis: u32,
) -> Result<CdsiLookup, cdsi::Error> {
) -> Result<CdsiLookup, cdsi::LookupError> {
let request = std::mem::take(&mut *request.0.lock().expect("not poisoned"));
let auth = Auth { username, password };
let connected = CdsiConnection::connect(&connection_manager.cdsi, auth).await?;
let (token, remaining_response) = timeout(
Duration::from_millis(timeout_millis.into()),
cdsi::Error::Net(NetError::Timeout),
cdsi::LookupError::Net(NetError::Timeout),
connected.send_request(request),
)
.await?;
@ -191,7 +191,7 @@ fn CdsiLookup_token(lookup: &CdsiLookup) -> &[u8] {
}
#[bridge_io(TokioAsyncContext, ffi = false)]
async fn CdsiLookup_complete(lookup: &CdsiLookup) -> Result<LookupResponse, cdsi::Error> {
async fn CdsiLookup_complete(lookup: &CdsiLookup) -> Result<LookupResponse, cdsi::LookupError> {
let CdsiLookup {
token: _,
remaining,

View File

@ -395,7 +395,7 @@ impl SignalNodeError for std::io::Error {
}
}
impl SignalNodeError for libsignal_net::cdsi::Error {
impl SignalNodeError for libsignal_net::cdsi::LookupError {
fn throw<'a>(
self,
cx: &mut impl Context<'a>,
@ -414,7 +414,7 @@ impl SignalNodeError for libsignal_net::cdsi::Error {
),
Self::Net(_)
| Self::Protocol
| Self::AttestationError
| Self::AttestationError(_)
| Self::InvalidResponse
| Self::ParseError => (IO_ERROR, None),
};

View File

@ -4,7 +4,7 @@
//
use libsignal_bridge_macros::*;
use libsignal_net::cdsi::{LookupResponse, LookupResponseEntry, E164};
use libsignal_net::cdsi::{LookupError, LookupResponse, LookupResponseEntry, E164};
use libsignal_protocol::{Aci, Pni};
use nonzero_ext::nonzero;
use uuid::Uuid;
@ -39,3 +39,8 @@ fn TESTING_CdsiLookupResponseConvert() -> LookupResponse {
debug_permits_used: DEBUG_PERMITS_USED,
}
}
#[bridge_fn(ffi = false)]
fn TESTING_CdsiLookupErrorConvert() -> Result<(), LookupError> {
Err(LookupError::ParseError)
}

View File

@ -10,7 +10,7 @@ use tokio::io::AsyncBufReadExt as _;
use libsignal_net::auth::Auth;
use libsignal_net::cdsi::{
CdsiConnection, CdsiConnectionParams, Error, LookupRequest, LookupResponse,
CdsiConnection, CdsiConnectionParams, LookupError, LookupRequest, LookupResponse,
};
use libsignal_net::enclave::EndpointConnection;
use libsignal_net::infra::errors::NetError;
@ -21,11 +21,11 @@ async fn cdsi_lookup(
cdsi: &impl CdsiConnectionParams,
request: LookupRequest,
timeout: Duration,
) -> Result<LookupResponse, Error> {
) -> Result<LookupResponse, LookupError> {
let connected = CdsiConnection::connect(cdsi, auth).await?;
let (_token, remaining_response) = libsignal_net::utils::timeout(
timeout,
Error::Net(NetError::Timeout),
LookupError::Net(NetError::Timeout),
connected.send_request(request),
)
.await?;

View File

@ -162,7 +162,7 @@ pub enum LookupResponseParseError {
InvalidNumberOfBytes { actual_length: usize },
}
impl From<LookupResponseParseError> for Error {
impl From<LookupResponseParseError> for LookupError {
fn from(value: LookupResponseParseError) -> Self {
match value {
LookupResponseParseError::InvalidNumberOfBytes { .. } => Self::ParseError,
@ -234,14 +234,15 @@ impl<S> AsMut<AttestedConnection<S>> for CdsiConnection<S> {
}
}
/// Anything that can go wrong during a CDSI lookup.
#[derive(Debug, Error, displaydoc::Display)]
pub enum Error {
pub enum LookupError {
/// Network error
Net(#[from] NetError),
/// Protocol error after establishing a connection.
Protocol,
/// SGX attestation failed.
AttestationError,
AttestationError(attest::enclave::Error),
/// Invalid response received from the server.
InvalidResponse,
/// Retry later.
@ -250,18 +251,33 @@ pub enum Error {
ParseError,
}
impl From<AttestedConnectionError> for Error {
/// CDSI-protocol-specific subset of [`LookupError`] cases.
///
/// Contains cases for errors that aren't covered by other error types.
#[derive(Debug, displaydoc::Display)]
pub enum CdsiError {
/// Protocol error after establishing a connection.
Protocol,
/// Invalid response received from the server.
InvalidResponse,
/// Retry later.
RateLimited { retry_after: Duration },
/// Failed to parse the response from the server.
ParseError,
}
impl From<AttestedConnectionError> for LookupError {
fn from(value: AttestedConnectionError) -> Self {
match value {
AttestedConnectionError::ClientConnection(_) => Self::Protocol,
AttestedConnectionError::Net(net) => Self::Net(net),
AttestedConnectionError::Protocol => Self::Protocol,
AttestedConnectionError::Sgx(_) => Self::AttestationError,
AttestedConnectionError::Sgx(e) => Self::AttestationError(e),
}
}
}
impl From<prost::DecodeError> for Error {
impl From<prost::DecodeError> for LookupError {
fn from(_value: prost::DecodeError) -> Self {
Self::Protocol
}
@ -281,7 +297,7 @@ pub struct ClientResponseCollector<S = SslStream<TcpStream>>(CdsiConnection<S>);
impl<S: AsyncDuplexStream> CdsiConnection<S> {
/// Connect to remote host and verify remote attestation.
pub async fn connect<P, T>(env: &P, auth: impl HttpBasicAuth) -> Result<Self, Error>
pub async fn connect<P, T>(env: &P, auth: impl HttpBasicAuth) -> Result<Self, LookupError>
where
P: CdsiConnectionParams<TransportConnector = T>,
T: TransportConnector<Stream = S>,
@ -293,9 +309,9 @@ impl<S: AsyncDuplexStream> CdsiConnection<S> {
let connection_attempt_result = service_initializer.connect().await;
let websocket = match connection_attempt_result {
ServiceState::Active(websocket, _) => Ok(websocket),
ServiceState::Cooldown(_) => Err(Error::Net(NetError::NoServiceConnection)),
ServiceState::Error(e) => Err(Error::Net(e)),
ServiceState::TimedOut => Err(Error::Net(NetError::Timeout)),
ServiceState::Cooldown(_) => Err(LookupError::Net(NetError::NoServiceConnection)),
ServiceState::Error(e) => Err(LookupError::Net(e)),
ServiceState::TimedOut => Err(LookupError::Net(NetError::Timeout)),
}?;
let attested = AttestedConnection::connect(websocket, |attestation_msg| {
attest::cds2::new_handshake(env.mr_enclave(), attestation_msg, SystemTime::now())
@ -308,7 +324,7 @@ impl<S: AsyncDuplexStream> CdsiConnection<S> {
pub async fn send_request(
mut self,
request: LookupRequest,
) -> Result<(Token, ClientResponseCollector<S>), Error> {
) -> Result<(Token, ClientResponseCollector<S>), LookupError> {
self.0.send(request.into_client_request()).await?;
let token_response: ClientResponse = match self.0.receive().await? {
NextOrClose::Next(response) => response,
@ -318,18 +334,18 @@ impl<S: AsyncDuplexStream> CdsiConnection<S> {
if let Ok(RateLimitExceededResponse { retry_after }) =
serde_json::from_str(&close.reason)
{
return Err(Error::RateLimited {
return Err(LookupError::RateLimited {
retry_after: Duration::from_secs(retry_after.into()),
});
}
}
};
return Err(Error::Protocol);
return Err(LookupError::Protocol);
}
};
if token_response.token.is_empty() {
return Err(Error::Protocol);
return Err(LookupError::Protocol);
}
Ok((
@ -340,7 +356,7 @@ impl<S: AsyncDuplexStream> CdsiConnection<S> {
}
impl<S: AsyncDuplexStream> ClientResponseCollector<S> {
pub async fn collect(self) -> Result<LookupResponse, Error> {
pub async fn collect(self) -> Result<LookupResponse, LookupError> {
let Self(mut connection) = self;
let token_ack = ClientRequest {
@ -349,10 +365,15 @@ impl<S: AsyncDuplexStream> ClientResponseCollector<S> {
};
connection.0.send(token_ack).await?;
let mut response: ClientResponse =
connection.0.receive().await?.next_or(Error::Protocol)?;
let mut response: ClientResponse = connection
.0
.receive()
.await?
.next_or(LookupError::Protocol)?;
while let NextOrClose::Next(decoded) = connection.0.receive_bytes().await? {
response.merge(decoded.as_ref()).map_err(Error::from)?;
response
.merge(decoded.as_ref())
.map_err(LookupError::from)?;
}
Ok(response.try_into()?)
}