Skip to content

Commit 1d64c68

Browse files
rmw42rmw42
committed
Code review fixes
Replace strcmp/memcmp with XSTRCMP/XMEMCMP for check-source Fix for check-defines Move ssh_test.c Remove #ifdef LTC_TEST XSTRCMP/XMEMCMP != 0
1 parent 760a4cd commit 1d64c68

File tree

9 files changed

+62
-57
lines changed

9 files changed

+62
-57
lines changed

‎src/headers/tomcrypt_pk.h‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -324,10 +324,6 @@ int ecc_recover_key(const unsigned char *sig, unsigned long siglen,
324324
const unsigned char *hash, unsigned long hashlen,
325325
int recid, ecc_signature_type sigformat, ecc_key *key);
326326

327-
#ifdef LTC_SSH
328-
int ecc_ssh_ecdsa_encode_name(char *buffer, unsigned long buflen, const ecc_key *key);
329-
#endif
330-
331327
#endif
332328

333329
#ifdef LTC_MDSA

‎src/headers/tomcrypt_private.h‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -191,6 +191,10 @@ int ecc_copy_curve(const ecc_key *srckey, ecc_key *key);
191191
int ecc_set_curve_by_size(int size, ecc_key *key);
192192
int ecc_import_subject_public_key_info(const unsigned char *in, unsigned long inlen, ecc_key *key);
193193

194+
#ifdef LTC_SSH
195+
int ecc_ssh_ecdsa_encode_name(char *buffer, unsigned long *buflen, const ecc_key *key);
196+
#endif
197+
194198
/* low level functions */
195199
ecc_point *ltc_ecc_new_point(void);
196200
void ltc_ecc_del_point(ecc_point *p);

‎src/misc/crypt/crypt.c‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -445,6 +445,9 @@ const char *crypt_build_settings =
445445
#if defined(LTC_HKDF)
446446
" HKDF "
447447
#endif
448+
#if defined(LTC_SSH)
449+
" SSH "
450+
#endif
448451
#if defined(LTC_DEVRANDOM)
449452
" LTC_DEVRANDOM "
450453
#endif

‎src/pk/ecc/ecc_cmp_oid_str.c‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -19,16 +19,18 @@
1919

2020
int ecc_cmp_oid_str(const char *oidstr, const ecc_key *key)
2121
{
22-
char oid[32];
23-
unsigned long oidlen = 32;
22+
char oid[64];
23+
unsigned long oidlen = sizeof(oid);
2424
int err;
2525

2626
LTC_ARGCHK(key != NULL);
2727

28-
err = pk_oid_num_to_str(key->dp.oid, key->dp.oidlen, oid, &oidlen);
29-
if (err != CRYPT_OK) return err;
30-
else if (!strcmp(oidstr, oid)) return CRYPT_OK;
31-
else return CRYPT_INVALID_ARG;
28+
if ((err = pk_oid_num_to_str(key->dp.oid, key->dp.oidlen, oid, &oidlen)) != CRYPT_OK) { goto error; }
29+
if (XSTRCMP(oidstr, oid) != 0) { err = CRYPT_INVALID_ARG; goto error; }
30+
31+
err = CRYPT_OK;
32+
error:
33+
return err;
3234
}
3335

3436
#endif

