winlin

fix #165, refine dh wrapper, ensure public key is 128bytes. 0.9.207.

@@ -31,7 +31,7 @@ CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. @@ -31,7 +31,7 @@ CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
31 // current release version 31 // current release version
32 #define VERSION_MAJOR "0" 32 #define VERSION_MAJOR "0"
33 #define VERSION_MINOR "9" 33 #define VERSION_MINOR "9"
34 -#define VERSION_REVISION "206" 34 +#define VERSION_REVISION "207"
35 #define RTMP_SIG_SRS_VERSION VERSION_MAJOR"."VERSION_MINOR"."VERSION_REVISION 35 #define RTMP_SIG_SRS_VERSION VERSION_MAJOR"."VERSION_MINOR"."VERSION_REVISION
36 // server info. 36 // server info.
37 #define RTMP_SIG_SRS_KEY "SRS" 37 #define RTMP_SIG_SRS_KEY "SRS"
@@ -190,7 +190,7 @@ namespace _srs_internal @@ -190,7 +190,7 @@ namespace _srs_internal
190 return ret; 190 return ret;
191 } 191 }
192 192
193 - int SrsDH::copy_public_key(char* pkey, int32_t* ppkey_size) 193 + int SrsDH::copy_public_key(char* pkey, int32_t& pkey_size)
194 { 194 {
195 int ret = ERROR_SUCCESS; 195 int ret = ERROR_SUCCESS;
196 196
@@ -199,20 +199,21 @@ namespace _srs_internal @@ -199,20 +199,21 @@ namespace _srs_internal
199 int32_t key_size = BN_num_bytes(pdh->pub_key); 199 int32_t key_size = BN_num_bytes(pdh->pub_key);
200 srs_assert(key_size > 0); 200 srs_assert(key_size > 0);
201 201
  202 + // maybe the key_size is 127, but dh will write all 128bytes pkey,
  203 + // so, donot need to set/initialize the pkey.
  204 + // @see https://github.com/winlinvip/simple-rtmp-server/issues/165
202 key_size = BN_bn2bin(pdh->pub_key, (unsigned char*)pkey); 205 key_size = BN_bn2bin(pdh->pub_key, (unsigned char*)pkey);
203 srs_assert(key_size > 0); 206 srs_assert(key_size > 0);
204 207
205 - if (ppkey_size != NULL) {  
206 // output the size of public key. 208 // output the size of public key.
207 // @see https://github.com/winlinvip/simple-rtmp-server/issues/165 209 // @see https://github.com/winlinvip/simple-rtmp-server/issues/165
208 - srs_assert(key_size <= *ppkey_size);  
209 - *ppkey_size = key_size;  
210 - } 210 + srs_assert(key_size <= pkey_size);
  211 + pkey_size = key_size;
211 212
212 return ret; 213 return ret;
213 } 214 }
214 215
215 - int SrsDH::copy_shared_key(const char* ppkey, int32_t ppkey_size, char* skey, int32_t* pskey_size) 216 + int SrsDH::copy_shared_key(const char* ppkey, int32_t ppkey_size, char* skey, int32_t& skey_size)
216 { 217 {
217 int ret = ERROR_SUCCESS; 218 int ret = ERROR_SUCCESS;
218 219
@@ -223,22 +224,19 @@ namespace _srs_internal @@ -223,22 +224,19 @@ namespace _srs_internal
223 } 224 }
224 225
225 // if failed, donot return, do cleanup, @see ./test/dhtest.c:168 226 // if failed, donot return, do cleanup, @see ./test/dhtest.c:168
  227 + // maybe the key_size is 127, but dh will write all 128bytes skey,
  228 + // so, donot need to set/initialize the skey.
  229 + // @see https://github.com/winlinvip/simple-rtmp-server/issues/165
