Add TLS13-KDF

[simo5]

Description

Add support for OpenSSL's TLS13-KDF which is a limited TLS-specific HKDF version.

Fixes #445

Checklist

• Code modified for feature
• Test suite updated with functionality tests
• [ ] Test suite updated with negative tests
• [ ] Documentation updated

Reviewer's checklist:

• Any issues marked for closing are addressed
• There is a test suite reasonably covering new functionality or modifications
• This feature/change has adequate documentation added
• Code conform to coding style that today cannot yet be enforced via the check style test
• Commits have short titles and sensible commit messages
• Coverity Scan has run if needed (code PR) and no new defects were found

[simo5]

@kshitizvars would you mind testing this PR? It should solve the issues you have with tls1.3 fully on the token, note that you have to block digest operations with the block operations quirk.

[simo5]

Note that the kryoptic build will fail until latchset/kryoptic#99 is merged

[kshitizvars]

Hi @simo5

Thanks for the patch!!
I have tested your TLS13-kdf patch. OpenSSL calls are coming to tls based kdf implementation but after reverting 4f67dfd

FYI, with 4f67dfd patch, I am getting below issue:-

ACCEPT
ERROR
20F076AFFFFF0000:error:0A00013A:SSL routines:tls_parse_ctos_key_share:unable to find ecdh parameters:ssl/statem/extensions_srvr.c:684:
shutting down SSL
CONNECTION CLOSED

We are currently working on fixing the issue, comimg with the faulty patch.

[simo5]

@kshitizvars so your token has issues storing/using session objects?

[simo5]

Covscan found an error:

*** CID 510334:  Memory - corruptions  (OVERRUN)
/src/kdf.c: 353 in p11prov_tls13_expand_label()
347         params.pInfo = info;
348         params.ulInfoLen = infolen;
349    
350         ret = inner_derive_key(hkdfctx->provctx, keyobj, &hkdfctx->session,
351                                &mechanism, keylen, dkey_handle);
352    
>>>     CID 510334:  Memory - corruptions  (OVERRUN)
>>>     Overrunning array "info" of 514 bytes by passing it to a function which accesses it at byte offset 766 using argument "infolen" (which evaluates to 767).
353         OPENSSL_cleanse(&info, infolen);
354         return ret;
355     }
356    
357     static CK_RV p11prov_tls13_derive_secret(P11PROV_KDF_CTX *hkdfctx,
358                                              P11PROV_OBJ *keyobj, size_t keylen,

[simo5]

Covscan found an error:

*** CID 510334:  Memory - corruptions  (OVERRUN)
/src/kdf.c: 353 in p11prov_tls13_expand_label()
347         params.pInfo = info;
348         params.ulInfoLen = infolen;
349    
350         ret = inner_derive_key(hkdfctx->provctx, keyobj, &hkdfctx->session,
351                                &mechanism, keylen, dkey_handle);
352    
>>>     CID 510334:  Memory - corruptions  (OVERRUN)
>>>     Overrunning array "info" of 514 bytes by passing it to a function which accesses it at byte offset 766 using argument "infolen" (which evaluates to 767).

I think Coverity here is confused as the big check on inputs should make it impossible to get such a large value.

353         OPENSSL_cleanse(&info, infolen);
354         return ret;
355     }
356    
357     static CK_RV p11prov_tls13_derive_secret(P11PROV_KDF_CTX *hkdfctx,
358                                              P11PROV_OBJ *keyobj, size_t keylen,

In any case I am changing the code to add belts and suspender so the total info size is more
explicitly checked, and also to reduce the number of variables used to the minimum necessary so there is no chance of mis-assigning anything and getting to an inconsistent state.

[beldmit]

LGTM

[simo5]

Thanks for all the reviews folks.