Skip to content

Commit fcdb14e

Browse files
sjaeckelsjaeckel
authored
Merge pull request #500 from libtom/fix-ssh-api
Fix SSH API
2 parents 5ded083 + 9b6bf32 commit fcdb14e

File tree

10 files changed

+219
-84
lines changed

10 files changed

+219
-84
lines changed

‎doc/crypt.tex‎

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7233,6 +7233,73 @@ \subsection{HKDF Extract-and-Expand}
72337233

72347234
Parameters are as in \textit{hkdf\_extract()} and \textit{hkdf\_expand()}.
72357235

7236+
7237+
\mysection{SSH}
7238+
7239+
The library provides functions to encode and decode SSH data as specified in RFC4251 Ch. 5.
7240+
7241+
\subsection{Data types}
7242+
7243+
The following enum is used to indicate a specific SSH data type
7244+
(besides EOL which is an internal one that indicates the end of a sequence).
7245+
7246+
\begin{figure}[h]
7247+
\begin{center}
7248+
\begin{small}
7249+
\begin{tabular}{|l|l|l|}
7250+
\hline \textbf{Definition} & \textbf{arg data Type} & \textbf{SSH Type} \\
7251+
\hline LTC\_SSHDATA\_EOL & - & End of SSH data sequence. \\
7252+
\hline LTC\_SSHDATA\_BYTE & \texttt{unsigned char} & \texttt{byte} type \\
7253+
\hline LTC\_SSHDATA\_BOOLEAN & \texttt{unsigned char} & \texttt{boolean} type \\
7254+
\hline LTC\_SSHDATA\_UINT32 & \texttt{ulong32} & \texttt{uint32} \\
7255+
\hline LTC\_SSHDATA\_UINT64 & \texttt{ulong64} & \texttt{uint64} \\
7256+
\hline LTC\_SSHDATA\_STRING & \texttt{char*} & \texttt{string} (one octet per char) \\
7257+
\hline LTC\_SSHDATA\_MPINT & \texttt{mp\_int} & \texttt{mpint} \\
7258+
\hline LTC\_SSHDATA\_NAMELIST & \texttt{char*} & \texttt{name-list} (which works exactly like a \texttt{string}) \\
7259+
\hline
7260+
\end{tabular}
7261+
\caption{List of SSH Supported Types}
7262+
\index{ssh\_data\_type}
7263+
\end{small}
7264+
\end{center}
7265+
\end{figure}
7266+
7267+
\subsection{De- and Encoding with Multiple Argument Lists}
7268+
7269+
\index{ssh\_encode\_sequence\_multi()}
7270+
\index{ssh\_decode\_sequence\_multi()}
7271+
7272+
7273+
The API works similar to the ASN.1 SEQUENCE multi en- and decoders.
7274+
7275+
They either encode or decode a sequence of the supported SSH types where the items are specified after the length parameter.
7276+
7277+
7278+
\begin{verbatim}
7279+
int ssh_encode_sequence_multi(unsigned char *out, unsigned long *outlen, ...);
7280+
\end{verbatim}
7281+
7282+
Where \texttt{out} points to the destination buffer and \texttt{outlen} points
7283+
on function invocation to the length of the destination buffer
7284+
and after returning it will be filled with the number of octets written to the buffer.
7285+
7286+
The encoding function \texttt{ssh\_encode\_sequence\_multi()} expects its items to be a pair of \texttt{(type, data)},
7287+
except for the \texttt{string} resp. \texttt{name-list} type, which expects the triple \texttt{(type, data, size)}
7288+
with \texttt{size} being of type \texttt{unsigned long}.
7289+
7290+
7291+
\begin{verbatim}
7292+
int ssh_decode_sequence_multi(const unsigned char *in, unsigned long *inlen, ...);
7293+
\end{verbatim}
7294+
7295+
Where \texttt{in} points to the buffer with the sequence to decode and \texttt{inlen} points
7296+
on function invocation to the length of the sequence
7297+
and after returning it will be filled with the decoded number of octets.
7298+
7299+
The decoding function \texttt{ssh\_decode\_sequence\_multi()} expects its items to be a pair of \texttt{(type, data*)},
7300+
except for the \texttt{string} resp. \texttt{name-list} type, which expects the triple \texttt{(type, data, size*)}
7301+
with \texttt{size*} being of type \texttt{unsigned long*}.
7302+
72367303
\chapter{Miscellaneous}
72377304
\mysection{Base64 Encoding and Decoding}
72387305
The library provides functions to encode and decode a RFC 4648 Base64 coding scheme.