226 int32_t key_size = DH_compute_key((unsigned char*)skey, ppk, pdh); 230 int32_t key_size = DH_compute_key((unsigned char*)skey, ppk, pdh);
227 231
228 if (key_size < ppkey_size) { 232 if (key_size < ppkey_size) {
229 srs_warn("shared key size=%d, ppk_size=%d", key_size, ppkey_size); 233 srs_warn("shared key size=%d, ppk_size=%d", key_size, ppkey_size);
230 } 234 }
231 235
232 - if (key_size < 0) {  
233 - ret = ERROR_OpenSslComputeSharedKey;  
234 - } else {  
235 - if (pskey_size != NULL) {  
236 - if (key_size > *pskey_size) { 236 + if (key_size < 0 || key_size > skey_size) {
237 ret = ERROR_OpenSslComputeSharedKey; 237 ret = ERROR_OpenSslComputeSharedKey;
238 } else { 238 } else {
239 - *pskey_size = key_size;  
240 - }  
241 - } 239 + skey_size = key_size;
242 } 240 }
243 241
244 if (ppk) { 242 if (ppk) {
@@ -936,29 +934,36 @@ namespace _srs_internal @@ -936,29 +934,36 @@ namespace _srs_internal
936 version = 0x01000504; // server s1 version 934 version = 0x01000504; // server s1 version
937 935
938 SrsDH dh; 936 SrsDH dh;
  937 +
  938 + // ensure generate 128bytes public key.
939 if ((ret = dh.initialize(true)) != ERROR_SUCCESS) { 939 if ((ret = dh.initialize(true)) != ERROR_SUCCESS) {
940 return ret; 940 return ret;
941 } 941 }
  942 +
942 if (schema == srs_schema0) { 943 if (schema == srs_schema0) {
943 srs_key_block_init(&block0.key); 944 srs_key_block_init(&block0.key);
944 srs_digest_block_init(&block1.digest); 945 srs_digest_block_init(&block1.digest);
945 946
946 // directly generate the public key. 947 // directly generate the public key.
947 // @see: https://github.com/winlinvip/simple-rtmp-server/issues/148 948 // @see: https://github.com/winlinvip/simple-rtmp-server/issues/148
948 - if ((ret = dh.copy_public_key((char*)block0.key.key, NULL)) != ERROR_SUCCESS) { 949 + int pkey_size = 128;
  950 + if ((ret = dh.copy_public_key((char*)block0.key.key, pkey_size)) != ERROR_SUCCESS) {
949 srs_error("calc s1 key failed. ret=%d", ret); 951 srs_error("calc s1 key failed. ret=%d", ret);
950 return ret; 952 return ret;
951 } 953 }
  954 + srs_assert(pkey_size == 128);
952 } else { 955 } else {
953 srs_digest_block_init(&block0.digest); 956 srs_digest_block_init(&block0.digest);
954 srs_key_block_init(&block1.key); 957 srs_key_block_init(&block1.key);
955 958
956 // directly generate the public key. 959 // directly generate the public key.
957 // @see: https://github.com/winlinvip/simple-rtmp-server/issues/148 960 // @see: https://github.com/winlinvip/simple-rtmp-server/issues/148
958 - if ((ret = dh.copy_public_key((char*)block1.key.key, NULL)) != ERROR_SUCCESS) { 961 + int pkey_size = 128;
  962 + if ((ret = dh.copy_public_key((char*)block1.key.key, pkey_size)) != ERROR_SUCCESS) {
959 srs_error("calc s1 key failed. ret=%d", ret); 963 srs_error("calc s1 key failed. ret=%d", ret);
960 return ret; 964 return ret;
961 } 965 }
  966 + srs_assert(pkey_size == 128);
962 } 967 }
963 srs_verbose("calc s1 key success."); 968 srs_verbose("calc s1 key success.");
964 969
@@ -141,21 +141,21 @@ namespace _srs_internal @@ -141,21 +141,21 @@ namespace _srs_internal
141 /** 141 /**
142 * copy the public key. 142 * copy the public key.
143 * @param pkey the bytes to copy the public key. 143 * @param pkey the bytes to copy the public key.
144 - * @param ppkey_size the max public key size, output the actual public key size.  
145 - * NULL to ignore. 144 + * @param pkey_size the max public key size, output the actual public key size.
  145 + * user should never ignore this size.
146 * @remark, when ensure_128bytes_public_key, the size always 128. 146 * @remark, when ensure_128bytes_public_key, the size always 128.
147 */ 147 */
148 - virtual int copy_public_key(char* pkey, int32_t* ppkey_size); 148 + virtual int copy_public_key(char* pkey, int32_t& pkey_size);
149 /** 149 /**
150 * generate and copy the shared key. 150 * generate and copy the shared key.
151 * generate the shared key with peer public key. 151 * generate the shared key with peer public key.
152 * @param ppkey peer public key. 152 * @param ppkey peer public key.
153 * @param ppkey_size the size of ppkey. 153 * @param ppkey_size the size of ppkey.
154 * @param skey the computed shared key. 154 * @param skey the computed shared key.
155 - * @param pskey_size the max shared key size, output the actual shared key size.  
156 - * NULL to ignore. 155 + * @param skey_size the max shared key size, output the actual shared key size.
  156 + * user should never ignore this size.
157 */ 157 */
158 - virtual int copy_shared_key(const char* ppkey, int32_t ppkey_size, char* skey, int32_t* pskey_size); 158 + virtual int copy_shared_key(const char* ppkey, int32_t ppkey_size, char* skey, int32_t& skey_size);
159 private: 159 private:
160 virtual int do_initialize(); 160 virtual int do_initialize();
161 }; 161 };
@@ -243,10 +243,13 @@ VOID TEST(ProtocolHandshakeTest, DHKey) @@ -243,10 +243,13 @@ VOID TEST(ProtocolHandshakeTest, DHKey)
243 ASSERT_TRUE(ERROR_SUCCESS == dh.initialize(true)); 243 ASSERT_TRUE(ERROR_SUCCESS == dh.initialize(true));
244 244
245 char pub_key1[128]; 245 char pub_key1[128];
246 - EXPECT_TRUE(ERROR_SUCCESS == dh.copy_public_key(pub_key1, NULL)); 246 + int pkey_size = 128;
  247 + EXPECT_TRUE(ERROR_SUCCESS == dh.copy_public_key(pub_key1, pkey_size));
  248 + ASSERT_EQ(128, pkey_size);
