fix for txt output length of plain PQ key material #268
Conversation
oqsprov/oqs_encode_key2any.c
Outdated
| @@ -1160,7 +1160,7 @@ static int oqsx_to_text(BIO *out, const void *key, int selection) | |||
| } | |||
|
|
|||
| if ((selection & OSSL_KEYMGMT_SELECT_PRIVATE_KEY) != 0) { | |||
| int classic_key_len = 0; | |||
| int classic_key_len = 0 - SIZE_OF_UINT32; | |||
There was a problem hiding this comment.
Initializing a length variable to a negative number, seems counterintuitive. I'll assume what you've done indeed fixes the bug, but I wonder if there's a more natural way, or at least if there should be a comment explaining this counterintuitive step.
There was a problem hiding this comment.
Perhaps you can walk me through the logic in the subsequent: does line 1169 add to classic_key_len, so that on 1171 it is now non-negative?
There was a problem hiding this comment.
Yes. The code is shared between hybrid (numkeys>1) and plain PQ keys. The classic_key_len element is (obviously) only relevant for hybrid keys (as is, err, should be, the 4 bytes storing it). This pattern is not unusual throughout provider -- but I agree that initializing a length to a negative value (to get it added back in the subsequent keytype independent subtraction for plain PQ keys) may be "surprising". Will revise the code for easier understanding -- but doing this "for real", i.e., in the complete code base, will probably require many more changes...
…#268) * fix for txt output length of plain PQ key material * clarify use of hybrids in txt encoder * add txt/DER/PEM test and make key output dependent on tool availability Signed-off-by: Felipe Ventura <[email protected]>
…#268) * fix for txt output length of plain PQ key material * clarify use of hybrids in txt encoder * add txt/DER/PEM test and make key output dependent on tool availability Signed-off-by: Felipe Ventura <[email protected]>
…#268) * fix for txt output length of plain PQ key material * clarify use of hybrids in txt encoder * add txt/DER/PEM test and make key output dependent on tool availability Signed-off-by: Felipe Ventura <[email protected]>
…#268) * fix for txt output length of plain PQ key material * clarify use of hybrids in txt encoder * add txt/DER/PEM test and make key output dependent on tool availability Signed-off-by: Felipe Ventura <[email protected]>
…#268) * fix for txt output length of plain PQ key material * clarify use of hybrids in txt encoder * add txt/DER/PEM test and make key output dependent on tool availability Signed-off-by: Felipe Ventura <[email protected]>
Fixes #267.