Explicitly zero potentially sensitive data in software crypto and cxgbe.

[jhb]

Summary

Add explicit bzero's of sensitive data in software crypto consumers.

Explicitly zero IVs, block buffers, and hashes/digests.

Explicitly zero on-stack IVs, tags, and HMAC keys.

Explicitly AES zero key schedules on the stack.

Explicitly zero IVs on the stack.

Test Plan

• tested with KTLS sending (uses AES-GCM) and cryptocheck

[jhb]

Xin,

Conrad suggested you might be the best one to review this. These buffers are all on-stack buffers and contain different types of data: IVs / nonces, temporary copies of blocks of cipher or plain text, key schedules, and hash results. The current patch takes the approach of trying to zero all of the things I could find in cryptosoft, aesni, and ccr. However, if some of these data types aren't really sensitive I'm happy to pare down the number of explicit zeroes.

[delphij]

Looks good to me (there are a few minor nits or optional alternative proposals)

sys/crypto/aesni/aesni.c

819

GOOD: iv and tag were properly zeroed here, no exit after their first taint with data.

849

[OPTIONAL] Personally I'd probably just copy key to hmac_key and pad the remaining cells with 0's, and do a full sized ^ HMAC_IPAD_VAL, but that's just a personal taste and I'm fine if this is kept as-is as long as keylen is guaranteed to be <= sizeof(hmac_key) which appear to be checked in aesni_authprepare() if I followed the code correctly.

874

GOOD: hmac_key is only used in this block and the cleanse was done correctly. I'd probably define the variable in this if block to make it clear.

898

GOOD: res2 is only used in this block and the cleanse was done correctly. I'd probably move the definition into this block to make that clear.

901

GOOD: no exit point after res gets taineted and cleanse here is correct.

sys/dev/cxgbe/crypto/t4_crypto.c

754

GOOD: tainted in line 675 and no exit point befor this cleanse.

1073

GOOD: tainted in line 975 and no other exit point before cleansed here.

1356

GOOD: tainted in line 1275 and no exit point before cleansed here.

1501

GOOD: defined locally, tainted in line 1482 and no exit point before cleanse.

1509

GOOD: no exit points prior to this point for all 3 variables.

1824

GOOD: tainted in line 1731 and no exit points.

1964

GOOD: local variable cleansed at end of scope and no exit point.

1972

GOOD: no exit point before cleansing.

sys/dev/cxgbe/crypto/t4_keyctx.c

78

GOOD.

173

GOOD.

sys/opencrypto/cryptosoft.c

280

GOOD: tainted in line 182 and no exit before cleanse.

281

GOOD: tainted in line 142 and cleansed without exit.

282

GOOD: First potential taint in line 216 and no exit before cleanse.

392

GOOD: note that uaalg should probably be defined in this if block to make it clear that it only needs to get cleansed here.

397

GOOD: First potential taint in line 362 and properly cleansed here with no exit before.

454

GOOD: note that uaalg should probably be defined in this if block to make it clear that it only needs to get cleansed here.

459

GOOD: First possible taint in line 434 and no exit prior to this.

460

GOOD: First possible taint in line 446 and no exit prior to this.

461

GOOD: First possible taint in line 429 and no exit prior to this.

555

GOOD: note that uaalg should probably be defined in this if block to make it clear that it only needs to get cleansed here.

580

GOOD: First possible taint in line 508 and no exit prior to this.

581

GOOD: First possible taint in line 546 and no exit prior to this.

582

GOOD: First possible taint in line 498 and no exit prior to this.

637

GOOD: note that uaalg should probably be defined in this if block to make it clear that it only needs to get cleansed here.

657

GOOD: ctx are cleansed by Final methods.

771

GOOD: tainted in line 697 and no return here.

772

GOOD: tainted in line 712 and cleansed here without exit.

773

GOOD: tainted in line 680 and no return here.

[jhb]

sys/crypto/aesni/aesni.c

849

Most other drivers now use a single function to generate the partial hash context which does exactly this (sys/opencrypto/crypto.c: hmac_init_pad()). The key length is checked and should be short enough, yes.

898

style(9) discourages moving the declarations into blocks, but I think in this case it might be a compelling reason to do so.