247 249
248 char pub_key2[128]; 250 char pub_key2[128];
249 - EXPECT_TRUE(ERROR_SUCCESS == dh.copy_public_key(pub_key2, NULL)); 251 + EXPECT_TRUE(ERROR_SUCCESS == dh.copy_public_key(pub_key2, pkey_size));
  252 + ASSERT_EQ(128, pkey_size);
250 253
251 EXPECT_TRUE(srs_bytes_equals(pub_key1, pub_key2, 128)); 254 EXPECT_TRUE(srs_bytes_equals(pub_key1, pub_key2, 128));
252 255
@@ -255,7 +258,8 @@ VOID TEST(ProtocolHandshakeTest, DHKey) @@ -255,7 +258,8 @@ VOID TEST(ProtocolHandshakeTest, DHKey)
255 258
256 ASSERT_TRUE(ERROR_SUCCESS == dh0.initialize(true)); 259 ASSERT_TRUE(ERROR_SUCCESS == dh0.initialize(true));
257 260
258 - EXPECT_TRUE(ERROR_SUCCESS == dh0.copy_public_key(pub_key2, NULL)); 261 + EXPECT_TRUE(ERROR_SUCCESS == dh0.copy_public_key(pub_key2, pkey_size));
  262 + ASSERT_EQ(128, pkey_size);
259 263
260 EXPECT_FALSE(srs_bytes_equals(pub_key1, pub_key2, 128)); 264 EXPECT_FALSE(srs_bytes_equals(pub_key1, pub_key2, 128));
261 } 265 }