Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle PCK certificates #1068

Merged
merged 44 commits into from
Oct 28, 2024
Merged
Show file tree
Hide file tree
Changes from 21 commits
Commits
Show all changes
44 commits
Select commit Hold shift + click to select a range
44501dc
Add x509-parser dependency and helper fns
ameba23 Sep 24, 2024
d27ceba
Add pck cert chain parser
ameba23 Sep 25, 2024
ca6e09c
Doccomments
ameba23 Sep 25, 2024
11575c4
Use x509 crate that works with no-std
ameba23 Sep 26, 2024
6834392
Tidy, comments
ameba23 Sep 26, 2024
a1ae9e4
Merge branch 'master' into peg/handle-pck-certs
ameba23 Oct 2, 2024
8ed311a
WIP - handle PCK certs in staking pallet
ameba23 Oct 2, 2024
721bc82
Rm pck cert stuff from attestation pallet
ameba23 Oct 2, 2024
94d563f
Fix mock pck cert chain verifying
ameba23 Oct 3, 2024
6420aa9
Taplo
ameba23 Oct 3, 2024
a170da5
Compress verifying key
ameba23 Oct 3, 2024
d0663b7
Add missing import
ameba23 Oct 3, 2024
eda8bf0
Add missing implementations of PckCertVerifyier
ameba23 Oct 3, 2024
e02b0f4
Fix staking pallet benchmarks
ameba23 Oct 3, 2024
aac90aa
Validate cert chain of arbitrary length
ameba23 Oct 3, 2024
f5941a2
Tidy, fix tests
ameba23 Oct 3, 2024
1d6da3a
Lockfile
ameba23 Oct 3, 2024
0c50bbb
Merge master
ameba23 Oct 3, 2024
55abbd3
Clippy
ameba23 Oct 3, 2024
7815a87
Error handling
ameba23 Oct 3, 2024
31eca99
Add test for production cert verifyer
ameba23 Oct 3, 2024
8c15035
Rm commented code in test
ameba23 Oct 3, 2024
198da10
Changelog
ameba23 Oct 3, 2024
eef45ce
Merge branch 'master' into peg/handle-pck-certs
ameba23 Oct 3, 2024
e8d7a78
update metadata
JesseAbram Oct 3, 2024
672a1bf
Merge master
ameba23 Oct 23, 2024
021aac3
Hopefully fix staking extension pallet benchmarking for validate extr…
ameba23 Oct 23, 2024
097c188
Update pallets/staking/src/lib.rs
ameba23 Oct 24, 2024
6f0acd6
Update pallets/staking/src/tests.rs
ameba23 Oct 24, 2024
689e2f0
Update pallets/staking/src/pck/production.rs
ameba23 Oct 24, 2024
4fc7cb2
Update pallets/staking/src/pck/production.rs
ameba23 Oct 24, 2024
a6c5cea
Typo in struct name and improve test readablilty
ameba23 Oct 24, 2024
841b167
Rm calls to mock_attest_validate in staking pallet test as fn is now …
ameba23 Oct 24, 2024
03c6b37
Handle errors in production verify_cert fn
ameba23 Oct 24, 2024
d99fc35
Doccomments
ameba23 Oct 24, 2024
8d57919
Alphabetically sort runtime config types
ameba23 Oct 24, 2024
beb4e7e
Changelog
ameba23 Oct 24, 2024
3968d6e
derive serialize and deserialize for JoiningServerInfo
ameba23 Oct 24, 2024
a053e3b
Rename struct (typo)
ameba23 Oct 24, 2024
d7feb00
Make it clearer what is happenning with the PCK generation in staking…
ameba23 Oct 25, 2024
27d64eb
Use intel root cert from web link and link to it in doccomments
ameba23 Oct 25, 2024
06d3028
Fix staking pallet benchmarks in always generate a mock quote before …
ameba23 Oct 28, 2024
944f931
Slightly improve error conversion
ameba23 Oct 28, 2024
14014a0
Merge master
ameba23 Oct 28, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
117 changes: 117 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions pallets/attestation/src/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -321,6 +321,7 @@ impl pallet_staking_extension::Config for Test {
type RuntimeEvent = RuntimeEvent;
type MaxPendingAttestations = MaxPendingAttestations;
type WeightInfo = ();
type PckCertChainVerifier = pallet_staking_extension::pck::MockPckCertChainVerifyer;
ameba23 marked this conversation as resolved.
Show resolved Hide resolved
}