‎src/pk/ecc/ecc_recover_key.c‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,7 @@ int ecc_recover_key(const unsigned char *sig, unsigned long siglen,
114114
#ifdef LTC_SSH
115115
else if (sigformat == LTC_ECCSIG_RFC5656) {
116116
char name[64], name2[64];
117+
unsigned long namelen = sizeof(name2);
117118

118119
/* Decode as SSH data sequence, per RFC4251 */
119120
if ((err = ssh_decode_sequence_multi(sig, siglen,
@@ -124,10 +125,10 @@ int ecc_recover_key(const unsigned char *sig, unsigned long siglen,
124125

125126

126127
/* Check curve matches identifier string */
127-
if ((err = ecc_ssh_ecdsa_encode_name(name2, 64, key)) != CRYPT_OK) { goto error; }
128-
if (strcmp(name,name2)) {
128+
if ((err = ecc_ssh_ecdsa_encode_name(name2, &namelen, key)) != CRYPT_OK) { goto error; }
129+
if (XSTRCMP(name,name2) != 0) {
129130
err = CRYPT_INVALID_ARG;
130-
goto error;
131+
goto error;
131132
}
132133
}
133134
#endif

‎src/pk/ecc/ecc_sign_hash.c‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -159,7 +159,8 @@ int ecc_sign_hash_ex(const unsigned char *in, unsigned long inlen,
159159
else if (sigformat == LTC_ECCSIG_RFC5656) {
160160
/* Get identifier string */
161161
char name[64];
162-
if ((err = ecc_ssh_ecdsa_encode_name(name, 64, key)) != CRYPT_OK) { goto errnokey; }
162+
unsigned long namelen = sizeof(name);
163+
if ((err = ecc_ssh_ecdsa_encode_name(name, &namelen, key)) != CRYPT_OK) { goto errnokey; }
163164

164165
/* Store as SSH data sequence, per RFC4251 */
165166
err = ssh_encode_sequence_multi(out, outlen,

‎src/pk/ecc/ecc_ssh_ecdsa_encode_name.c‎

Lines changed: 23 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -14,37 +14,40 @@
1414
Russ Williams
1515
*/
1616

17-
int ecc_ssh_ecdsa_encode_name(char *buffer, unsigned long buflen, const ecc_key *key)
17+
int ecc_ssh_ecdsa_encode_name(char *buffer, unsigned long *buflen, const ecc_key *key)
1818
{
19+
char oidstr[64];
20+
unsigned long oidlen = sizeof(oidstr);
21+
unsigned long size;
22+
const char *pattern = "ecdsa-sha2-%s";
1923
int err;
2024

25+
LTC_ARGCHK(buffer != NULL);
26+
LTC_ARGCHK(buflen != NULL);
27+
LTC_ARGCHK(key != NULL);
28+
29+
/* Get the OID of the curve */
30+
if ((err = ecc_get_oid_str(oidstr, &oidlen, key)) != CRYPT_OK) goto error;
31+
2132
/* Check for three named curves: nistp256, nistp384, nistp521 */
22-
if (ecc_cmp_oid_str("1.2.840.10045.3.1.7", key) == CRYPT_OK) {
33+
if (XSTRCMP("1.2.840.10045.3.1.7", oidstr) == 0) {
2334
/* nistp256 - secp256r1 - OID 1.2.840.10045.3.1.7 */
24-
25-
if (buflen <= 19) { err = CRYPT_BUFFER_OVERFLOW; goto error; }
26-
strcpy(buffer, "ecdsa-sha2-nistp256");
35+
pattern = "ecdsa-sha2-nistp256";
2736
}
28-
else if (ecc_cmp_oid_str("1.3.132.0.34", key) == CRYPT_OK) {
37+
else if (XSTRCMP("1.3.132.0.34", oidstr) == 0) {
2938
/* nistp384 - secp384r1 - OID 1.3.132.0.34 */
30-
31-
if (buflen <= 19) { err = CRYPT_BUFFER_OVERFLOW; goto error; }
32-
strcpy(buffer, "ecdsa-sha2-nistp384");
39+
pattern = "ecdsa-sha2-nistp384";
3340
}
34-
else if (ecc_cmp_oid_str("1.3.132.0.35", key) == CRYPT_OK) {
41+
else if (XSTRCMP("1.3.132.0.35", oidstr) == 0) {
3542
/* nistp521 - secp521r1 - OID 1.3.132.0.35 */
36-
37-
if (buflen <= 19) { err = CRYPT_BUFFER_OVERFLOW; goto error; }
38-
strcpy(buffer, "ecdsa-sha2-nistp512");
43+
pattern = "ecdsa-sha2-nistp521";
3944
}
40-
else { /* Otherwise, we use the OID of the curve */
41-
char oidstr[32];
42-
unsigned long oidlen = 32;
43-
if ((err = ecc_get_oid_str(oidstr, &oidlen, key)) != CRYPT_OK) goto error;
4445

45-
if (buflen <= strlen(oidstr)+11) { err = CRYPT_BUFFER_OVERFLOW; goto error; }
46-
sprintf(buffer, "ecdsa-sha2-%s", oidstr);
47-
}
46+
/* pattern is either a constant string, or one with a hole for an OID... */
47+
size = snprintf(buffer, *buflen, pattern, oidstr);
48+
/* snprintf returns size that would have been written, but limits to buflen-1 chars plus terminator */
49+
if (size >= *buflen) { err = CRYPT_BUFFER_OVERFLOW; goto error; }
50+
*buflen = size;
4851

4952
err = CRYPT_OK;
5053
error:

‎src/pk/ecc/ecc_verify_hash.c‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,7 @@ int ecc_verify_hash_ex(const unsigned char *sig, unsigned long siglen,
100100
#ifdef LTC_SSH
101101
else if (sigformat == LTC_ECCSIG_RFC5656) {
102102
char name[64], name2[64];
103+
unsigned long namelen = sizeof(name2);
103104

104105
/* Decode as SSH data sequence, per RFC4251 */
105106
if ((err = ssh_decode_sequence_multi(sig, siglen,
@@ -110,8 +111,8 @@ int ecc_verify_hash_ex(const unsigned char *sig, unsigned long siglen,
110111

111112

112113
/* Check curve matches identifier string */
113-
if ((err = ecc_ssh_ecdsa_encode_name(name2, 64, key)) != CRYPT_OK) { goto error; }
114-
if (strcmp(name,name2)) {
114+
if ((err = ecc_ssh_ecdsa_encode_name(name2, &namelen, key)) != CRYPT_OK) { goto error; }
115+
if (XSTRCMP(name,name2) != 0) {
115116
err = CRYPT_INVALID_ARG;
116117
goto error;
117118
}

src/misc/ssh/ssh_test.c renamed to tests/ssh_test.c

Lines changed: 15 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,7 @@
66
* The library is free for all purposes without any express
77
* guarantee it works.
88
*/
9-
#include "tomcrypt_private.h"
10-
9+
#include "tomcrypt_test.h"
1110

1211
/**
1312
@file ssh_test.c
@@ -16,7 +15,6 @@
1615

1716
#ifdef LTC_SSH
1817

19-
#define DO(x) do { int __err = (x); if (__err != CRYPT_OK) return __err; } while (0)
2018
#define BUFSIZE 64
2119

2220
/**
@@ -56,7 +54,7 @@ static const unsigned char nlist3[] = {0x00, 0x00, 0x00, 0x09, 0x7a, 0x6c, 0x69,
5654
LTC_SSH encoding test
5755
@return CRYPT_OK if successful
5856
*/
59-
int _ssh_encoding_test(void)
57+
static int _ssh_encoding_test(void)
6058
{
6159
unsigned char buffer[BUFSIZE];
6260
unsigned long buflen;
@@ -78,7 +76,7 @@ int _ssh_encoding_test(void)
7876
DO(ssh_encode_sequence_multi(buffer, &buflen,
7977
LTC_SSHDATA_UINT32, 0x29b7f4aa,
8078
LTC_SSHDATA_EOL, NULL));
81-
if (memcmp(buffer, uint32, buflen)) return CRYPT_FAIL_TESTVECTOR;
79+
if (XMEMCMP(buffer, uint32, buflen) != 0) return CRYPT_FAIL_TESTVECTOR;
8280

8381

8482
/* string */
@@ -87,7 +85,7 @@ int _ssh_encoding_test(void)
8785
DO(ssh_encode_sequence_multi(buffer, &buflen,
8886
LTC_SSHDATA_STRING, "testing",
8987
LTC_SSHDATA_EOL, NULL));
90-
if (memcmp(buffer, string, buflen)) return CRYPT_FAIL_TESTVECTOR;
88+
if (XMEMCMP(buffer, string, buflen) != 0) return CRYPT_FAIL_TESTVECTOR;
9189

9290

9391
/* mpint */
@@ -101,23 +99,23 @@ int _ssh_encoding_test(void)
10199
DO(ssh_encode_sequence_multi(buffer, &buflen,
102100
LTC_SSHDATA_MPINT, zero,
103101
LTC_SSHDATA_EOL, NULL));
104-
if (memcmp(buffer, mpint1, buflen)) return CRYPT_FAIL_TESTVECTOR;
102+
if (XMEMCMP(buffer, mpint1, buflen) != 0) return CRYPT_FAIL_TESTVECTOR;
105103

106104
buflen = BUFSIZE;
107105
zeromem(buffer, BUFSIZE);
108106
DO(mp_read_radix(v, "9a378f9b2e332a7", 16));
109107
DO(ssh_encode_sequence_multi(buffer, &buflen,
110108
LTC_SSHDATA_MPINT, v,
111109
LTC_SSHDATA_EOL, NULL));
112-
if (memcmp(buffer, mpint2, buflen)) return CRYPT_FAIL_TESTVECTOR;
110+
if (XMEMCMP(buffer, mpint2, buflen) != 0) return CRYPT_FAIL_TESTVECTOR;
113111

114112
buflen = BUFSIZE;
115113
zeromem(buffer, BUFSIZE);
116114
DO(mp_set(v, 0x80));
117115
DO(ssh_encode_sequence_multi(buffer, &buflen,
118116
LTC_SSHDATA_MPINT, v,
119117
LTC_SSHDATA_EOL, NULL));
120-
if (memcmp(buffer, mpint3, buflen)) return CRYPT_FAIL_TESTVECTOR;
118+
if (XMEMCMP(buffer, mpint3, buflen) != 0) return CRYPT_FAIL_TESTVECTOR;
121119

122120
mp_clear_multi(v, zero, NULL);
123121

@@ -128,21 +126,21 @@ int _ssh_encoding_test(void)
128126
DO(ssh_encode_sequence_multi(buffer, &buflen,
129127
LTC_SSHDATA_NAMELIST, "",
130128
LTC_SSHDATA_EOL, NULL));
131-
if (memcmp(buffer, nlist1, buflen)) return CRYPT_FAIL_TESTVECTOR;
129+
if (XMEMCMP(buffer, nlist1, buflen) != 0) return CRYPT_FAIL_TESTVECTOR;
132130

133131
buflen = BUFSIZE;
134132
zeromem(buffer, BUFSIZE);
135133
DO(ssh_encode_sequence_multi(buffer, &buflen,
136134
LTC_SSHDATA_NAMELIST, "zlib",
137135
LTC_SSHDATA_EOL, NULL));
138-
if (memcmp(buffer, nlist2, buflen)) return CRYPT_FAIL_TESTVECTOR;
136+
if (XMEMCMP(buffer, nlist2, buflen) != 0) return CRYPT_FAIL_TESTVECTOR;
139137

140138
buflen = BUFSIZE;
141139
zeromem(buffer, BUFSIZE);
142140
DO(ssh_encode_sequence_multi(buffer, &buflen,
143141
LTC_SSHDATA_NAMELIST, "zlib,none",
144142
LTC_SSHDATA_EOL, NULL));
145-
if (memcmp(buffer, nlist3, buflen)) return CRYPT_FAIL_TESTVECTOR;
143+
if (XMEMCMP(buffer, nlist3, buflen) != 0) return CRYPT_FAIL_TESTVECTOR;
146144

147145
return CRYPT_OK;
148146
}
@@ -151,7 +149,7 @@ int _ssh_encoding_test(void)
151149
LTC_SSH decoding test
152150
@return CRYPT_OK if successful
153151
*/
154-
int _ssh_decoding_test(void)
152+
static int _ssh_decoding_test(void)
155153
{
156154
char strbuf[BUFSIZE];
157155
void *u, *v;
@@ -169,7 +167,7 @@ int _ssh_decoding_test(void)
169167
DO(ssh_decode_sequence_multi(string, sizeof(string),
170168
LTC_SSHDATA_STRING, strbuf, BUFSIZE,
171169
LTC_SSHDATA_EOL, NULL));
172-
if (strcmp(strbuf, "testing")) return CRYPT_FAIL_TESTVECTOR;
170+
if (XSTRCMP(strbuf, "testing") != 0) return CRYPT_FAIL_TESTVECTOR;
173171

174172
/* mpint */
175173
if ((err = mp_init_multi(&u, &v, NULL)) != CRYPT_OK) {
@@ -199,19 +197,19 @@ int _ssh_decoding_test(void)
199197
DO(ssh_decode_sequence_multi(nlist1, sizeof(string),
200198
LTC_SSHDATA_NAMELIST, strbuf, BUFSIZE,
201199
LTC_SSHDATA_EOL, NULL));
202-
if (strcmp(strbuf, "")) return CRYPT_FAIL_TESTVECTOR;
200+
if (XSTRCMP(strbuf, "") != 0) return CRYPT_FAIL_TESTVECTOR;
203201

204202
zeromem(strbuf, BUFSIZE);
205203
DO(ssh_decode_sequence_multi(nlist2, sizeof(string),
206204
LTC_SSHDATA_NAMELIST, strbuf, BUFSIZE,
207205
LTC_SSHDATA_EOL, NULL));
208-
if (strcmp(strbuf, "zlib")) return CRYPT_FAIL_TESTVECTOR;
206+
if (XSTRCMP(strbuf, "zlib") != 0) return CRYPT_FAIL_TESTVECTOR;
209207

210208
zeromem(strbuf, BUFSIZE);
211209
DO(ssh_decode_sequence_multi(nlist3, sizeof(string),
212210
LTC_SSHDATA_NAMELIST, strbuf, BUFSIZE,
213211
LTC_SSHDATA_EOL, NULL));
214-
if (strcmp(strbuf, "zlib,none")) return CRYPT_FAIL_TESTVECTOR;
212+
if (XSTRCMP(strbuf, "zlib,none") != 0) return CRYPT_FAIL_TESTVECTOR;
215213

216214

217215
return CRYPT_OK;
@@ -223,14 +221,10 @@ int _ssh_decoding_test(void)
223221
*/
224222
int ssh_test(void)
225223
{
226-
#ifndef LTC_TEST
227-
return CRYPT_NOP;
228-
#else
229224
DO(_ssh_encoding_test());
230225
DO(_ssh_decoding_test());
231226

232227
return CRYPT_OK;
233-
#endif
234228
}
235229

236230

0 commit comments

Comments
 (0)