pairing: Add validity check to PairingData

This adds a method to the PairingService that allows to check the validity of the PairingData. Also Refactors the initilization of the pairing data in the state machine. ZSH_THEME="peter Signed-off-by: Peter Neuroth <[email protected]>
Blockstream · Nov 10, 2023 · 5c485cd · 5c485cd
1 parent 0026267
commit 5c485cd
Show file tree

Hide file tree

Showing 5 changed files with 130 additions and 30 deletions.
diff --git a/libs/gl-client-py/glclient/__init__.py b/libs/gl-client-py/glclient/__init__.py
@@ -120,6 +120,13 @@ def get_session_data(self, session_id: int) -> schedpb.PairingSessionDataRespons
     def approve_session(self, session_id: int, timestamp: int, node_id: bytes, device_name: str, device_pubkey: bytes, restrictions: str):
         self.inner.approve_session(session_id, timestamp, node_id, device_name, device_pubkey, restrictions)
 
+    def verify_pairing_data(self, data: schedpb.PairingData) -> Optional[str]:
+        try:
+            self.inner.verify_pairing_data(data.SerializeToString())
+            return None
+        except Exception as e:
+            return str(e)
+
 
 class Node(object):
     def __init__(self, node_id: bytes, network: str, tls: TlsConfig, grpc_uri: str, rune: str) -> None:

diff --git a/libs/gl-client-py/glclient/glclient.pyi b/libs/gl-client-py/glclient/glclient.pyi
@@ -33,6 +33,7 @@ class PairingService:
     def new_session(self, name: str, desc: str, restrs: str): ...
     def get_session_data(self, session_id: int) -> bytes: ...
     def approve_session(self, session_id: int, timestamp: int, node_id: bytes, device_name: str, device_pubkey: bytes, restrictions: str) -> bytes: ...
+    def verify_pairing_data(self, data: bytes): ...
 
 class Node:
     def __init__(

diff --git a/libs/gl-client-py/src/pairing.rs b/libs/gl-client-py/src/pairing.rs
@@ -2,10 +2,13 @@ use crate::pb::scheduler::PairingSessionResponse;
 use crate::runtime::exec;
 use crate::scheduler::convert;
 use crate::tls::TlsConfig;
-use anyhow::anyhow;
+use anyhow::{anyhow, Error};
 use gl_client::pairing::service::Pairing;
+use gl_client::pb::scheduler::PairingData;
+use prost::Message;
 use pyo3::exceptions::PyValueError;
 use pyo3::prelude::*;
+use std::convert::Into;
 use tokio::sync::mpsc;
 
 #[pyclass]
@@ -83,6 +86,18 @@ impl PairingService {
                 .await
         }))
     }
+
+    fn verify_pairing_data(&self, data: Vec<u8>) -> PyResult<()> {
+        let pd = PairingData::decode(&data[..]).map_err(|e| {
+            PyValueError::new_err(format!(
+                "could not decode data={:?} as PairingData: {}",
+                data, e,
+            ))
+        })?;
+        self.inner
+            .verify_pairing_data(pd)
+            .map_err(|e| PyValueError::new_err(format!("{}", e)))
+    }
 }
 
 /// A wrapper class to return an iterable from a mpsc channel.

diff --git a/libs/gl-client-py/tests/test_pairing.py b/libs/gl-client-py/tests/test_pairing.py
@@ -44,4 +44,23 @@ def test_pairing_session(scheduler, nobody_id, sclient, signer, tls):
     # gl-scheduler. We await the signed CSR, the rune and the auth 
     # blob.
     m = next(session_iter)
-    assert(m)
+    assert(m)
+
+
+def test_paring_data_validation(scheduler, nobody_id, sclient, signer, tls):
+    """A simple test to ensure that data validation works as intended.
+
+    If the data is valid, the public key belongs to the private key that was
+    used to sign the message.
+    """
+    ps = PairingService()
+    session = ps.new_session("","","")
+    session_iter = iter(session)
+    m = next(session_iter)
+    assert(ps.verify_pairing_data(m.pairing_new_session.pairing_data) is None)
+
+    # Change the public key and try again
+    pk = bytearray(m.pairing_new_session.pairing_data.public_key)
+    pk[-1] = 0x01 if pk[-1] == 0x00 else 0x00
+    m.pairing_new_session.pairing_data.public_key = bytes(pk)
+    assert(ps.verify_pairing_data(m.pairing_new_session.pairing_data))
diff --git a/libs/gl-client/src/pairing/service.rs b/libs/gl-client/src/pairing/service.rs
@@ -12,7 +12,9 @@ use bytes::BufMut;
 use futhark::Rune;
 use log::{debug, trace};
 use prost::Message;
