Crypto updates #45
Crypto updates #45
Conversation
|
|
||
| System.arraycopy(PS, 0, DB, 0, emLen - sLen - hLen - 2); | ||
| DB[emLen - sLen - hLen - 2] = (byte) 0x01; | ||
| System.arraycopy(salt, 0, DB, emLen - sLen - hLen - 1, sLen); |
There was a problem hiding this comment.
This would probably be more clear if you precalc the length of PS and put it in a variable or use PS.lenght()
|
|
||
| // 10. Let maskedDB = DB \xor dbMask. | ||
| for (int i = 0; i < maskedDB.length; i++) { | ||
| maskedDB[i] = (byte) (((int) DB[i]) ^ ((int) dbMask[i])); |
There was a problem hiding this comment.
(byte) (((int) DB[i]) ^ ((int) dbMask[i])); [](start = 26, length = 43)
Do we need to explicitly cast to int here before doing the XOR
| // Let mHash = Hash(M), an octet string of length hLen. | ||
| // function takes in the mHash. | ||
|
|
||
| // salt length for the service corresponds to the digest length, which is the hash length / 8 |
There was a problem hiding this comment.
which is the hash length / 8 [](start = 72, length = 29)
perhaps you should call this hash length in bytes as it might be confused with mHash.length() / 8
| */ | ||
| protected boolean EMSA_PSS_VERIFY(byte[] mHash, byte[] EM, int emBits, MessageDigest messageDigest) { | ||
|
|
||
| // salt length for the service corresponds to the digest length, which is the hash length / 8 |
There was a problem hiding this comment.
which is the hash length / 8 [](start = 72, length = 29)
Same as above
| return false; | ||
| } | ||
| mask = 0x00; | ||
| } |
There was a problem hiding this comment.
Could be simplified to:
byte mask = (byte)0x000000FF >> (8*emLen - emBits);
if(mask | maskedDB[0] != mask) { return false; }
| byte[] EM = I2OSP(m, _emLen); | ||
|
|
||
| MessageDigest messageDigest = MessageDigest.getInstance(_digestName); | ||
| return EMSA_PSS_VERIFY(digest, EM, _modBits, messageDigest); |
There was a problem hiding this comment.
_modBits [](start = 47, length = 8)
_modBits-1
-
EMSA-PSS verification: Apply the EMSA-PSS verification operation
(Section 9.1.2) to the message M and the encoded message EM to
determine whether they are consistent:Result = EMSA-PSS-VERIFY (M, EM, modBits - 1).
There was a problem hiding this comment.
EMSA-PSS-VERIFY is actually supposed to use emBits, not modBits. emBits is the bit length of EM, and in the ctor of we call getOctetLength to get _emLen which is the octet length of EM. In theory, emBits should always be the same as _emLen / 8.
emBits and emLen are defined in https://tools.ietf.org/html/rfc3447#section-2
| byte[] DB = new byte[Math.min(maskedDB.length, dbMask.length)]; | ||
| for (int i = 0; i < maskedDB.length; i++) { | ||
| DB[i] = (byte) (maskedDB[i] ^ dbMask[i]); | ||
| } |
There was a problem hiding this comment.
are maskedDB and dbMask expected to be the same length here? If so we should probably assert that they are rather than taking the min length and truncating the data in dbMask or maskedDB.
| */ | ||
| protected byte[] MGF(byte[] mgfSeed, int maskLen, MessageDigest digest) { | ||
|
|
||
| if (maskLen > Math.pow(2, 32) * 32) { |
There was a problem hiding this comment.
- 32 [](start = 37, length = 5)
shouldn't this be (digest.getDigestLength()) instead of hardcoding 32
There was a problem hiding this comment.
sorry its not super clear which 32 I was commenting on. shouldn't this be:
masklen > Math.pow(2,32) * (digest.getDigestLength())
| */ | ||
| protected byte[] MGF(byte[] mgfSeed, int maskLen, MessageDigest digest) { | ||
|
|
||
| if (maskLen > Math.pow(2, 32) * 32) { |
There was a problem hiding this comment.
Add a comment denoting the step. //1. If maskLen > 2^32 hLen, output "mask too long" and stop.
| // b.Concatenate the hash of the seed Z and C to the octet string T: T = | ||
| // T || Hash (Z || C) | ||
| byte[] hash = digest.digest(); | ||
| mask = Bytes.concat(mask, hash); |
There was a problem hiding this comment.
mask = Bytes.concat(mask, hash); [](start = 12, length = 32)
we should preallocate the buffer and load it rather than using concat and allocating a new byte[] on each iteration
| /** | ||
| * | ||
| */ | ||
| public abstract class PsBase extends RsaSignature { |
There was a problem hiding this comment.
Suggest RsaPssSignature as the name for this class.
| super(name); | ||
| } | ||
|
|
||
| class PsBaseSignatureTransform implements ISignatureTransform { |
There was a problem hiding this comment.
And RsaPssSignatureTransform for this one :-)
| _keyPair = keyPair; | ||
| BigInteger modulus = ((RSAPublicKey) _keyPair.getPublic()).getModulus(); | ||
| _emLen = getOctetLength(modulus.bitLength()); | ||
| _modBits = ((RSAPublicKey) _keyPair.getPublic()).getModulus().bitLength(); |
There was a problem hiding this comment.
modulus was already extracted above, so _modBits is just modulus.bitLength(). And that was used in getOctetLength as well. So an opportunity to condense this a bit.
| // integer OS2IP (EM) (see Section 4.2) is at most modBits - 1, where | ||
| // modBits is the length in bits of the RSA modulus n: | ||
| MessageDigest messageDigest = MessageDigest.getInstance(_digestName); | ||
| byte[] EM = EMSA_PSS_ENCODE_HASH(digest, _modBits - 1, messageDigest); |
There was a problem hiding this comment.
Shouldn't EMSA_PSS_ENCODE use emBits rather than modBits? Where emBits would be emLen / 8?
No description provided.