Add EK Certificate Chain support #116
Conversation
1552_ek_certificate_chain.md
Outdated
|
|
||
| ## Proposal | ||
|
|
||
| * keylime_agent, send `ek_ca_chain` to registrar |
There was a problem hiding this comment.
We might get away with just storing the entire chain in the ekcert field. @ansasaki any opinion on that?
There was a problem hiding this comment.
I agree with @THS-on that most probably a new field is not needed. We could simply append the intermediate certificates to the EK certificate.
There was a problem hiding this comment.
I will then implement the necessary changes to store the chain in the ekcert field and update the existing fields in the database. I will need to add "-----BEGIN CERTIFICATE-----" and "-----END CERTIFICATE-----" (in PEM format) to the existing ekcerts in the database, as I require a delimiter in the chain.
Please notify me when everyone agrees that this is the way to proceed.
There was a problem hiding this comment.
Yep using PEM encoding for the chain is probably a good idea, as this is standard and then can also be easily read by e.g. openssl
1552_ek_certificate_chain.md
Outdated
|
|
||
| #### Providing big chains as attack | ||
| * Limit the amount of allowed chain size | ||
| * Use mTLS to only allow verified clients access |
There was a problem hiding this comment.
Not really related to this change.
1552_ek_certificate_chain.md
Outdated
| ## Proposal | ||
|
|
||
| * keylime_agent, send `ek_ca_chain` to registrar | ||
| * keylime registrar, store `ek_ca_chain` in database |
There was a problem hiding this comment.
If possible, the chain should be stored with the EK certificate and no new entry is added to the database.
Sorry for suggesting that before, but I think the less we modify the database the better.
1552_ek_certificate_chain.md
Outdated
|
|
||
| * keylime_agent, send `ek_ca_chain` to registrar | ||
| * keylime registrar, store `ek_ca_chain` in database | ||
| * keylime tenant, verify `ekcert` against `ek_ca_chain` and `ek_ca_chain` against `tpm_cert_store` |
There was a problem hiding this comment.
The process of verifying the EK certificate necessarily involves verifying its signature against the whole chain.
I think this sentence can be removed or rephrased to not imply that there are separate steps on the verification process.
1552_ek_certificate_chain.md
Outdated
| ### Risks and Mitigations | ||
|
|
||
| #### Registrar/Tenant could be become incompatible with older database | ||
| * Update database to new scheme, only a single key is added to the registar db 'ek_ca_chain' |
There was a problem hiding this comment.
If possible, avoid changing the database and store the certificate chain with the EK certificate.
1552_ek_certificate_chain.md
Outdated
| * Update database to new scheme, only a single key is added to the registar db 'ek_ca_chain' | ||
|
|
||
| #### Registrar/Tenant could become incompatible with older Agent | ||
| * Make 'ek_ca_chain' optional |
There was a problem hiding this comment.
If possible, avoid adding a new parameter and simply append the chain to the EK certificate.
|
Shall i update the enhancement request and then just open a new pull request? |
@ematery sorry this got lost in my inbox. You can just update the current PR |
|
@ematery what is the status here? I think once the suggested changes are in the PR, we can merge this. |
|
@THS-on Thanks for the reminder. I updated the pull request. |
|
@ematery our PR now also includes another markdown document. Can you rebase on latest master and ensure that you are not accidentally including |
ematery
force-pushed
the
master
branch
2 times, most recently
from
a608d7a to
b54a1cd
Compare
Signed-off-by: Eugen Matery <e.matery@digitalendoscopy.de>
Signed-off-by: Eugen Matery <e.matery@digitalendoscopy.de>
Co-authored-by: Thore Sommer <THS-on@users.noreply.github.com> Signed-off-by: Eugen Matery <e.matery@digitalendoscopy.de>
Co-authored-by: Thore Sommer <THS-on@users.noreply.github.com> Signed-off-by: Eugen Matery <e.matery@digitalendoscopy.de>
|
Are there still open points? |
|
Don't think so. @ansasaki can you have a look at it before merging? |
| ### Risks and Mitigations | ||
|
|
||
| #### Registrar/Tenant could be become incompatible with older database | ||
| * Update existing database values for `ekcert` to PEM format. |
There was a problem hiding this comment.
Note: make sure the newer version of the registrar and tenant can read both formats from the database (DER and PEM), to keep it compatible with data coming from an existing database.
|
|
||
| ## Proposal | ||
|
|
||
| * keylime_agent: update `ekcert` field to use PEM format, so multiple certificates can be stored in the field and a marker exists that shows the start and end of each certificate. |
There was a problem hiding this comment.
Just a note: this is an API breaking change as it changes the ABI (it is backwards incompatible). This requires a major API version bump.
No description provided.