Skip to content

Commit 5c8caa7

Browse files
peterdettmanpeterdettman
committed
Cleanup obsolete TODO
1 parent ae57c7a commit 5c8caa7

File tree

2 files changed

+41
-81
lines changed

2 files changed

+41
-81
lines changed

‎tls/src/main/java/org/bouncycastle/tls/crypto/impl/bc/BcDefaultTlsCredentialedDecryptor.java

+22-42
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@
1313
import org.bouncycastle.tls.TlsCredentialedDecryptor;
1414
import org.bouncycastle.tls.crypto.TlsCryptoParameters;
1515
import org.bouncycastle.tls.crypto.TlsSecret;
16-
import org.bouncycastle.tls.crypto.impl.TlsImplUtils;
1716
import org.bouncycastle.util.Arrays;
1817

1918
/**
@@ -76,11 +75,11 @@ public TlsSecret decrypt(TlsCryptoParameters cryptoParams, byte[] ciphertext) th
7675
}
7776

7877
/*
79-
* TODO[tls-ops] Probably need to make RSA encryption/decryption into TlsCrypto functions so
80-
* that users can implement "generic" encryption credentials externally
78+
* TODO[tls-ops] Probably need to make RSA encryption/decryption into TlsCrypto functions so that users
79+
* can implement "generic" encryption credentials externally
8180
*/
82-
protected TlsSecret safeDecryptPreMasterSecret(TlsCryptoParameters cryptoParams, RSAKeyParameters rsaServerPrivateKey,
83-
byte[] encryptedPreMasterSecret)
81+
protected TlsSecret safeDecryptPreMasterSecret(TlsCryptoParameters cryptoParams,
82+
RSAKeyParameters rsaServerPrivateKey, byte[] encryptedPreMasterSecret)
8483
{
8584
SecureRandom secureRandom = crypto.getSecureRandom();
8685

@@ -89,12 +88,8 @@ protected TlsSecret safeDecryptPreMasterSecret(TlsCryptoParameters cryptoParams,
8988
*/
9089
ProtocolVersion expectedVersion = cryptoParams.getRSAPreMasterSecretVersion();
9190

92-
// TODO Provide as configuration option?
93-
boolean versionNumberCheckDisabled = false;
94-
9591
/*
96-
* Generate 48 random bytes we can use as a Pre-Master-Secret, if the
97-
* PKCS1 padding check should fail.
92+
* Generate 48 random bytes we can use as a Pre-Master-Secret, if the PKCS1 padding check should fail.
9893
*/
9994
byte[] fallback = new byte[48];
10095
secureRandom.nextBytes(fallback);
@@ -110,46 +105,31 @@ protected TlsSecret safeDecryptPreMasterSecret(TlsCryptoParameters cryptoParams,
110105
catch (Exception e)
111106
{
112107
/*
113-
* This should never happen since the decryption should never throw an exception
114-
* and return a random value instead.
108+
* This should never happen since the decryption should never throw an exception and return a
109+
* random value instead.
115110
*
116-
* In any case, a TLS server MUST NOT generate an alert if processing an
117-
* RSA-encrypted premaster secret message fails, or the version number is not as
118-
* expected. Instead, it MUST continue the handshake with a randomly generated
119-
* premaster secret.
111+
* In any case, a TLS server MUST NOT generate an alert if processing an RSA-encrypted premaster
112+
* secret message fails, or the version number is not as expected. Instead, it MUST continue the
113+
* handshake with a randomly generated premaster secret.
120114
*/
121115
}
122116

123117
/*
124-
* If ClientHello.legacy_version is TLS 1.1 or higher, server implementations MUST check the
125-
* version number [..].
118+
* Compare the version number in the decrypted Pre-Master-Secret with the legacy_version field from
119+
* the ClientHello. If they don't match, continue the handshake with the randomly generated 'fallback'
120+
* value.
121+
*
122+
* NOTE: The comparison and replacement must be constant-time.
126123
*/
127-
if (versionNumberCheckDisabled && !TlsImplUtils.isTLSv11(expectedVersion))
128-
{
129-
/*
130-
* If the version number is TLS 1.0 or earlier, server implementations SHOULD check the
131-
* version number, but MAY have a configuration option to disable the check.
132-
*/
133-
}
134-
else
135-
{
136-
/*
137-
* Compare the version number in the decrypted Pre-Master-Secret with the legacy_version
138-
* field from the ClientHello. If they don't match, continue the handshake with the
139-
* randomly generated 'fallback' value.
140-
*
141-
* NOTE: The comparison and replacement must be constant-time.
142-
*/
143-
int mask = (expectedVersion.getMajorVersion() ^ (M[0] & 0xFF))
144-
| (expectedVersion.getMinorVersion() ^ (M[1] & 0xFF));
124+
int mask = (expectedVersion.getMajorVersion() ^ (M[0] & 0xFF))
125+
| (expectedVersion.getMinorVersion() ^ (M[1] & 0xFF));
145126

146-
// 'mask' will be all 1s if the versions matched, or else all 0s.
147-
mask = (mask - 1) >> 31;
127+
// 'mask' will be all 1s if the versions matched, or else all 0s.
128+
mask = (mask - 1) >> 31;
148129

149-
for (int i = 0; i < 48; i++)
150-
{
151-
M[i] = (byte)((M[i] & mask) | (fallback[i] & ~mask));
152-
}
130+
for (int i = 0; i < 48; i++)
131+
{
132+
M[i] = (byte)((M[i] & mask) | (fallback[i] & ~mask));
153133
}
154134

155135
return crypto.createSecret(M);

‎tls/src/main/java/org/bouncycastle/tls/crypto/impl/jcajce/JceDefaultTlsCredentialedDecryptor.java

+19-39
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@
1212
import org.bouncycastle.tls.TlsCredentialedDecryptor;
1313
import org.bouncycastle.tls.crypto.TlsCryptoParameters;
1414
import org.bouncycastle.tls.crypto.TlsSecret;
15-
import org.bouncycastle.tls.crypto.impl.TlsImplUtils;
1615
import org.bouncycastle.util.Arrays;
1716

1817
/**
@@ -70,11 +69,11 @@ public TlsSecret decrypt(TlsCryptoParameters cryptoParams, byte[] ciphertext) th
7069
}
7170

7271
/*
73-
* TODO[tls-ops] Probably need to make RSA encryption/decryption into TlsCrypto functions so
74-
* that users can implement "generic" encryption credentials externally
72+
* TODO[tls-ops] Probably need to make RSA encryption/decryption into TlsCrypto functions so that users
73+
* can implement "generic" encryption credentials externally
7574
*/
7675
protected TlsSecret safeDecryptPreMasterSecret(TlsCryptoParameters cryptoParams, PrivateKey rsaServerPrivateKey,
77-
byte[] encryptedPreMasterSecret)
76+
byte[] encryptedPreMasterSecret)
7877
{
7978
SecureRandom secureRandom = crypto.getSecureRandom();
8079

@@ -83,12 +82,8 @@ protected TlsSecret safeDecryptPreMasterSecret(TlsCryptoParameters cryptoParams,
8382
*/
8483
ProtocolVersion expectedVersion = cryptoParams.getRSAPreMasterSecretVersion();
8584

86-
// TODO Provide as configuration option?
87-
boolean versionNumberCheckDisabled = false;
88-
8985
/*
90-
* Generate 48 random bytes we can use as a Pre-Master-Secret, if the
91-
* PKCS1 padding check should fail.
86+
* Generate 48 random bytes we can use as a Pre-Master-Secret, if the PKCS1 padding check should fail.
9287
*/
9388
byte[] fallback = new byte[48];
9489
secureRandom.nextBytes(fallback);
@@ -107,43 +102,28 @@ protected TlsSecret safeDecryptPreMasterSecret(TlsCryptoParameters cryptoParams,
107102
catch (Exception e)
108103
{
109104
/*
110-
* A TLS server MUST NOT generate an alert if processing an
111-
* RSA-encrypted premaster secret message fails, or the version number is not as
112-
* expected. Instead, it MUST continue the handshake with a randomly generated
113-
* premaster secret.
105+
* A TLS server MUST NOT generate an alert if processing an RSA-encrypted premaster secret message
106+
* fails, or the version number is not as expected. Instead, it MUST continue the handshake with a
107+
* randomly generated premaster secret.
114108
*/
115109
}
116110

117111
/*
118-
* If ClientHello.legacy_version is TLS 1.1 or higher, server implementations MUST check the
119-
* version number [..].
112+
* Compare the version number in the decrypted Pre-Master-Secret with the legacy_version field from
113+
* the ClientHello. If they don't match, continue the handshake with the randomly generated 'fallback'
114+
* value.
115+
*
116+
* NOTE: The comparison and replacement must be constant-time.
120117
*/
121-
if (versionNumberCheckDisabled && !TlsImplUtils.isTLSv11(expectedVersion))
122-
{
123-
/*
124-
* If the version number is TLS 1.0 or earlier, server implementations SHOULD check the
125-
* version number, but MAY have a configuration option to disable the check.
126-
*/
127-
}
128-
else
129-
{
130-
/*
131-
* Compare the version number in the decrypted Pre-Master-Secret with the legacy_version
132-
* field from the ClientHello. If they don't match, continue the handshake with the
133-
* randomly generated 'fallback' value.
134-
*
135-
* NOTE: The comparison and replacement must be constant-time.
136-
*/
137-
int mask = (expectedVersion.getMajorVersion() ^ (M[0] & 0xFF))
138-
| (expectedVersion.getMinorVersion() ^ (M[1] & 0xFF));
118+
int mask = (expectedVersion.getMajorVersion() ^ (M[0] & 0xFF))
119+
| (expectedVersion.getMinorVersion() ^ (M[1] & 0xFF));
139120

140-
// 'mask' will be all 1s if the versions matched, or else all 0s.
141-
mask = (mask - 1) >> 31;
121+
// 'mask' will be all 1s if the versions matched, or else all 0s.
122+
mask = (mask - 1) >> 31;
142123

143-
for (int i = 0; i < 48; i++)
144-
{
145-
M[i] = (byte)((M[i] & mask) | (fallback[i] & ~mask));
146-
}
124+
for (int i = 0; i < 48; i++)
125+
{
126+
M[i] = (byte)((M[i] & mask) | (fallback[i] & ~mask));
147127
}
148128

149129
return crypto.createSecret(M);

0 commit comments

Comments
 (0)