Handle PCK certificates

[ameba23]

This is a followup to #1051 in that it adds a way of parsing and validating the PCK certificates, as well as intermediary 'provider' certificates which sign the PCK cert and are in turn signed by intel's root certificate authority public key, which is hard coded. If all goes well the PCK public key is extracted from the certificate to be used in the ServerInfo struct.

This parsing / validating needs to happen when a validator initially joins, or when they change their endpoint (IP address).

@junkicide from poetic has also provided us with a function to validate a certificate chain of arbitrary length - im not sure whether in practice we can expect that there will be more than one intermediary provider beween the PCK cert and the root CA.

Related issue: #982

[ameba23]

@HCastano now that #1063 is finished, i am a little bit unsure how to go ahead with this.

The function parse_pck_cert_chain which i have exposed here in the attestation pallet needs to be called by the staking pallet's validate function.

But it seems we can't have the attestation pallet be a dependency of the staking pallet.

Easiest thing to do would be to move this code to the staking pallet. But logically it kinda belongs here because its kinda related to attestation.

So dunno if i should make a crate for this, or think up another place to put it, or just not worry and stick it in the staking pallet (in its own module).

[HCastano]

@ameba23 I think it's fine for this to live somewhere in/near the staking pallet for now as long as it get abstracted correctly, e.g we go through an associated type (T::CertificateVerifier::verify_pck(...)) rather than hardcoding some of these decisions into the pallet.

A module with the trait implementation is probably alright. A standalone crate seems overkill at this point.

[ameba23]

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.

[HCastano]

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.

[ameba23]

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.

[ameba23]

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.

[ameba23]

@JesseAbram @HCastano a gentle reminder this has been up for review for a few weeks now. There were some messy conflicts and i'd like to avoid having more.

[ameba23]

Still getting an error when running the staking pallet benchmarks, will fix tomorrow.

[HCastano]

@ameba23 sorry, forgot about this. Will take a look today 🙇

[HCastano]

Left a few comments but nothing major.

I wasn't able to dig into your error conversion problem, if I have time tomorrow I might take another look at it.

[HCastano]

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

[ameba23]

👍 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>.

[HCastano]

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.