parameter_types! {
Expand Down
1 change: 1 addition & 0 deletions pallets/propagation/src/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -313,6 +313,7 @@ impl pallet_staking_extension::Config for Test {
type RuntimeEvent = RuntimeEvent;
type MaxPendingAttestations = MaxPendingAttestations;
type WeightInfo = ();
type PckCertChainVerifier = pallet_staking_extension::pck::MockPckCertChainVerifyer;
}

parameter_types! {
Expand Down
1 change: 1 addition & 0 deletions pallets/registry/src/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -310,6 +310,7 @@ impl pallet_staking_extension::Config for Test {
type RuntimeEvent = RuntimeEvent;
type MaxPendingAttestations = MaxPendingAttestations;
type WeightInfo = ();
type PckCertChainVerifier = pallet_staking_extension::pck::MockPckCertChainVerifyer;
}

parameter_types! {
Expand Down
4 changes: 4 additions & 0 deletions pallets/staking/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,10 @@ sp-runtime ={ version="32.0.0", default-features=false }
sp-staking ={ version="27.0.0", default-features=false }
sp-std ={ version="14.0.0", default-features=false }
sp-consensus-babe ={ version="0.33.0", default-features=false }
x509-verify ={ version="0.4.6", features=["x509"] }
spki ="0.7.3"
p256 ={ version="0.13.2", default-features=false, features=["ecdsa"] }
rand ={ version="0.8.5", default-features=false, features=["alloc"] }

pallet-parameters={ version="0.2.0", path="../parameters", default-features=false }
entropy-shared={ version="0.2.0", path="../../crates/shared", features=[
Expand Down
27 changes: 19 additions & 8 deletions pallets/staking/src/benchmarking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
//! Benchmarking setup for pallet-propgation
#![allow(unused_imports)]
use super::*;
use crate::pck::MOCK_PCK_DERIVED_FROM_NULL_ARRAY;
#[allow(unused_imports)]
use crate::Pallet as Staking;
use entropy_shared::MAX_SIGNERS;
Expand Down Expand Up @@ -90,25 +91,35 @@ fn prep_bond_and_validate<T: Config>(
reward_destination,
));

let server_info = ServerInfo {
let joining_server_info = JoiningServerInfo {
tss_account: threshold,
x25519_public_key,
endpoint: vec![20, 20],
provisioning_certification_key: BoundedVec::with_max_capacity(),
pck_certificate_chain: vec![[0u8; 32].to_vec()],
};

if validate_also {
assert_ok!(<Staking<T>>::validate(
RawOrigin::Signed(bonder.clone()).into(),
ValidatorPrefs::default(),
server_info.clone(),
joining_server_info.clone(),
));

let validator_id = <T as pallet_session::Config>::ValidatorId::try_from(bonder)
.or(Err(Error::<T>::InvalidValidatorId))
.unwrap();

ThresholdToStash::<T>::insert(&server_info.tss_account, &validator_id);
ThresholdToStash::<T>::insert(&joining_server_info.tss_account, &validator_id);

let server_info = ServerInfo {
tss_account: joining_server_info.tss_account,
x25519_public_key: joining_server_info.x25519_public_key,
endpoint: joining_server_info.endpoint,
provisioning_certification_key: MOCK_PCK_DERIVED_FROM_NULL_ARRAY
.to_vec()
.try_into()
.unwrap(),
};
ThresholdServers::<T>::insert(&validator_id, server_info);
}
}
Expand Down Expand Up @@ -146,7 +157,7 @@ benchmarks! {
endpoint: vec![20, 20],
tss_account: _bonder.clone(),
x25519_public_key: NULL_ARR,
provisioning_certification_key: BoundedVec::with_max_capacity(),
provisioning_certification_key: MOCK_PCK_DERIVED_FROM_NULL_ARRAY.to_vec().try_into().unwrap(),
};
assert_last_event::<T>(Event::<T>::ThresholdAccountChanged(bonder, server_info).into());
}
Expand Down Expand Up @@ -261,14 +272,14 @@ benchmarks! {
prep_bond_and_validate::<T>(false, caller.clone(), bonder.clone(), threshold.clone(), NULL_ARR);
let validator_preference = ValidatorPrefs::default();

let server_info = ServerInfo {
let joining_server_info = JoiningServerInfo {
tss_account: threshold.clone(),
x25519_public_key: NULL_ARR,
endpoint: vec![20],
provisioning_certification_key: BoundedVec::with_max_capacity(),
pck_certificate_chain: vec![[0u8; 32].to_vec()],
ameba23 marked this conversation as resolved.
Show resolved Hide resolved
};

}: _(RawOrigin::Signed(bonder.clone()), validator_preference, server_info)
}: _(RawOrigin::Signed(bonder.clone()), validator_preference, joining_server_info)
verify {
assert_last_event::<T>(Event::<T>::AttestationCheckQueued(bonder).into());
}
Expand Down
39 changes: 38 additions & 1 deletion pallets/staking/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@ use serde::{Deserialize, Serialize};

pub use crate::weights::WeightInfo;

pub mod pck;

#[cfg(test)]
mod mock;

Expand Down Expand Up @@ -69,6 +71,7 @@ pub mod pallet {
DefaultNoBound,
};
use frame_system::pallet_prelude::*;
use pck::PckCertChainVerifier;
use rand_chacha::{
rand_core::{RngCore, SeedableRng},
ChaCha20Rng, ChaChaRng,
Expand Down Expand Up @@ -98,6 +101,7 @@ pub mod pallet {
type MaxPendingAttestations: Get<u32>;
/// The weight information of this pallet.
type WeightInfo: WeightInfo;
type PckCertChainVerifier: PckCertChainVerifier;
}

/// A unique identifier of a subgroup or partition of validators that have the same set of
Expand All @@ -120,6 +124,15 @@ pub mod pallet {
pub endpoint: TssServerURL,
pub provisioning_certification_key: VerifyingKey,
}

#[derive(Encode, Decode, Clone, Eq, PartialEq, RuntimeDebug, TypeInfo)]
ameba23 marked this conversation as resolved.
Show resolved Hide resolved
pub struct JoiningServerInfo<AccountId> {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is given as an argument to the validate extrinsic. I don't like having this extra struct, but i couldn't come up with another way of doing it which didn't make it possible that there exists a ServerInfo with no associated PCK which i want to avoid.

Ideally i would like to impl From<ServerInfo> for JoiningServerInfo but because it needs to call the cert validator which is a trait method, i couldn't figure out how to do that either.

Copy link
Collaborator

@HCastano HCastano Oct 23, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ideally i would like to impl From for JoiningServerInfo but because it needs to call the cert validator which is a trait method, i couldn't figure out how to do that either.

I think you actually want the other way around, but either way ya it seems a little tricky because of the T bound. We can do it as an internal method on the pallet implementation though.

    impl<T: Config> Pallet<T> {
        // ... snip ...
        fn server_info(joining_server_info: JoiningServerInfo<T::AccountId>) -> ServerInfo<T::AccountId> {
            let provisioning_certification_key =
                T::PckCertChainVerifier::verify_pck_certificate_chain(
                    joining_server_info.pck_certificate_chain,
                )
                .unwrap();

            ServerInfo::<T::AccountId> {
                tss_account: joining_server_info.tss_account,
                x25519_public_key: joining_server_info.x25519_public_key,
                endpoint: joining_server_info.endpoint,
                provisioning_certification_key,
            }
        }
}

This is given as an argument to the validate extrinsic. I don't like having this extra struct, but i couldn't come up with another way of doing it which didn't make it possible that there exists a ServerInfo with no associated PCK which i want to avoid.

But really the above conversion doesn't really handle the crux of the problem which is this. One approach is that we can pass in the certificate chain directly to the extrinsic and then have the ServerInfo.provisioning_certification_key field be an Option. It doesn't guarantee that it can't be None, but it is something that can be verified before writing ServerInfo to storage.

Copy link
Contributor Author

@ameba23 ameba23 Oct 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel pretty strongly about wanting assurance at the type level that every ServerInfo has a PCK - i don't want it to be an Option.

I would like to somehow improve the conversion between JoiningServerInfo and ServerInfo but its going to have to be in a follow up as i don't wanna keep resolving conflicts here.

pub tss_account: AccountId,
pub x25519_public_key: X25519PublicKey,
pub endpoint: TssServerURL,
pub pck_certificate_chain: Vec<Vec<u8>>,
}

/// Info that is requiered to do a proactive refresh
#[derive(Clone, Encode, Decode, Eq, PartialEq, RuntimeDebug, TypeInfo, Default)]
pub struct RefreshInfo {
Expand Down Expand Up @@ -344,6 +357,10 @@ pub mod pallet {
NoUnnominatingWhenSigner,
NoUnnominatingWhenNextSigner,
NoChangingThresholdAccountWhenSigner,
PckCertificateParse,
PckCertificateVerify,
PckCertificateBadPublicKey,
PckCertificateNoCertificate,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried to add the error variant for these as PckCertificate(PckParseVerifyError), but i couldn't get adding #[pallet::Error] to that error type to work, and without it it doesn't implement the needed traits.

}

#[pallet::event]
Expand Down Expand Up @@ -546,10 +563,30 @@ pub mod pallet {
pub fn validate(
origin: OriginFor<T>,
prefs: ValidatorPrefs,
server_info: ServerInfo<T::AccountId>,
joining_server_info: JoiningServerInfo<T::AccountId>,
ameba23 marked this conversation as resolved.
Show resolved Hide resolved
) -> DispatchResult {
let who = ensure_signed(origin.clone())?;

let provisioning_certification_key =
T::PckCertChainVerifier::verify_pck_certificate_chain(
joining_server_info.pck_certificate_chain,
)
.map_err(|error| match error {
pck::PckParseVerifyError::Parse => Error::<T>::PckCertificateParse,
pck::PckParseVerifyError::Verify => Error::<T>::PckCertificateVerify,
pck::PckParseVerifyError::BadPublicKey => {
Error::<T>::PckCertificateBadPublicKey
},
pck::PckParseVerifyError::NoCertificate => {
Error::<T>::PckCertificateNoCertificate
},
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should try and implement a From implementation on these so that we can use ?/into() here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 i've added one but we still need to call map_err to tell rust what type we are converting into because this returns a DispatchResult, not a pallet::Error<T>.

})?;
let server_info = ServerInfo::<T::AccountId> {
tss_account: joining_server_info.tss_account,
x25519_public_key: joining_server_info.x25519_public_key,
endpoint: joining_server_info.endpoint,
provisioning_certification_key,
};
ensure!(
server_info.endpoint.len() as u32 <= T::MaxEndpointLength::get(),
Error::<T>::EndpointTooLong
Expand Down
Loading
Loading