Re: [PATCH] Callback-based interface to libtls

[busterb]

Hey, the typedef came in handy :) Ok bcook@

> On Sep 5, 2016, at 11:52 AM, Bob Beck  wrote:
> 
> I am in agreement in principle, but please coordinate with bcook@ and/or 
> jsing@ who were possibly doing
> some related adjustments. 
> 
> 
> 
>> On Mon, Sep 5, 2016 at 4:44 AM, Ted Unangst  wrote:
>> Bob Beck wrote:
>> > >
>> > > Agreed, I was also a bit unclear on payload at first (though it grew on
>> > > me over time, so I didn't change it). Here's an update with the
>> > > parameter renamed and better documented.
>> > >
>> > > ok?
>> >
>> > Yeah. I'm good with this
>> >
>> > IMO get it in so we can tweak it in tree.
>> 
>> first tweak: the context has a type: struct tls *, so use it.
>> 
>> Index: tls.h
>> ===
>> RCS file: /cvs/src/lib/libtls/tls.h,v
>> retrieving revision 1.37
>> diff -u -p -r1.37 tls.h
>> --- tls.h   4 Sep 2016 14:15:44 -   1.37
>> +++ tls.h   5 Sep 2016 10:42:50 -
>> @@ -44,9 +44,9 @@ extern "C" {
>>  struct tls;
>>  struct tls_config;
>> 
>> -typedef ssize_t (*tls_read_cb)(void *_ctx, void *_buf, size_t _buflen,
>> +typedef ssize_t (*tls_read_cb)(struct tls *_ctx, void *_buf, size_t _buflen,
>>  void *_cb_arg);
>> -typedef ssize_t (*tls_write_cb)(void *_ctx, const void *_buf,
>> +typedef ssize_t (*tls_write_cb)(struct tls *_ctx, const void *_buf,
>>  size_t _buflen, void *_cb_arg);
>> 
>>  int tls_init(void);
>> Index: tls_init.3
>> ===
>> RCS file: /cvs/src/lib/libtls/tls_init.3,v
>> retrieving revision 1.71
>> diff -u -p -r1.71 tls_init.3
>> --- tls_init.3  4 Sep 2016 16:37:18 -   1.71
>> +++ tls_init.3  5 Sep 2016 10:43:43 -
>> @@ -189,13 +189,13 @@
>>  .Ft "int"
>>  .Fn tls_connect_socket "struct tls *ctx" "int s" "const char *servername"
>>  .Ft "int"
>> -.Fn tls_connect_cbs "struct tls *ctx" "ssize_t (*tls_read_cb)(void *ctx, 
>> void *buf, size_t buflen, void *cb_arg)" "ssize_t (*tls_write_cb)(void *ctx, 
>> const void *buf, size_t buflen, void *cb_arg)" "void *cb_arg" "const char 
>> *servername"
>> +.Fn tls_connect_cbs "struct tls *ctx" "ssize_t (*tls_read_cb)(struct tls 
>> *ctx, void *buf, size_t buflen, void *cb_arg)" "ssize_t 
>> (*tls_write_cb)(struct tls *ctx, const void *buf, size_t buflen, void 
>> *cb_arg)" "void *cb_arg" "const char *servername"
>>  .Ft "int"
>>  .Fn tls_accept_fds "struct tls *tls" "struct tls **cctx" "int fd_read" "int 
>> fd_write"
>>  .Ft "int"
>>  .Fn tls_accept_socket "struct tls *tls" "struct tls **cctx" "int socket"
>>  .Ft "int"
>> -.Fn tls_accept_cbs "struct tls *ctx" "struct tls **cctx" "ssize_t 
>> (*tls_read_cb)(void *ctx, void *buf, size_t buflen, void *cb_arg)" "ssize_t 
>> (*tls_write_cb)(void *ctx, const void *buf, size_t buflen, void *cb_arg)" 
>> "void *cb_arg"
>> +.Fn tls_accept_cbs "struct tls *ctx" "struct tls **cctx" "ssize_t 
>> (*tls_read_cb)(struct *ctx, void *buf, size_t buflen, void *cb_arg)" 
>> "ssize_t (*tls_write_cb)(struct tls *ctx, const void *buf, size_t buflen, 
>> void *cb_arg)" "void *cb_arg"
>>  .Ft "int"
>>  .Fn tls_handshake "struct tls *ctx"
>>  .Ft "ssize_t"
> 

Re: ntpd: strptime portability problem

[busterb]

Ok bcook@

> On Mar 4, 2016, at 4:32 PM, Christian Weisgerber  wrote:
> 
> Matthias Andree  noticed that the constraint
> offset was always off by 3600 seconds for him (running OpenNTPD on
> FreeBSD with CET timezone).
> 
> The way we parse the HTTP date in the ntpd constraint code isn't
> portable:
> 
>if (strptime(p, "%a, %d %h %Y %T %Z",
>>tls_tm) == NULL) {
> ...
>/* Return parsed date as local time */
>t = timegm(>tls_tm);
> 
> Neither %Z nor any kind of timezone information in struct tm are
> in POSIX.  We end up with a time that, depending on the operating
> system, is off by the offset between local time and UTC.
> 
> Since we only support the preferred HTTP date format from
> https://tools.ietf.org/html/rfc7231#section-7.1.1.1
> for which the timezone is fixed as the literal string "GMT",
> I suggest to simply replace %Z with GMT in the strptime() call.
> 
> Comments?  OK?
> 
> 
> Index: constraint.c
> ===
> RCS file: /cvs/src/usr.sbin/ntpd/constraint.c,v
> retrieving revision 1.25
> diff -u -p -r1.25 constraint.c
> --- constraint.c27 Jan 2016 21:48:34 -1.25
> +++ constraint.c4 Mar 2016 22:12:37 -
> @@ -903,7 +903,7 @@ httpsdate_request(struct httpsdate *http
> * or ANSI C's asctime() - the latter doesn't include
> * the timezone which is required here.
> */
> -if (strptime(p, "%a, %d %h %Y %T %Z",
> +if (strptime(p, "%a, %d %h %Y %T GMT",
>>tls_tm) == NULL) {
>log_warnx("unsupported date format");
>free(line);
> 
> -- 
> Christian "naddy" Weisgerber  na...@mips.inka.de
> 

[PATCH 2/2] include openssl/evp.h for OPENSSL_add_all_algorithms_noconf()

[busterb]

From: Brent Cook bust...@gmail.com

---
 rc4/rc4test.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/rc4/rc4test.c b/rc4/rc4test.c
index a0b08a6..c4d34b1 100644
--- a/rc4/rc4test.c
+++ b/rc4/rc4test.c
@@ -60,6 +60,7 @@
 #include stdlib.h
 #include string.h
 
+#include openssl/evp.h
 #include openssl/rc4.h
 #include openssl/sha.h
 
-- 
1.9.2

[PATCH 0/2] build warning fixes for regress/libcrypto

[busterb]

Hi all,

Here are some minor build fixes for the libcrypto regression tests.

 - Brent

[PATCH 1/2] use correct size_t formatter, include string.h for memcmp

[busterb]

From: Brent Cook bust...@gmail.com

---
 base64/base64test.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/base64/base64test.c b/base64/base64test.c
index a7d167e..fedbdcd 100644
--- a/base64/base64test.c
+++ b/base64/base64test.c
@@ -19,6 +19,7 @@
 
 #include err.h
 #include stdio.h
+#include string.h
 
 #define BUF_SIZE 128
 
@@ -205,7 +206,7 @@ base64_encoding_test(int test_no, struct base64_test *bt, 
int test_nl)
 
len = BIO_write(bio_mem, bt-in, bt-in_len);
if (len != bt-in_len) {
-   fprintf(stderr, FAIL: test %i - only wrote %i out of %i 
+   fprintf(stderr, FAIL: test %i - only wrote %i out of %zu 
characters\n, test_no, len, bt-in_len);
failure = 1;
goto done;
@@ -296,7 +297,7 @@ base64_decoding_test(int test_no, struct base64_test *bt, 
int test_nl)
len = BIO_read(bio_mem, buf, BUF_SIZE);
if (len != bt-valid_len  (bt-in_len != 0 || len != -1)) {
fprintf(stderr, FAIL: test %i - decoding resulted in %i 
-   characters instead of %i\n, test_no, len, bt-valid_len);
+   characters instead of %zu\n, test_no, len, bt-valid_len);
fprintf(stderr,   input: );
for (i = 0; i  inlen; i++)
fprintf(stderr, %c, input[i]);
-- 
1.9.2

[PATCH 1/6] initialize variable that can be used uninitialized

[busterb]

From: Brent Cook bust...@gmail.com

If EVP_DecryptInit_ex() returns NULL, j is incremented by a random amount.

clang warning:
pem/pem_lib.c:472:6: error: variable 'i' is used uninitialized whenever 'if' 
condition is false
  [-Werror,-Wsometimes-uninitialized]
if (o)
^
pem/pem_lib.c:479:7: note: uninitialized use occurs here
j += i;
 ^
pem/pem_lib.c:472:2: note: remove the 'if' if its condition is always true
if (o)
^~
pem/pem_lib.c:446:7: note: initialize the variable 'i' to silence this warning
int i, j, o, klen;
---
 src/crypto/pem/pem_lib.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/crypto/pem/pem_lib.c b/src/crypto/pem/pem_lib.c
index 9bc2b27..4e9bd82 100644
--- a/src/crypto/pem/pem_lib.c
+++ b/src/crypto/pem/pem_lib.c
@@ -465,6 +465,7 @@ PEM_do_header(EVP_CIPHER_INFO *cipher, unsigned char *data, 
long *plen,
(unsigned char *)buf, klen, 1, key, NULL))
return 0;
 
+   i = 0;
j = (int)len;
EVP_CIPHER_CTX_init(ctx);
o = EVP_DecryptInit_ex(ctx, cipher-cipher, NULL, key,
-- 
1.9.2

[PATCH 0/6] libssl compiler warning fixes

[busterb]

Hi all,

This is a set of generic fixes to libssl/crypto that I found building with
GCC 4.8 and Clang from LLVM 5.1.

The first patch seems to be an actual bug, while the others are a little more
pedantic (removing unused variables, fixing signed/unsigned char * aliases).

- Brent

[PATCH 2/6] fix type string conversion warning

[busterb]

From: Brent Cook bust...@gmail.com

ASN1_STRING_data returns an unsigned char *, but strlcat's second
parameter is a const char *
---
 src/crypto/ts/ts_rsp_verify.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/crypto/ts/ts_rsp_verify.c b/src/crypto/ts/ts_rsp_verify.c
index 2a4c0c5..49754b5 100644
--- a/src/crypto/ts/ts_rsp_verify.c
+++ b/src/crypto/ts/ts_rsp_verify.c
@@ -564,7 +564,7 @@ TS_get_status_text(STACK_OF(ASN1_UTF8STRING) *text)
ASN1_UTF8STRING *current = sk_ASN1_UTF8STRING_value(text, i);
if (i  0)
strlcat(result, /, length);
-   strlcat(result, ASN1_STRING_data(current), length);
+   strlcat(result, (const char *)ASN1_STRING_data(current), 
length);
}
return result;
 }
-- 
1.9.2

[PATCH 6/6] use BIO_write instead of an unchecked write()

[busterb]

From: Brent Cook bust...@gmail.com

write() warns if its return value is unchecked. Replace with a BIO_write
like all of the surrounding code uses anyway.
---
 src/apps/s_server.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/apps/s_server.c b/src/apps/s_server.c
index 4e1..729c94d 100644
--- a/src/apps/s_server.c
+++ b/src/apps/s_server.c
@@ -1769,7 +1769,7 @@ sv_body(char *hostname, int s, unsigned char *context)
i = SSL_read(con, (char *) buf, bufsize);
switch (SSL_get_error(con, i)) {
case SSL_ERROR_NONE:
-   write(fileno(stdout), buf,
+   BIO_write(bio_s_out, buf,
(unsigned int) i);
if (SSL_pending(con))
goto again;
-- 
1.9.2

[PATCH 3/6] remove unused static datastructures

[busterb]

From: Brent Cook bust...@gmail.com

Neither of these is used anywhere within their object files.
---
 src/crypto/ec/ec_lib.c  | 3 ---
 src/crypto/engine/eng_dyn.c | 3 ---
 2 files changed, 6 deletions(-)

diff --git a/src/crypto/ec/ec_lib.c b/src/crypto/ec/ec_lib.c
index b37efac..37dfd17 100644
--- a/src/crypto/ec/ec_lib.c
+++ b/src/crypto/ec/ec_lib.c
@@ -68,9 +68,6 @@
 
 #include ec_lcl.h
 
-static const char EC_version[] = EC OPENSSL_VERSION_PTEXT;
-
-
 /* functions for EC_GROUP objects */
 
 EC_GROUP *
diff --git a/src/crypto/engine/eng_dyn.c b/src/crypto/engine/eng_dyn.c
index 7878bd8..08a1cde 100644
--- a/src/crypto/engine/eng_dyn.c
+++ b/src/crypto/engine/eng_dyn.c
@@ -114,9 +114,6 @@ static const ENGINE_CMD_DEFN dynamic_cmd_defns[] = {
ENGINE_CMD_FLAG_NO_INPUT},
{0, NULL, NULL, 0}
};
-static const ENGINE_CMD_DEFN dynamic_cmd_defns_empty[] = {
-   {0, NULL, NULL, 0}
-   };
 
 /* Loading code stores state inside the ENGINE structure via the ex_data
  * element. We load all our state into a single structure and use that as a
-- 
1.9.2

[PATCH 4/6] do not include public headers as though they are local

[busterb]

From: Brent Cook bust...@gmail.com

Avoid having to use -I trickery to find public header files included as
though they are private.
---
 src/crypto/bn/bn_const.c | 2 +-
 src/crypto/chacha/chacha.c   | 2 +-
 src/crypto/o_init.c  | 2 +-
 src/crypto/pqueue/pqueue.c   | 2 +-
 src/crypto/ts/ts_lib.c   | 2 +-
 src/crypto/ts/ts_rsp_print.c | 2 +-
 6 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/src/crypto/bn/bn_const.c b/src/crypto/bn/bn_const.c
index 1c8bc9b..2ba2bbd 100644
--- a/src/crypto/bn/bn_const.c
+++ b/src/crypto/bn/bn_const.c
@@ -1,7 +1,7 @@
 /* crypto/bn/knownprimes.c */
 /* Insert boilerplate */
 
-#include bn.h
+#include openssl/bn.h
 
 /* First Oakley Default Group from RFC2409, section 6.1.
  *
diff --git a/src/crypto/chacha/chacha.c b/src/crypto/chacha/chacha.c
index 1bc95f5..f0e818e 100644
--- a/src/crypto/chacha/chacha.c
+++ b/src/crypto/chacha/chacha.c
@@ -14,7 +14,7 @@
  * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
  */
 
-#include chacha.h
+#include openssl/chacha.h
 #include chacha-merged.c
 
 void
diff --git a/src/crypto/o_init.c b/src/crypto/o_init.c
index 09c9820..2c95a9e 100644
--- a/src/crypto/o_init.c
+++ b/src/crypto/o_init.c
@@ -1,5 +1,5 @@
 /* Ted Unangst places this file in the public domain. */
-#include crypto.h
+#include openssl/crypto.h
 
 void
 OPENSSL_init(void)
diff --git a/src/crypto/pqueue/pqueue.c b/src/crypto/pqueue/pqueue.c
index fc68ae1..3239abf 100644
--- a/src/crypto/pqueue/pqueue.c
+++ b/src/crypto/pqueue/pqueue.c
@@ -59,7 +59,7 @@
 
 #include cryptlib.h
 #include openssl/bn.h
-#include pqueue.h
+#include openssl/pqueue.h
 
 typedef struct _pqueue {
pitem *items;
diff --git a/src/crypto/ts/ts_lib.c b/src/crypto/ts/ts_lib.c
index 81a5f23..152320f 100644
--- a/src/crypto/ts/ts_lib.c
+++ b/src/crypto/ts/ts_lib.c
@@ -61,7 +61,7 @@
 #include openssl/objects.h
 #include openssl/bn.h
 #include openssl/x509v3.h
-#include ts.h
+#include openssl/ts.h
 
 /* Local function declarations. */
 
diff --git a/src/crypto/ts/ts_rsp_print.c b/src/crypto/ts/ts_rsp_print.c
index 248674f..6615e20 100644
--- a/src/crypto/ts/ts_rsp_print.c
+++ b/src/crypto/ts/ts_rsp_print.c
@@ -61,7 +61,7 @@
 #include openssl/objects.h
 #include openssl/bn.h
 #include openssl/x509v3.h
-#include ts.h
+#include openssl/ts.h
 
 struct status_map_st {
int bit;
-- 
1.9.2

[PATCH 5/6] do not pretend to support -rand in apps

[busterb]

From: Brent Cook bust...@gmail.com

Remove support for parsing the unused -rand option and the unused random
buffer variables.
---
 src/apps/cms.c  |  9 -
 src/apps/dgst.c |  8 ++--
 src/apps/dhparam.c  | 10 +-
 src/apps/dsaparam.c |  7 +--
 src/apps/ecparam.c  |  9 +
 src/apps/gendh.c| 10 +-
 src/apps/gendsa.c   | 11 ++-
 src/apps/genrsa.c   |  9 -
 src/apps/pkcs12.c   | 10 --
 src/apps/rand.c | 10 +-
 src/apps/req.c  |  9 -
 src/apps/s_client.c |  8 +---
 src/apps/s_server.c |  7 ---
 src/apps/smime.c| 10 --
 src/apps/ts.c   |  7 +--
 15 files changed, 11 insertions(+), 123 deletions(-)

diff --git a/src/apps/cms.c b/src/apps/cms.c
index aa59b28..103ff7a 100644
--- a/src/apps/cms.c
+++ b/src/apps/cms.c
@@ -126,7 +126,6 @@ cms_main(int argc, char **argv)
char *to = NULL, *from = NULL, *subject = NULL;
char *CAfile = NULL, *CApath = NULL;
char *passargin = NULL, *passin = NULL;
-   char *inrand = NULL;
const EVP_MD *sign_md = NULL;
int informat = FORMAT_SMIME, outformat = FORMAT_SMIME;
int rctformat = FORMAT_SMIME, keyform = FORMAT_PEM;
@@ -324,11 +323,6 @@ cms_main(int argc, char **argv)
BIO_printf(bio_err, Invalid OID %s\n, *args);
goto argerr;
}
-   } else if (!strcmp(*args, -rand)) {
-   if (!args[1])
-   goto argerr;
-   args++;
-   inrand = *args;
}
 #ifndef OPENSSL_NO_ENGINE
else if (!strcmp(*args, -engine)) {
@@ -565,9 +559,6 @@ argerr:
BIO_printf(bio_err, -engine e  use engine e, possibly a 
hardware device.\n);
 #endif
BIO_printf(bio_err, -passin arginput file pass phrase 
source\n);
-   BIO_printf(bio_err, -rand file:file:...\n);
-   BIO_printf(bio_err,load the file (or the files 
in the directory) into\n);
-   BIO_printf(bio_err,the random number 
generator\n);
BIO_printf(bio_err, cert.pem   recipient certificate(s) 
for encryption\n);
goto end;
}
diff --git a/src/apps/dgst.c b/src/apps/dgst.c
index 523f5cc..34b4866 100644
--- a/src/apps/dgst.c
+++ b/src/apps/dgst.c
@@ -115,7 +115,7 @@ dgst_main(int argc, char **argv)
int debug = 0;
int keyform = FORMAT_PEM;
const char *outfile = NULL, *keyfile = NULL;
-   const char *sigfile = NULL, *randfile = NULL;
+   const char *sigfile = NULL;
int out_bin = -1, want_pub = 0, do_verify = 0;
EVP_PKEY *sigkey = NULL;
unsigned char *sigbuf = NULL;
@@ -156,11 +156,7 @@ dgst_main(int argc, char **argv)
separator = 1;
else if (strcmp(*argv, -r) == 0)
separator = 2;
-   else if (strcmp(*argv, -rand) == 0) {
-   if (--argc  1)
-   break;
-   randfile = *(++argv);
-   } else if (strcmp(*argv, -out) == 0) {
+   else if (strcmp(*argv, -out) == 0) {
if (--argc  1)
break;
outfile = *(++argv);
diff --git a/src/apps/dhparam.c b/src/apps/dhparam.c
index b09f641..fa020bd 100644
--- a/src/apps/dhparam.c
+++ b/src/apps/dhparam.c
@@ -156,7 +156,6 @@ dhparam_main(int argc, char **argv)
BIO *in = NULL, *out = NULL;
int informat, outformat, check = 0, noout = 0, C = 0, ret = 1;
char *infile, *outfile, *prog;
-   char *inrand = NULL;
 #ifndef OPENSSL_NO_ENGINE
char *engine = NULL;
 #endif
@@ -220,11 +219,7 @@ dhparam_main(int argc, char **argv)
g = 2;
else if (strcmp(*argv, -5) == 0)
g = 5;
-   else if (strcmp(*argv, -rand) == 0) {
-   if (--argc  1)
-   goto bad;
-   inrand = *(++argv);
-   } else if (((sscanf(*argv, %d, num) == 0) || (num = 0)))
+   else if (((sscanf(*argv, %d, num) == 0) || (num = 0)))
goto bad;
argv++;
argc--;
@@ -250,9 +245,6 @@ bad:
 #ifndef OPENSSL_NO_ENGINE
BIO_printf(bio_err,  -engine e use engine e, possibly a 
hardware device.\n);
 #endif
-   BIO_printf(bio_err,  -rand file:file:...\n);
-   BIO_printf(bio_err,- load the file (or the 
files in the directory) into\n);
-   BIO_printf(bio_err,the random number 
generator\n);
BIO_printf(bio_err,  -nooutno output\n);
goto end;
}
diff --git a/src/apps/dsaparam.c