‎src/headers/tomcrypt_misc.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -163,19 +163,19 @@ int padding_depad(const unsigned char *data, unsigned long *length, unsigned lon
163163

164164
#ifdef LTC_SSH
165165
typedef enum ssh_data_type_ {
166+
LTC_SSHDATA_EOL,
166167
LTC_SSHDATA_BYTE,
167168
LTC_SSHDATA_BOOLEAN,
168169
LTC_SSHDATA_UINT32,
169170
LTC_SSHDATA_UINT64,
170171
LTC_SSHDATA_STRING,
171172
LTC_SSHDATA_MPINT,
172173
LTC_SSHDATA_NAMELIST,
173-
LTC_SSHDATA_EOL
174174
} ssh_data_type;
175175

176176
/* VA list handy helpers with tuples of <type, data> */
177177
int ssh_encode_sequence_multi(unsigned char *out, unsigned long *outlen, ...);
178-
int ssh_decode_sequence_multi(const unsigned char *in, unsigned long inlen, ...);
178+
int ssh_decode_sequence_multi(const unsigned char *in, unsigned long *inlen, ...);
179179
#endif /* LTC_SSH */
180180

181181
int compare_testvector(const void* is, const unsigned long is_len, const void* should, const unsigned long should_len, const char* what, int which);

‎src/misc/ssh/ssh_decode_sequence_multi.c‎

Lines changed: 35 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -18,12 +18,12 @@
1818

1919
/**
2020
Decode a SSH sequence using a VA list
21-
@param in Data to decode
22-
@param inlen Length of buffer to decode
23-
@remark <...> is of the form <type, data> (int, void*) except for string <type, data, size>
21+
@param in The input buffer
22+
@param inlen [in/out] The length of the input buffer and on output the amount of decoded data
23+
@remark <...> is of the form <type, data*> (int, <unsigned char*,ulong32*,ulong64*>) except for string&name-list <type, data, size*> (int, void*, unsigned long*)
2424
@return CRYPT_OK on success
2525
*/
26-
int ssh_decode_sequence_multi(const unsigned char *in, unsigned long inlen, ...)
26+
int ssh_decode_sequence_multi(const unsigned char *in, unsigned long *inlen, ...)
2727
{
2828
int err;
2929
va_list args;
@@ -33,11 +33,14 @@ int ssh_decode_sequence_multi(const unsigned char *in, unsigned long inlen, ...)
3333
char *sdata;
3434
ulong32 *u32data;
3535
ulong64 *u64data;
36-
unsigned long bufsize;
36+
unsigned long *bufsize;
3737
ulong32 size;
38+
unsigned long remaining;
3839

3940
LTC_ARGCHK(in != NULL);
41+
LTC_ARGCHK(inlen != NULL);
4042

43+
remaining = *inlen;
4144
/* Decode values from buffer */
4245
va_start(args, inlen);
4346
while ((type = (ssh_data_type)va_arg(args, int)) != LTC_SSHDATA_EOL) {
@@ -47,7 +50,7 @@ int ssh_decode_sequence_multi(const unsigned char *in, unsigned long inlen, ...)
4750
type == LTC_SSHDATA_MPINT)
4851
{
4952
/* Check we'll not read too far */
50-
if (inlen < 4) {
53+
if (remaining < 4) {
5154
err = CRYPT_BUFFER_OVERFLOW;
5255
goto error;
5356
}
@@ -71,7 +74,7 @@ int ssh_decode_sequence_multi(const unsigned char *in, unsigned long inlen, ...)
7174
case LTC_SSHDATA_MPINT:
7275
LOAD32H(size, in);
7376
in += 4;
74-
inlen -= 4;
77+
remaining -= 4;
7578
break;
7679

7780
case LTC_SSHDATA_EOL:
@@ -81,55 +84,63 @@ int ssh_decode_sequence_multi(const unsigned char *in, unsigned long inlen, ...)
8184
}
8285

8386
/* Check we'll not read too far */
84-
if (inlen < size) {
87+
if (remaining < size) {
8588
err = CRYPT_BUFFER_OVERFLOW;
8689
goto error;
8790
} else {
88-
inlen -= size;
91+
remaining -= size;
92+
}
93+
94+
vdata = va_arg(args, void*);
95+
if (vdata == NULL) {
96+
err = CRYPT_INVALID_ARG;
97+
goto error;
8998
}
9099

91100
/* Read data */
92101
switch (type) {
93102
case LTC_SSHDATA_BYTE:
94-
cdata = va_arg(args, unsigned char*);
103+
cdata = vdata;
95104
*cdata = *in++;
96105
break;
97106
case LTC_SSHDATA_BOOLEAN:
98-
cdata = va_arg(args, unsigned char*);
107+
cdata = vdata;
99108
/*
100109
The value 0 represents FALSE, and the value 1 represents TRUE. All non-zero values MUST be
101110
interpreted as TRUE; however, applications MUST NOT store values other than 0 and 1.
102-
*/
111+
*/
103112
*cdata = (*in++)?1:0;
104113
break;
105114
case LTC_SSHDATA_UINT32:
106-
u32data = va_arg(args, ulong32*);
115+
u32data = vdata;
107116
LOAD32H(*u32data, in);
108117
in += 4;
109118
break;
110119
case LTC_SSHDATA_UINT64:
111-
u64data = va_arg(args, ulong64*);
120+
u64data = vdata;
112121
LOAD64H(*u64data, in);
113122
in += 8;
114123
break;
115124
case LTC_SSHDATA_STRING:
116125
case LTC_SSHDATA_NAMELIST:
117-
sdata = va_arg(args, char*);
118-
bufsize = va_arg(args, unsigned long);
119-
if (size >= bufsize) {
126+
sdata = vdata;
127+
bufsize = va_arg(args, unsigned long*);
128+
if (bufsize == NULL) {
129+
err = CRYPT_INVALID_ARG;
130+
goto error;
131+
}
132+
if (size + 1 >= *bufsize) {
120133
err = CRYPT_BUFFER_OVERFLOW;
121134
goto error;
122135
}
123136
if (size > 0) {
124-
XSTRNCPY(sdata, (const char *)in, size);
125-
sdata[size] = '\0'; /* strncpy doesn't NUL-terminate */
126-
} else {
127-
*sdata = '\0';
137+
XMEMCPY(sdata, (const char *)in, size);
128138
}
139+
sdata[size] = '\0';
140+
*bufsize = size;
129141
in += size;
130142
break;
131143
case LTC_SSHDATA_MPINT:
132-
vdata = va_arg(args, void*);
133144
if (size == 0) {
134145
if ((err = mp_set(vdata, 0)) != CRYPT_OK) { goto error; }
135146
} else if ((in[0] & 0x80) != 0) {
@@ -150,6 +161,8 @@ int ssh_decode_sequence_multi(const unsigned char *in, unsigned long inlen, ...)
150161
}
151162
err = CRYPT_OK;
152163

164+
*inlen -= remaining;
165+
153166
error:
154167
va_end(args);
155168
return err;

‎src/misc/ssh/ssh_encode_sequence_multi.c‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@
2020
Encode a SSH sequence using a VA list
2121
@param out [out] Destination for data
2222
@param outlen [in/out] Length of buffer and resulting length of output
23-
@remark <...> is of the form <type, data> (int, void*)
23+
@remark <...> is of the form <type, data> (int, <int,ulong32,ulong64>) except for string&name-list <type, data, size> (int, void*, unsigned long)
2424
@return CRYPT_OK on success
2525
*/
2626
int ssh_encode_sequence_multi(unsigned char *out, unsigned long *outlen, ...)
@@ -29,8 +29,8 @@ int ssh_encode_sequence_multi(unsigned char *out, unsigned long *outlen, ...)
2929
va_list args;
3030
ulong32 size;
3131
ssh_data_type type;
32-
void *vdata;
33-
const char *sdata;
32+
void *vdata;
33+
const char *sdata;
3434
int idata;
3535
ulong32 u32data;
3636
ulong64 u64data;
@@ -58,9 +58,9 @@ int ssh_encode_sequence_multi(unsigned char *out, unsigned long *outlen, ...)
5858
break;
5959
case LTC_SSHDATA_STRING:
6060
case LTC_SSHDATA_NAMELIST:
61-
sdata = va_arg(args, char*);
61+
LTC_UNUSED_PARAM( va_arg(args, char*) );
62+
size += va_arg(args, unsigned long);
6263
size += 4;
63-
size += strlen(sdata);
6464
break;
6565
case LTC_SSHDATA_MPINT:
6666
vdata = va_arg(args, void*);
@@ -102,7 +102,7 @@ int ssh_encode_sequence_multi(unsigned char *out, unsigned long *outlen, ...)
102102
/*
103103
The value 0 represents FALSE, and the value 1 represents TRUE. All non-zero values MUST be
104104
interpreted as TRUE; however, applications MUST NOT store values other than 0 and 1.
105-
*/
105+
*/
106106
*out++ = (idata)?1:0;
107107
break;
108108
case LTC_SSHDATA_UINT32:
@@ -118,7 +118,7 @@ int ssh_encode_sequence_multi(unsigned char *out, unsigned long *outlen, ...)
118118
case LTC_SSHDATA_STRING:
119119
case LTC_SSHDATA_NAMELIST:
120120
sdata = va_arg(args, char*);
121-
size = strlen(sdata);
121+
size = va_arg(args, unsigned long);
122122
STORE32H(size, out);
123123
out += 4;
124124
XMEMCPY(out, sdata, size);

‎src/pk/ecc/ecc_recover_key.c‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -114,19 +114,20 @@ 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);
117+
unsigned long namelen = sizeof(name);
118+
unsigned long name2len = sizeof(name2);
118119

119120
/* Decode as SSH data sequence, per RFC4251 */
120-
if ((err = ssh_decode_sequence_multi(sig, siglen,
121-
LTC_SSHDATA_STRING, name, 64,
121+
if ((err = ssh_decode_sequence_multi(sig, &siglen,
122+
LTC_SSHDATA_STRING, name, &namelen,
122123
LTC_SSHDATA_MPINT, r,
123124
LTC_SSHDATA_MPINT, s,
124125
LTC_SSHDATA_EOL, NULL)) != CRYPT_OK) { goto error; }
125126

126127

127128
/* Check curve matches identifier string */
128-
if ((err = ecc_ssh_ecdsa_encode_name(name2, &namelen, key)) != CRYPT_OK) { goto error; }
129-
if (XSTRCMP(name,name2) != 0) {
129+
if ((err = ecc_ssh_ecdsa_encode_name(name2, &name2len, key)) != CRYPT_OK) { goto error; }
130+
if ((namelen != name2len) || (XSTRCMP(name, name2) != 0)) {
130131
err = CRYPT_INVALID_ARG;
131132
goto error;
132133
}

‎src/pk/ecc/ecc_sign_hash.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -164,7 +164,7 @@ int ecc_sign_hash_ex(const unsigned char *in, unsigned long inlen,
164164

165165
/* Store as SSH data sequence, per RFC4251 */
166166
err = ssh_encode_sequence_multi(out, outlen,
167-
LTC_SSHDATA_STRING, name,
167+
LTC_SSHDATA_STRING, name, namelen,
168168
LTC_SSHDATA_MPINT, r,
169169
LTC_SSHDATA_MPINT, s,
170170
LTC_SSHDATA_EOL, NULL);

‎src/pk/ecc/ecc_ssh_ecdsa_encode_name.c‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,7 @@ int ecc_ssh_ecdsa_encode_name(char *buffer, unsigned long *buflen, const ecc_key
2525
{
2626
char oidstr[64];
2727
unsigned long oidlen = sizeof(oidstr);
28-
unsigned long size = 0;
29-
int err;
28+
int err, size = 0;
3029

3130
LTC_ARGCHK(buffer != NULL);
3231
LTC_ARGCHK(buflen != NULL);
@@ -52,8 +51,11 @@ int ecc_ssh_ecdsa_encode_name(char *buffer, unsigned long *buflen, const ecc_key
5251
size = snprintf(buffer, *buflen, "ecdsa-sha2-%s", oidstr);
5352
}
5453

55-
/* snprintf returns size that would have been written, but limits to buflen-1 chars plus terminator */
56-
if (size >= *buflen) {
54+
/* snprintf returns a negative value on error
55+
* or the size that would have been written, but limits to buflen-1 chars plus terminator */
56+
if (size < 0) {
57+
err = CRYPT_ERROR;
58+
} else if ((unsigned)size >= *buflen) {
5759
err = CRYPT_BUFFER_OVERFLOW;
5860
} else {
5961
err = CRYPT_OK;

‎src/pk/ecc/ecc_verify_hash.c‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -100,19 +100,20 @@ 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);
103+
unsigned long namelen = sizeof(name);
104+
unsigned long name2len = sizeof(name2);
104105

105106
/* Decode as SSH data sequence, per RFC4251 */
106-
if ((err = ssh_decode_sequence_multi(sig, siglen,
107-
LTC_SSHDATA_STRING, name, 64,
107+
if ((err = ssh_decode_sequence_multi(sig, &siglen,
108+
LTC_SSHDATA_STRING, name, &namelen,
108109
LTC_SSHDATA_MPINT, r,
109110
LTC_SSHDATA_MPINT, s,
110111
LTC_SSHDATA_EOL, NULL)) != CRYPT_OK) { goto error; }
111112

112113

113114
/* Check curve matches identifier string */
114-
if ((err = ecc_ssh_ecdsa_encode_name(name2, &namelen, key)) != CRYPT_OK) { goto error; }
115-
if (XSTRCMP(name,name2) != 0) {
115+
if ((err = ecc_ssh_ecdsa_encode_name(name2, &name2len, key)) != CRYPT_OK) { goto error; }
116+
if ((namelen != name2len) || (XSTRCMP(name, name2) != 0)) {
116117
err = CRYPT_INVALID_ARG;
117118
goto error;
118119
}

‎tests/common.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,12 @@ extern prng_state yarrow_prng;
1717
#define DO(x) do { fprintf(stderr, "%s:\n", #x); run_cmd((x), __LINE__, __FILE__, #x, NULL); } while (0)
1818
#define DOX(x, str) do { fprintf(stderr, "%s - %s:\n", #x, (str)); run_cmd((x), __LINE__, __FILE__, #x, (str)); } while (0)
1919
#define SHOULD_FAIL(x) do { fprintf(stderr, "%s:\n", #x); run_cmd((x) != CRYPT_OK ? CRYPT_OK : CRYPT_FAIL_TESTVECTOR, __LINE__, __FILE__, #x, NULL); } while (0)
20+
#define ENSURE(x) do { fprintf(stderr, "%s:\n", #x); run_cmd(((x)) ? CRYPT_OK : CRYPT_FAIL_TESTVECTOR, __LINE__, __FILE__, #x, NULL); } while (0)
2021
#else
2122
#define DO(x) do { run_cmd((x), __LINE__, __FILE__, #x, NULL); } while (0)
2223
#define DOX(x, str) do { run_cmd((x), __LINE__, __FILE__, #x, (str)); } while (0)
2324
#define SHOULD_FAIL(x) do { run_cmd((x) != CRYPT_OK ? CRYPT_OK : CRYPT_FAIL_TESTVECTOR, __LINE__, __FILE__, #x, NULL); } while (0)
25+
#define ENSURE(x) do { run_cmd(((x)) ? CRYPT_OK : CRYPT_FAIL_TESTVECTOR, __LINE__, __FILE__, #x, NULL); } while (0)
2426
#endif
2527

2628
#define COMPARE_TESTVECTOR(i, il, s, sl, wa, wi) do { DO(do_compare_testvector((i), (il), (s), (sl), (wa), (wi))); } while(0)

0 commit comments

Comments
 (0)