-use ring::signature::KeyPair;
+use ring::signature::{
+    KeyPair, UnparsedPublicKey, ECDSA_P256_SHA256_FIXED, ECDSA_P256_SHA256_FIXED_SIGNING,
+};
 use ring::{
     rand,
     signature::{self, EcdsaKeyPair},
@@ -158,43 +160,27 @@ impl Pairing {
                             r
                         );
 
-                        // Create necessary key pair and representations.
+                        // Create necessary key pair.
                         key_pair_pem = Some(tls::generate_ecdsa_key_pair().serialize_pem());
-                        let pem = key_pair_pem.clone().unwrap();
-                        let mut crs = std::io::Cursor::new(pem.as_bytes());
-                        let key = pemfile::pkcs8_private_keys(&mut crs)
-                            .unwrap()
-                            .pop()
-                            .unwrap();
-                        let kp = EcdsaKeyPair::from_pkcs8(
-                            &signature::ECDSA_P256_SHA256_FIXED_SIGNING,
-                            key.as_ref(),
-                        )
-                        .unwrap();
 
-                        // Get data that needs a signature.
-                        let timestamp = chrono::Utc::now().timestamp_micros() as u64;
-                        let mut buf = vec![];
-                        buf.put_u64(r.session_id);
-                        buf.put_u64(timestamp);
+                        // Create Pairing data.
+                        let pd = PairingData {
+                            session_id: r.session_id,
+                            timestamp: chrono::Utc::now().timestamp_micros() as u64,
+                            public_key: Default::default(),
+                            signature: Default::default(),
+                        };
 
-                        // Sign data.
-                        let rng = rand::SystemRandom::new();
-                        let public_key = kp.public_key().as_ref().to_vec();
-                        let signature = kp.sign(&rng, &buf).unwrap().as_ref().to_vec();
+                        // Sign Paring data.
+                        let pd = pd.sign(key_pair_pem.as_ref().unwrap().as_bytes()).unwrap();
 
                         session_state = SessionState::Initialized;
                         response_tx
                             .send(PairingSessionResponse {
                                 response: Some(Response::PairingNewSession(
                                     PairingNewSessionResponse {
                                         session_id: r.session_id,
-                                        pairing_data: Some(PairingData {
-                                            session_id: r.session_id,
-                                            timestamp,
-                                            public_key,
-                                            signature,
-                                        }),
+                                        pairing_data: Some(pd),
                                     },
                                 )),
                             })
@@ -343,6 +329,10 @@ impl Pairing {
             .await?
             .into_inner())
     }
+
+    pub fn verify_pairing_data(&self, data: PairingData) -> Result<()> {
+        data.verify()
+    }
 }
 
 impl TryInto<Vec<u8>> for PairingData {
@@ -377,6 +367,47 @@ pub fn deserialize_qr_data(data: &[u8]) -> Result<PairingData> {
     })
 }
 
+impl PairingData {
+    fn get_sig_data(&self) -> Vec<u8> {
+        let mut buf = vec![];
+        buf.put_u64(self.session_id);
+        buf.put_u64(self.timestamp);
+        buf
+    }
+
+    fn sign(&self, pem: &[u8]) -> Result<Self> {
+        let mut crs = std::io::Cursor::new(pem);
+        let key = pemfile::pkcs8_private_keys(&mut crs)?
+            .pop()
+            .ok_or(anyhow!("could not extract key from pkcs8"))?;
+        let kp = EcdsaKeyPair::from_pkcs8(&ECDSA_P256_SHA256_FIXED_SIGNING, key.as_ref())
+            .map_err(|e| anyhow!("could not create keypair from pkcs8: {}", e))?;
+
+        // Sign data.
+        let rng = rand::SystemRandom::new();
+        let public_key = kp.public_key().as_ref().to_vec();
+        let signature = kp
+            .sign(&rng, &self.get_sig_data())
+            .unwrap()
+            .as_ref()
+            .to_vec();
+
+        Ok(Self {
+            session_id: self.session_id,
+            timestamp: self.timestamp,
+            public_key,
+            signature,
+        })
+    }
+
+    fn verify(&self) -> Result<()> {
+        UnparsedPublicKey::new(&ECDSA_P256_SHA256_FIXED, &self.public_key)
+            .verify(&self.get_sig_data(), &self.signature)
+            .map_err(|e| anyhow!("qr_data not verified: {:?}", e))?;
+        Ok(())
+    }
+}
+
 #[derive(Debug)]
 enum SessionState {
     Requested,
@@ -401,4 +432,31 @@ pub mod tests {
         let npd = deserialize_qr_data(&data);
         assert_eq!(pd, npd.unwrap());
     }
+
+    #[test]
+    fn sign_and_verify_pairing_data() {
+        let pem = tls::generate_ecdsa_key_pair().serialize_pem();
+        let d1 = PairingData {
+            session_id: 158,
+            timestamp: chrono::Utc::now().timestamp_micros() as u64,
+            public_key: Default::default(),
+            signature: Default::default(),
+        };
+
+        let signed_d1 = d1.sign(pem.as_bytes()).unwrap();
+        assert!(signed_d1.verify().is_ok());
+
+        // Check wrong public key, change last byte, should return an
+        // error.
+        let mut signed_d2 = signed_d1.clone();
+        if let Some(lb) = signed_d2.public_key.last_mut() {
+            *lb = if *lb == 0 { 1 } else { 0 };
+        }
+        assert!(signed_d2.verify().is_err());
+
+        // Manipulate the data, this should return an error.
+        let mut signed_d3 = signed_d1.clone();
+        signed_d3.session_id = 1581;
+        assert!(signed_d3.verify().is_err());
+    }
 }