boltlabs-inc / key-mgmt-spec

Formal specification for the key management project
MIT License
3 stars 2 forks source link

Fix public-key return bug #140

Closed marsella closed 2 years ago

marsella commented 2 years ago

Closes #135.

This modifies the spec to return a public key after generation and import of signing keys. The protocols for arbitrary secrets stay the same; I added the changes in a small changelog in the signing key section.

Question: when the client imports a public key, should we also have a validation step at the end where they check that the public key is "what they expect it to be"? It's only useful to catch a mistake by the key server (or e.g. a network error) in case they stored the wrong thing, but a malicious key server could just lie about what they're storing.

Once this content is approved, I'll make issues in key-mgmt to document the code changes that need to be done.

indomitableSwan commented 2 years ago

@marsella Are we hoping to include these changes during this development phase?

We should:

marsella commented 2 years ago

Should the client run validation checks on the exported (full) keypair?

As with my answer above, it seems simpler to return just the signing key and have the client locally compute the public key. But if we do return both, I can add clarification that they need to be validated.

Should we clarify that export returns the full keypair?

I definitely think we should be explicit about what gets returned from export. Right now, the generic retrieve says

If context is set to "export", the client computes exported key as len || arbitrary_key || len_ad || associated_data, where len is one byte describing the length of arbitrary_key in bytes, and len_ad is one byte describing the length of associated_data in bytes, and outputs exported_key to the calling application.

There's no clarification as far as I can tell in the signing keys section that gives a specific format for signing keys. I also just now realize that the import section should probably define the input format the same way.

indomitableSwan commented 2 years ago

Should the client run validation checks on the exported (full) keypair?

As with my answer above, it seems simpler to return just the signing key and have the client locally compute the public key. But if we do return both, I can add clarification that they need to be validated.

I think we should keep things simple, as you suggest. If we were building a user-facing application on top of this, I'd want to include some extra features here for ease of use, like key fingerprints a human can read to double check they've exported the key they intended to or some such. But we're not.

Should we clarify that export returns the full keypair?

I definitely think we should be explicit about what gets returned from export. Right now, the generic retrieve says

If context is set to "export", the client computes exported key as len || arbitrary_key || len_ad || associated_data, where len is one byte describing the length of arbitrary_key in bytes, and len_ad is one byte describing the length of associated_data in bytes, and outputs exported_key to the calling application.

There's no clarification as far as I can tell in the signing keys section that gives a specific format for signing keys. I also just now realize that the import section should probably define the input format the same way.

I agree with both of your points above. We should fix or document as a bug.

marsella commented 2 years ago

Updated boltlabs-inc/key-mgmt#306 to include the specific spec changes required. I can break that into two issues (spec and implementation) if you'd rather.

indomitableSwan commented 2 years ago

boltlabs-inc/key-mgmt#306

I think it's fine as is.