security: zeroize pem encoded key#504
Conversation
|
I had quite some fun while implementing this, especially recalling the TLV encoding and writing a custom recursive parser. |
`PemEncodedKey` is now zeroized and no longer contains ASN.1 parsed data. Because `Vec<simple_asn1::ASN1Block>` owns the underlying bytes and does not support zeroization, the full ASN.1 parsing has been replaced by a custom partial ASN.1 implementation. This parsing is performed only to classify the key type during construction and to extract the key bytes on access. This change also allows the removal of the `simple_asn1` dependency.
|
This will likely take me a while to review, my ASN.1 skills are rusty at best |
|
Let me
|
arckoor
left a comment
There was a problem hiding this comment.
I haven't reviewed the logic yet, but I fuzzed it a bit
| }; | ||
|
|
||
| // See <https://en.wikipedia.org/wiki/X.690#Length_octets> for length encoding | ||
| let len = bytes[0]; |
There was a problem hiding this comment.
Out of bounds:
let _ = read_tlv(&mut [0x1f, 0]);
// with a certificate:
let data = "
-----BEGIN PUBLIC KEY-----
AA==
-----END PUBLIC KEY-----";
let _ = DecodingKey::from_rsa_pem(data.as_bytes());Add a regression test as well please
PemEncodedKeyis now zeroized and no longer contains ASN.1 parseddata. Because
Vec<simple_asn1::ASN1Block>owns the underlying bytesand does not support zeroization, the full ASN.1 parsing has been
replaced by a custom partial ASN.1 implementation. This parsing is
performed only to classify the key type during construction and to
extract the key bytes on access.
This change also allows the removal of the
simple_asn1dependency.Closes #337 after #483 is merged.