Re: malloc in libssl/src/apps

[Joel Sing]

On Mon, 5 May 2014, Jean-Philippe Ouellet wrote:
 On Mon, May 05, 2014 at 11:12:00AM +1000, Joel Sing wrote:
   -   i = 0;
   if (arg-count == 0) {
   arg-count = 20;
   -   arg-data = (char **)malloc(sizeof(char *) * arg-count);
   +   arg-data = calloc(arg-count, sizeof(char *));
   }
   -   for (i = 0; i  arg-count; i++)
   -   arg-data[i] = NULL;
 
  This one is a change in behaviour - if arg-count is  0 then previously
  we zeroed arg-data; now we do not.

 This one is calloc, not reallocarray, so unless I'm seriously missing
 something obvious here, it is indeed zero'd, no?

Run the following before and after your change:

#include stdio.h
#include strings.h

#include openssl/bio.h
#include openssl/conf.h

#include apps.h

BIO *bio_err;
CONF *config;

int
main(int argc, char **argv)
{
char buf[128] = -one -two -three -four -five;
ARGS args;
int i;

memset(args, 0, sizeof(args));

chopup_args(args, buf, argc, argv);

for (i = 0; i  args.count; i++)
printf(%i = %p\n, i, args.data[i]);

strlcpy(buf, -one -two, sizeof(buf));

chopup_args(args, buf, argc, argv);

for (i = 0; i  args.count; i++)
printf(%i = %p\n, i, args.data[i]);

}

$ gcc -o chopup chopup.c /usr/src/lib/libssl/src/apps/apps.c -I 
/usr/src/lib/libssl/src/apps -lcrypto
-- 

Action without study is fatal. Study without action is futile.
-- Mary Ritter Beard

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

[Joel Sing]

Thanks for the diff - I've committed a more comprehensive diff that makes it 
clean with WARNINGS=Yes.

On Mon, 12 May 2014, bust...@gmail.com wrote:
 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]);



-- 

Action without study is fatal. Study without action is futile.
-- Mary Ritter Beard

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

[Joel Sing]

On Mon, 12 May 2014, bust...@gmail.com wrote:
 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

Committed. Thanks.
-- 

Action without study is fatal. Study without action is futile.
-- Mary Ritter Beard

Re: [PATCH 9] installboot: malloc/memset = calloc

[Joel Sing]

On Sat, 31 May 2014, Benjamin Baier wrote:
 This one splits up the malloc parameter, taking full potential from calloc,
 hurting readability a bit. which one is preferred? more
 readable/maintainable or using the calloc overflow protection?

In this case I think readability wins. I do not believe that there is a lot to 
gain from overflow protection given the numbers used in these calculations 
are very small (and many are already bounds checked in some form or other).

 Index: bootstrap.c
 ===
 RCS file: /cvs/src/usr.sbin/installboot/bootstrap.c,v
 retrieving revision 1.3
 diff -u -p -r1.3 bootstrap.c
 --- bootstrap.c   28 Dec 2013 12:01:33 -  1.3
 +++ bootstrap.c   31 May 2014 10:26:27 -
 @@ -68,10 +68,9 @@ bootstrap(int devfd, char *dev, char *bo
   fprintf(stderr, bootstrap is %zu bytes 
   (%zu sectors @ %u bytes = %zu bytes)\n,
   (ssize_t)sb.st_size, bootsec, dl.d_secsize, bootsize);
 - boot = malloc(bootsize);
 + boot = calloc(bootsec, dl.d_secsize);
   if (boot == NULL)
 - err(1, malloc);
 - memset(boot, 0, bootsize);
 + err(1, calloc);
   if (read(fd, boot, bootsize) != (ssize_t)sb.st_size)
   err(1, read);
   close(fd);
 Index: i386_softraid.c
 ===
 RCS file: /cvs/src/usr.sbin/installboot/i386_softraid.c,v
 retrieving revision 1.1
 diff -u -p -r1.1 i386_softraid.c
 --- i386_softraid.c   19 Jan 2014 02:58:50 -  1.1
 +++ i386_softraid.c   31 May 2014 10:26:27 -
 @@ -129,11 +129,10 @@ sr_install_bootldr(int devfd, char *dev)
   inodeblk = nblocks - 1;
   bootsize = nblocks * SR_FS_BLOCKSIZE;

 - p = malloc(bootsize);
 + p = calloc(nblocks, SR_FS_BLOCKSIZE);
   if (p == NULL)
   err(1, NULL);

 - memset(p, 0, bootsize);
   fd = open(stage2, O_RDONLY, 0);
   if (fd == -1)
   err(1, NULL);
 Index: sparc64_installboot.c
 ===
 RCS file: /cvs/src/usr.sbin/installboot/sparc64_installboot.c,v
 retrieving revision 1.1
 diff -u -p -r1.1 sparc64_installboot.c
 --- sparc64_installboot.c 19 Jan 2014 02:58:50 -  1.1
 +++ sparc64_installboot.c 31 May 2014 10:26:27 -
 @@ -68,10 +68,9 @@ md_loadboot(void)
   if (blksize  SBSIZE - DEV_BSIZE)
   errx(1, boot blocks too big (%zu  %d),
   blksize, SBSIZE - DEV_BSIZE);
 - blkstore = malloc(blksize);
 + blkstore = calloc(blocks, DEV_BSIZE);
   if (blkstore == NULL)
 - err(1, malloc);
 - memset(blkstore, 0, blksize);
 + err(1, calloc);
   if (read(fd, blkstore, sb.st_size) != (ssize_t)sb.st_size)
   err(1, read);
   close(fd);

-- 

Action without study is fatal. Study without action is futile.
-- Mary Ritter Beard

Re: [PATCH 1/7] If EVP_DecryptInit_ex() returns NULL, j is incremented by a random amount in PEM_do_header()

[Joel Sing]

On Sun, 1 Jun 2014, Brent Cook wrote:
 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 945262f..92c3dc4 100644
 --- a/src/crypto/pem/pem_lib.c
 +++ b/src/crypto/pem/pem_lib.c
 @@ -454,6 +454,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,

This is a non-issue since the value of j is unused in the !o case. That
said, I've just commited the following diff actually fixes the code,
rather than just addressing the uninitialised variable:

Index: pem_lib.c
===
RCS file: /cvs/src/lib/libssl/src/crypto/pem/pem_lib.c,v
retrieving revision 1.23
diff -u -p -r1.23 pem_lib.c
--- pem_lib.c   26 Apr 2014 18:56:38 -  1.23
+++ pem_lib.c   29 May 2014 15:39:26 -
@@ -476,12 +476,11 @@ PEM_do_header(EVP_CIPHER_INFO *cipher, u
EVP_CIPHER_CTX_cleanup(ctx);
OPENSSL_cleanse((char *)buf, sizeof(buf));
OPENSSL_cleanse((char *)key, sizeof(key));
-   j += i;
if (!o) {
PEMerr(PEM_F_PEM_DO_HEADER, PEM_R_BAD_DECRYPT);
return (0);
}
-   *plen = j;
+   *plen = j + i;
return (1);
 }
 


-- 

Action without study is fatal. Study without action is futile.
-- Mary Ritter Beard

Re: [PATCH 6/7] remove parsing of -rand options in openssl apps

[Joel Sing]

On Sun, 1 Jun 2014, Brent Cook wrote:
 Since the random number generator no longer allows being seeded, remove
 support for parsing the unused -rand option and the unused random buffer
 variables. Better to fail than to be surprised when the RNG seed does not
 function as expected.

 This fixes compiler warnings about unused random seed variables.

Commited, thanks.

 ---
  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 56a7c95..76178b4 100644
 --- a/src/apps/cms.c
 +++ b/src/apps/cms.c
 @@ -127,7 +127,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;
 @@ -315,11 +314,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)) {
 @@ -553,9 +547,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 23b7d40..a862da9 100644
 --- a/src/apps/dgst.c
 +++ b/src/apps/dgst.c
 @@ -116,7 +116,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;
 @@ -151,11 +151,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 3245e69..c35f902 100644
 --- a/src/apps/dhparam.c
 +++ b/src/apps/dhparam.c
 @@ -159,7 +159,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
 @@ -217,11 +216,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--;
 @@ -247,9 +242,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, 

Re: clean/portable crypto code...

[Joel Sing]

On Sat, 7 Jun 2014, John-Mark Gurney wrote:
 Hello,

 I've been doing some work recently on crypto code, and noticed that
 there aren't many/any good clean implementations of performant crypto
 code out there (or maybe I just don't know of them).  Both OpenSSL's
 and NSS's code has issues w/ portability and/or cleanliness.

There are a few places that tend to have good clean (and generally portable) 
implementations:

 http://www.literatecode.com/aes256

 https://github.com/floodyberry?tab=repositories

 http://cr.yp.to/chacha.html

One of the biggest issues is that performant code tends to counter 
cleanliness, since the optimisations required usually result in less readable 
and maintainable code. That said, it also depends on what architecture you 
are optimising for.

 But, I prefer to reuse code so that hopefully, when one bug is found,
 derivatives can be fixed.

 Is there any interest in collaberation?

 My current interest is in AES-GCM and AES-XTS.

OpenBSD has AES-GCM and AES-XTS in the kernel crypto code (built around the 
cryptodev API) and there is also a standalone version of AES-XTS in libsa 
since it is used by our boot loader:

 http://www.openbsd.org/cgi-bin/cvsweb/src/sys/lib/libsa/aes_xts.c
 http://www.openbsd.org/cgi-bin/cvsweb/src/sys/lib/libsa/rijndael.c

 http://www.openbsd.org/cgi-bin/cvsweb/src/sys/crypto/xform.c
 http://www.openbsd.org/cgi-bin/cvsweb/src/sys/crypto/rijndael.c

There is also the AES-NI implementations of AES-XTS and AES-GCM, for hardware 
has AES-NI support:

 http://www.openbsd.org/cgi-bin/cvsweb/src/sys/arch/amd64/amd64/aesni.c
 http://www.openbsd.org/cgi-bin/cvsweb/src/sys/arch/amd64/amd64/aes_intel.S

Not sure if any of these align with your interests.

 I'm looking at taking a version of the AES-GCM code from NSS (heavily
 modified as it is unportable) for import into FreeBSD.

 Thanks.
-- 

Action without study is fatal. Study without action is futile.
-- Mary Ritter Beard

Re: [PATCH 9] installboot: malloc/memset = calloc

[Joel Sing]

Commited. Thanks.

On Sun, 1 Jun 2014, Benjamin Baier wrote:
 On Sun, 1 Jun 2014 00:57:43 +1000

 Joel Sing j...@sing.id.au wrote:
  In this case I think readability wins. I do not believe that there is a
  lot to gain from overflow protection given the numbers used in these
  calculations are very small (and many are already bounds checked in some
  form or other).

 Well, I had to ask. Here is option 2.

 Index: bootstrap.c
 ===
 RCS file: /cvs/src/usr.sbin/installboot/bootstrap.c,v
 retrieving revision 1.3
 diff -u -p -r1.3 bootstrap.c
 --- bootstrap.c   28 Dec 2013 12:01:33 -  1.3
 +++ bootstrap.c   31 May 2014 18:14:56 -
 @@ -68,10 +68,9 @@ bootstrap(int devfd, char *dev, char *bo
   fprintf(stderr, bootstrap is %zu bytes 
   (%zu sectors @ %u bytes = %zu bytes)\n,
   (ssize_t)sb.st_size, bootsec, dl.d_secsize, bootsize);
 - boot = malloc(bootsize);
 + boot = calloc(1, bootsize);
   if (boot == NULL)
 - err(1, malloc);
 - memset(boot, 0, bootsize);
 + err(1, calloc);
   if (read(fd, boot, bootsize) != (ssize_t)sb.st_size)
   err(1, read);
   close(fd);
 Index: i386_softraid.c
 ===
 RCS file: /cvs/src/usr.sbin/installboot/i386_softraid.c,v
 retrieving revision 1.1
 diff -u -p -r1.1 i386_softraid.c
 --- i386_softraid.c   19 Jan 2014 02:58:50 -  1.1
 +++ i386_softraid.c   31 May 2014 18:14:56 -
 @@ -129,11 +129,10 @@ sr_install_bootldr(int devfd, char *dev)
   inodeblk = nblocks - 1;
   bootsize = nblocks * SR_FS_BLOCKSIZE;

 - p = malloc(bootsize);
 + p = calloc(1, bootsize);
   if (p == NULL)
   err(1, NULL);

 - memset(p, 0, bootsize);
   fd = open(stage2, O_RDONLY, 0);
   if (fd == -1)
   err(1, NULL);
 Index: sparc64_installboot.c
 ===
 RCS file: /cvs/src/usr.sbin/installboot/sparc64_installboot.c,v
 retrieving revision 1.1
 diff -u -p -r1.1 sparc64_installboot.c
 --- sparc64_installboot.c 19 Jan 2014 02:58:50 -  1.1
 +++ sparc64_installboot.c 31 May 2014 18:14:56 -
 @@ -68,10 +68,9 @@ md_loadboot(void)
   if (blksize  SBSIZE - DEV_BSIZE)
   errx(1, boot blocks too big (%zu  %d),
   blksize, SBSIZE - DEV_BSIZE);
 - blkstore = malloc(blksize);
 + blkstore = calloc(1, blksize);
   if (blkstore == NULL)
 - err(1, malloc);
 - memset(blkstore, 0, blksize);
 + err(1, calloc);
   if (read(fd, blkstore, sb.st_size) != (ssize_t)sb.st_size)
   err(1, read);
   close(fd);



-- 

Action without study is fatal. Study without action is futile.
-- Mary Ritter Beard

Re: openssl smime -sign broken

[Joel Sing]

On Sun, 29 Jun 2014, Stuart Henderson wrote:
 Does anyone have ideas about this before I start digging to find when
 it got broken?

Still digging, but it looks like it will be caused by crypto/pkcs7/pk7_doit.c. 
r1.20...

 On -current:
 | $ echo test message | openssl smime -sign -signer mail.cert -inkey
 | mail.key Enter pass phrase for mail.key:
 | MIME-Version: 1.0
 | Content-Type: multipart/signed; protocol=application/x-pkcs7-signature;
 | micalg=sha1; boundary=D6AE072D584EB7DD96921A8B5D85CEB4
 |
 | This is an S/MIME signed message
 |
 | --D6AE072D584EB7DD96921A8B5D85CEB4
 | test message

 i.e. no signature section.

 On 5.4 :-

 | $ echo test message | openssl smime -sign -signer mail.cert -inkey
 | mail.key Enter pass phrase for mail.key
 | MIME-Version: 1.0
 | Content-Type: multipart/signed; protocol=application/x-pkcs7-signature;
 | micalg=sha1; boundary=4B2635AD8016836C6AB87A92750410E5
 |
 | This is an S/MIME signed message
 |
 | --4B2635AD8016836C6AB87A92750410E5
 | test message
 |
 | --4B2635AD8016836C6AB87A92750410E5
 | Content-Type: application/x-pkcs7-signature; name=smime.p7s
 | Content-Transfer-Encoding: base64
 | Content-Disposition: attachment; filename=smime.p7s
 |
 | snip 49 lines
 |
 | --4B2635AD8016836C6AB87A92750410E5--

 Also with -outform DER added to the command line, on -current:

 24566492045156:error:21080082:PKCS7 routines:PKCS7_dataFinal:decode
 error:/usr/src/lib/libcrypto/crypto/../../libssl/src/crypto/pkcs7/pk7_doit.
c:791: 24566492045156:error:21086091:PKCS7 routines:PKCS7_final:pkcs7
 datasign:/usr/src/lib/libcrypto/crypto/../../libssl/src/crypto/pkcs7/pk7_sm
ime.c:132:

 On 5.4, binary data is output as expected.

 openssl smime -encrypt *does* work.



-- 

Action without study is fatal. Study without action is futile.
-- Mary Ritter Beard

Re: openssl smime -sign broken

[Joel Sing]

On Mon, 30 Jun 2014, Joel Sing wrote:
 On Sun, 29 Jun 2014, Stuart Henderson wrote:
  Does anyone have ideas about this before I start digging to find when
  it got broken?

 Still digging, but it looks like it will be caused by
 crypto/pkcs7/pk7_doit.c. r1.20...

The following diff resolves the issue:

Index: pk7_doit.c
===
RCS file: /cvs/src/lib/libssl/src/crypto/pkcs7/pk7_doit.c,v
retrieving revision 1.22
diff -u -p -r1.22 pk7_doit.c
--- pk7_doit.c  12 Jun 2014 15:49:30 -  1.22
+++ pk7_doit.c  29 Jun 2014 15:56:29 -
@@ -787,7 +787,7 @@ int PKCS7_dataFinal(PKCS7 *p7, BIO *bio)
case NID_pkcs7_signed:
si_sk=p7-d.sign-signer_info;
os=PKCS7_get_octet_string(p7-d.sign-contents);
-   if (os == NULL) {
+   if (os == NULL  !PKCS7_is_detached(p7)) {
PKCS7err(PKCS7_F_PKCS7_DATAFINAL, PKCS7_R_DECODE_ERROR);
goto err;
}
-- 

Action without study is fatal. Study without action is futile.
-- Mary Ritter Beard

Re: LibreSSL: base64 decoding error

[Joel Sing]

On Thu, 31 Jul 2014, Joel Sing wrote:
 On Thu, 31 Jul 2014, Dmitry Eremin-Solenikov wrote:
  Hello,
 
  I have spotted a problem with the patch of crypto/evp/encode.c done by
  jsing on May 3.
  Sometimes decoding of base64 will fail. For example the attached file
  will fail decodiding
  (and produce an empty output):
 
  ./apps/openssl enc -d -base64  34.10-01.key
 
  The OpenSSL team has applied another fix:
 
  http://git.openssl.org/gitweb/?p=openssl.git;a=commitdiff;h=fce382e33
 07 a599d2378f2cca2ef2097c6c4;hp=12e9f627f9dd9a9f75d4a7beb6baf30a3697d8e0
 
  The attached patch (differing from OpenSSL one) fixes base64 decoding for
  me.

 PEM != base64 - there is base64 content inside the PEM markers, but you're
 trying to decode the entire thing, with PEM markers, as base64. If you
 remove the PEM markers it decodes correctly. I suspect that this is related
 to the end-of-line handling flags, which will be causing the '\n' to be
 discarded and the next character ('-') to be treated as part of the base64
 content (which, sadly, is likely working-as-intended).

Just to confirm, this is not actually related to BIO_FLAGS_BASE64_NO_NL - as 
far as the base64 decoding is concerned, the '-END PRIVATE KEY-' 
marker is considered to be part of the base64 content, since we've not yet 
reached the end of the file. This is obviously invalid base64 content, hence 
decoding fails. 
-- 

Action without study is fatal. Study without action is futile.
-- Mary Ritter Beard

Re: Is there a repo for the latest LibreSSL portable?

[Joel Sing]

On Mon, 11 Aug 2014, Nicholas Wilson wrote:
 Hi Ingo,

 On 10 August 2014 15:54, Ingo Schwarze schwa...@usta.de wrote:
  Portability goo clutters code and reduces readability, and hence
  endangers correctness and security ...
  Making a portable version is *impossible*
  without some clutter (even though the portability goo in OpenBSD
  sub-projects is often less heavy than the clutter you find in some
  other project's master repos).

 I understand the reasoning, but for LibreSSL it seems a shame since
 the portable goo is so minimal. Unlike OpenSSH, which has by
 necessity tons of hooks for platform behaviour, the only changes so
 far in LibreSSL portable are adding an implementation of OpenBSD
 functions like getentropy(), and some headers. Having those platform
 implementations sitting there in a compat directory doesn't make it
 harder to audit the code, does it?

 Oh well! The project will work it out if it becomes a common problem.

 My main question is still unanswered, namely what the ideas are for
 the API exposing the RSA PSS/OAEP MGF1 hash. Should I send in a patch
 porting over the OpenSSL 1.0.2 API for it?

Which API are you referring to? You are certainly welcome to send a diff - I 
cannot guarantee that it will be committed, however we would certainly review 
and consider it.

 Better, I'd ideally like to 
 split out libcrypto into more modular components so that LibreSSL can
 be used without all the horrific layers of goo (ECDH_METHOD structure
 and other useless clutter!). The OpenSSL API goo can remain as a way
 to access the underlying crypto functions, but the internal API should
 be cleaner. I'd be interested in making those changes for the RSA and
 EC code.

At this stage our primary approach is to maintain API compatiability (as far 
as possible) with OpenSSL. That said, I have been pondering an easy to use 
and robust interface for ed25519. If you came up with an API that was 
consistent/clean and worked for both ed25519 and RSA-PSS, then I'd certainly 
be interested. That said, we would probably look at providing the OpenSSL API 
as a wrapper around the cleaner API.
-- 

   Stop assuming that systems are secure unless demonstrated insecure;
start assuming that systems are insecure unless designed securely.
  - Bruce Schneier

Re: improve ressl config setting

[Joel Sing]

On Fri, 12 Sep 2014, Ted Unangst wrote:
 On Wed, Sep 10, 2014 at 16:38, Ted Unangst wrote:
  On Fri, Aug 15, 2014 at 13:06, Ted Unangst wrote:
  Instead, ressl should copy all parameters as necessary and
  free them. This does introduce an error case into formerly void
  functions, but I think that's ok. The alternative would be to use
  fixed arrays inside ressl_config, but that seems much worse.
 
  Here's a complete diff.

Initial comments inline...

  (I think we should also zero the key memory if not null, but that's not
  included in this change.)

 Kent Spillner noticed I hadn't cleaned up the references to default
 config in ressl.c. Here's a fixed diff, that also zeroes key memory.

 Index: ressl.c
 ===
 RCS file: /cvs/src/lib/libressl/ressl.c,v
 retrieving revision 1.12
 diff -u -p -r1.12 ressl.c
 --- ressl.c   15 Aug 2014 16:55:32 -  1.12
 +++ ressl.c   11 Sep 2014 19:18:49 -
 @@ -29,7 +29,7 @@
  #include ressl.h
  #include ressl_internal.h

 -extern struct ressl_config ressl_config_default;
 +static struct ressl_config *ressl_config_default;

  int
  ressl_init(void)
 @@ -42,6 +42,9 @@ ressl_init(void)
   SSL_load_error_strings();
   SSL_library_init();

 + if ((ressl_config_default = ressl_config_new()) == NULL)
 + return (-1);
 +
   ressl_initialised = 1;

   return (0);
 @@ -78,7 +81,7 @@ ressl_new(void)
   if ((ctx = calloc(1, sizeof(*ctx))) == NULL)
   return (NULL);

 - ctx-config = ressl_config_default;
 + ctx-config = ressl_config_default;

   ressl_reset(ctx);

 @@ -89,7 +92,7 @@ int
  ressl_configure(struct ressl *ctx, struct ressl_config *config)
  {
   if (config == NULL)
 - config = ressl_config_default;
 + config = ressl_config_default;

   ctx-config = config;

 Index: ressl.h
 ===
 RCS file: /cvs/src/lib/libressl/ressl.h,v
 retrieving revision 1.13
 diff -u -p -r1.13 ressl.h
 --- ressl.h   27 Aug 2014 10:46:53 -  1.13
 +++ ressl.h   10 Sep 2014 20:23:46 -
 @@ -31,15 +31,15 @@ const char *ressl_error(struct ressl *ct
  struct ressl_config *ressl_config_new(void);
  void ressl_config_free(struct ressl_config *config);

 -void ressl_config_set_ca_file(struct ressl_config *config, char *ca_file);
 -void ressl_config_set_ca_path(struct ressl_config *config, char *ca_path);
 -void ressl_config_set_cert_file(struct ressl_config *config, char
 *cert_file); -void ressl_config_set_cert_mem(struct ressl_config *config,
 char *cert, +int ressl_config_set_ca_file(struct ressl_config *config, char
 *ca_file); +int ressl_config_set_ca_path(struct ressl_config *config, char
 *ca_path); +int ressl_config_set_cert_file(struct ressl_config *config,
 char *cert_file); +int ressl_config_set_cert_mem(struct ressl_config
 *config, char *cert, size_t len);
 -void ressl_config_set_ciphers(struct ressl_config *config, char *ciphers);
 +int ressl_config_set_ciphers(struct ressl_config *config, char *ciphers);
  int ressl_config_set_ecdhcurve(struct ressl_config *config, const char *);
 -void ressl_config_set_key_file(struct ressl_config *config, char
 *key_file); -void ressl_config_set_key_mem(struct ressl_config *config,
 char *key, +int ressl_config_set_key_file(struct ressl_config *config, char
 *key_file); +int ressl_config_set_key_mem(struct ressl_config *config, char
 *key, size_t len);
  void ressl_config_set_verify_depth(struct ressl_config *config,
  int verify_depth);
 Index: ressl_config.c
 ===
 RCS file: /cvs/src/lib/libressl/ressl_config.c,v
 retrieving revision 1.8
 diff -u -p -r1.8 ressl_config.c
 --- ressl_config.c27 Aug 2014 10:46:53 -  1.8
 +++ ressl_config.c11 Sep 2014 19:14:27 -
 @@ -21,27 +21,55 @@
  #include ressl.h
  #include ressl_internal.h

 -/*
 - * Default configuration.
 - */
 -struct ressl_config ressl_config_default = {
 - .ca_file = _PATH_SSL_CA_FILE,
 - .ca_path = NULL,
 - .ciphers = NULL,
 - .ecdhcurve = NID_X9_62_prime256v1,
 - .verify = 1,
 - .verify_depth = 6,
 -};
 +#define SET_STRING(config, name, val) do {   \
 + free(config-name); \
 + config-name = NULL;\
 + if (val != NULL) {  \
 + if ((config-name = strdup(val)) == NULL)   \
 + return -1;  \
 + }   \
 +} while (0)
 +
 +#define SET_MEM(config, name, namelen, val, vallen) do { \
 + free(config-name); \
 + config-name = NULL;\
 + if (val != NULL) {  \
 + if ((config-name = memdup(val, vallen)) == NULL)   \
 + return -1;  \
 + config-namelen = vallen;   \
 + } 

Re: openssl.cnf req defaults - default_md sha256

[Joel Sing]

On Wed, 1 Oct 2014, Stuart Henderson wrote:
 Over the coming months, web browsers will progressively start to first
 warn for certificate chains including SHA-1 hashes, then treat them
 as insecure (including disabling certain content - scripts etc).
 Chrome are initially doing this for certs expiring after Jan 2017,
 but will progressively slide it forward to certs expiring after
 Jan 2016.

 Since my previous attempt to at least show this in ssl(8) examples
 for openssl req a few months ago, I've spent some time digging for
 where the defaults are set in the code as a nicer place to set sane
 values, but haven't tracked it down yet. Would it be OK to set it
 in the default config for now? (or does anyone have an idea of where
 in the code this comes from?)

Welcome to libkitchensink...

I'd need to quadruple check, however this should come from openssl/req.c 
do_X509_sign() being called with a NULL digest, which calls openssl/req.c 
do_sign_init() with a NULL md, which calls crypto/evp/m_sigver.c 
EVP_DigestSignInit() with md being NULL, which calls crypto/evp/m_sigver.c 
do_sigver_init() with type being NULL, which results in:

if (type == NULL) {
int def_nid;
if (EVP_PKEY_get_default_digest_nid(pkey, def_nid)  0)
type = EVP_get_digestbynid(def_nid);
}

EVP_PKEY_get_default_digest_nid() returns the default digest associated with 
the given PKEY. Since you're using RSA, pkey_ctrl is implemented by 
crypto/rsa/rsa_ameth.c rsa_pkey_ctrl(), which has:

case ASN1_PKEY_CTRL_DEFAULT_MD_NID:
*(int *)arg2 = NID_sha1;
return 1;

Catch all that?

To make SHA-256 the default for RSA, we'd have to change that from NID_sha1 to 
NID_sha256...

(and yes, clearly I've spent too much time in this code base recently... :)

 Index: openssl.cnf
 ===
 RCS file: /cvs/src/lib/libcrypto/openssl.cnf,v
 retrieving revision 1.1
 diff -u -p -r1.1 openssl.cnf
 --- openssl.cnf   11 Apr 2014 22:51:53 -  1.1
 +++ openssl.cnf   30 Sep 2014 22:42:53 -
 @@ -7,7 +7,8 @@

  
  [ req ]
 -default_bits = 1024
 +default_bits = 2048
 +default_md   = sha256
  default_keyfile  = privkey.pem
  distinguished_name   = req_distinguished_name
  attributes   = req_attributes



-- 

Action without study is fatal. Study without action is futile.
-- Mary Ritter Beard

Re: openssl.cnf req defaults - default_md sha256

[Joel Sing]

On Wed, 1 Oct 2014, Joel Sing wrote:
 On Wed, 1 Oct 2014, Stuart Henderson wrote:
  Over the coming months, web browsers will progressively start to first
  warn for certificate chains including SHA-1 hashes, then treat them
  as insecure (including disabling certain content - scripts etc).
  Chrome are initially doing this for certs expiring after Jan 2017,
  but will progressively slide it forward to certs expiring after
  Jan 2016.
 
  Since my previous attempt to at least show this in ssl(8) examples
  for openssl req a few months ago, I've spent some time digging for
  where the defaults are set in the code as a nicer place to set sane
  values, but haven't tracked it down yet. Would it be OK to set it
  in the default config for now? (or does anyone have an idea of where
  in the code this comes from?)

 Welcome to libkitchensink...

 I'd need to quadruple check, however this should come from openssl/req.c
 do_X509_sign() being called with a NULL digest, which calls openssl/req.c
 do_sign_init() with a NULL md, which calls crypto/evp/m_sigver.c
 EVP_DigestSignInit() with md being NULL, which calls crypto/evp/m_sigver.c
 do_sigver_init() with type being NULL, which results in:

 if (type == NULL) {
 int def_nid;
 if (EVP_PKEY_get_default_digest_nid(pkey, def_nid)  0)
 type = EVP_get_digestbynid(def_nid);
 }

 EVP_PKEY_get_default_digest_nid() returns the default digest associated
 with the given PKEY. Since you're using RSA, pkey_ctrl is implemented by
 crypto/rsa/rsa_ameth.c rsa_pkey_ctrl(), which has:

 case ASN1_PKEY_CTRL_DEFAULT_MD_NID:
 *(int *)arg2 = NID_sha1;
 return 1;

 Catch all that?

 To make SHA-256 the default for RSA, we'd have to change that from NID_sha1
 to NID_sha256...

I should also add that the other obvious/easy fix is to initialise digest in 
openssl/req.c to the SHA-256 EVP. That only changes 'openssl req' though.

 (and yes, clearly I've spent too much time in this code base recently... :)

  Index: openssl.cnf
  ===
  RCS file: /cvs/src/lib/libcrypto/openssl.cnf,v
  retrieving revision 1.1
  diff -u -p -r1.1 openssl.cnf
  --- openssl.cnf 11 Apr 2014 22:51:53 -  1.1
  +++ openssl.cnf 30 Sep 2014 22:42:53 -
  @@ -7,7 +7,8 @@
 
   
   [ req ]
  -default_bits   = 1024
  +default_bits   = 2048
  +default_md = sha256
   default_keyfile= privkey.pem
   distinguished_name = req_distinguished_name
   attributes = req_attributes



-- 

Action without study is fatal. Study without action is futile.
-- Mary Ritter Beard

Re: openssl.cnf req defaults - default_md sha256

[Joel Sing]

 I should also add that the other obvious/easy fix is to initialise digest
 in openssl/req.c to the SHA-256 EVP. That only changes 'openssl req'
 though.

  (and yes, clearly I've spent too much time in this code base recently...
  :)
 
   Index: openssl.cnf
   ===
   RCS file: /cvs/src/lib/libcrypto/openssl.cnf,v
   retrieving revision 1.1
   diff -u -p -r1.1 openssl.cnf
   --- openssl.cnf   11 Apr 2014 22:51:53 -  1.1
   +++ openssl.cnf   30 Sep 2014 22:42:53 -
   @@ -7,7 +7,8 @@
  

[ req ]
   -default_bits = 1024
   +default_bits = 2048
   +default_md   = sha256
default_keyfile  = privkey.pem
distinguished_name   = req_distinguished_name
attributes   = req_attributes

The following does this, however note that the default_bits of 1024 from
openssl.cnf trumps the 2048 in the define... we probably should also stop
making EVP_des_ede3_cbc() the default cipher...

Index: req.c
===
RCS file: /cvs/src/usr.bin/openssl/req.c,v
retrieving revision 1.2
diff -u -p -r1.2 req.c
--- req.c   28 Aug 2014 14:23:52 -  1.2
+++ req.c   1 Oct 2014 08:59:54 -
@@ -97,7 +97,7 @@
 #define STRING_MASKstring_mask
 #define UTF8_INutf8
 
-#define DEFAULT_KEY_LENGTH 512
+#define DEFAULT_KEY_LENGTH 2048
 #define MIN_KEY_LENGTH 384
 
 
@@ -187,6 +187,7 @@ req_main(int argc, char **argv)
 #ifndef OPENSSL_NO_DES
cipher = EVP_des_ede3_cbc();
 #endif
+   digest = EVP_sha256();
 
infile = NULL;
outfile = NULL;

-- 

Action without study is fatal. Study without action is futile.
-- Mary Ritter Beard

Re: libcrypto: use libc string fns

[Joel Sing]

On Fri, 31 Oct 2014, Ted Unangst wrote:
 Don't need BUF_ and its NULL arg handling here.

Looks like you need to cvs up... beck@ nuked these and put BUF_strdup() under 
LIBRESSL_INTERNAL about two weeks ago. He missed the comment (second chunk) 
though.

 Index: x509/x509_trs.c
 ===
 RCS file: /cvs/src/lib/libssl/src/crypto/x509/x509_trs.c,v
 retrieving revision 1.16
 diff -u -p -r1.16 x509_trs.c
 --- x509/x509_trs.c   28 Sep 2014 10:52:59 -  1.16
 +++ x509/x509_trs.c   31 Oct 2014 04:13:14 -
 @@ -57,6 +57,7 @@
   */

  #include stdio.h
 +#include string.h

  #include openssl/err.h
  #include openssl/x509v3.h
 @@ -202,7 +203,7 @@ X509_TRUST_add(int id, int flags, int (*
   if (trtmp-flags  X509_TRUST_DYNAMIC_NAME)
   free(trtmp-name);
   /* dup supplied name */
 - if ((trtmp-name = BUF_strdup(name)) == NULL)
 + if ((trtmp-name = strdup(name)) == NULL)
   goto err;
   /* Keep the dynamic flag of existing entry */
   trtmp-flags = X509_TRUST_DYNAMIC;
 Index: x509v3/v3_addr.c
 ===
 RCS file: /cvs/src/lib/libssl/src/crypto/x509v3/v3_addr.c,v
 retrieving revision 1.13
 diff -u -p -r1.13 v3_addr.c
 --- x509v3/v3_addr.c  13 Jul 2014 16:03:10 -  1.13
 +++ x509v3/v3_addr.c  31 Oct 2014 04:12:45 -
 @@ -1019,7 +1019,7 @@ v2i_IPAddrBlocks(const struct v3_ext_met
   length = length_from_afi(afi);

   /*
 -  * Handle SAFI, if any, and BUF_strdup() so we can 
 null-terminate
 +  * Handle SAFI, if any, and strdup() so we can null-terminate
* the other input values.
*/
   if (safi != NULL) {
 Index: store/str_lib.c
 ===
 RCS file: /cvs/src/lib/libssl/src/crypto/store/str_lib.c,v
 retrieving revision 1.10
 diff -u -p -r1.10 str_lib.c
 --- store/str_lib.c   10 Jul 2014 22:45:58 -  1.10
 +++ store/str_lib.c   31 Oct 2014 04:12:30 -
 @@ -1341,7 +1341,7 @@ STORE_ATTR_INFO_set_cstr(STORE_ATTR_INFO
   return 0;
   }
   if (!ATTR_IS_SET(attrs, code)) {
 - if ((attrs-values[code].cstring = BUF_strndup(cstr, 
 cstr_size)))
 + if ((attrs-values[code].cstring = strndup(cstr, cstr_size)))
   return 1;
   STOREerr(STORE_F_STORE_ATTR_INFO_SET_CSTR,
   ERR_R_MALLOC_FAILURE);

-- 

Action without study is fatal. Study without action is futile.
-- Mary Ritter Beard

Re: ressl: two way fds extention

[Joel Sing]

On Thu, 30 Oct 2014, Jan Klemkow wrote:
 Hello,

 This diff enables libressl to use two file descriptors for read and
 write.  This is feature is necessary for communication over two pipes
 like in the UCSPI protocol [1].  resslc[3] is a general ssl-client.

 +---+ ++ ++

 | tcpserver | -- | resslc | -- | client |
 |
 |   | -- || -- ||

 +---+ ++ ++

 This diff adds a new function ressl_set_fds() to set a separate file
 descriptors for read and write inside of the ressl context structure.
 The function ressl_connect_socket() sets the read and write file
 descriptors if their were set before.  I also adapt the related manpage.

How about this API - instead of having a (now) tls_set_fds() function and then
calling tls_connect_socket(), you call tls_connect_fds() directly if you need
that functionality?

Also, should tls_close() handle the read/write file descriptors or leave them
for the caller to close?

 This approach may not the best to get this feature.  I am open to every
 idea that solves this problem in a better way.  I am not sure whether it
 is nessacery to touch shlib_version.  So, I leave it untouched.

When adding new symbols shlib_version should be bumped, but whoever commits
would handle that.

Index: tls.c
===
RCS file: /cvs/src/lib/libtls/tls.c,v
retrieving revision 1.1
diff -u -p -r1.1 tls.c
--- tls.c   31 Oct 2014 13:46:17 -  1.1
+++ tls.c   31 Oct 2014 16:20:52 -
@@ -217,6 +217,8 @@ tls_reset(struct tls *ctx)
ctx-ssl_conn = NULL;
ctx-ssl_ctx = NULL;
 
+   ctx-fd_read = -1;
+   ctx-fd_write = -1;
ctx-socket = -1;
 
ctx-err = 0;
Index: tls.h
===
RCS file: /cvs/src/lib/libtls/tls.h,v
retrieving revision 1.1
diff -u -p -r1.1 tls.h
--- tls.h   31 Oct 2014 13:46:17 -  1.1
+++ tls.h   31 Oct 2014 16:20:52 -
@@ -66,6 +66,8 @@ void tls_free(struct tls *ctx);
 
 int tls_accept_socket(struct tls *ctx, struct tls **cctx, int socket);
 int tls_connect(struct tls *ctx, const char *host, const char *port);
+int tls_connect_fds(struct tls *ctx, int fd_read, int fd_write,
+const char *hostname);
 int tls_connect_socket(struct tls *ctx, int s, const char *hostname);
 int tls_read(struct tls *ctx, void *buf, size_t buflen, size_t *outlen);
 int tls_write(struct tls *ctx, const void *buf, size_t buflen, size_t *outlen);
Index: tls_client.c
===
RCS file: /cvs/src/lib/libtls/tls_client.c,v
retrieving revision 1.1
diff -u -p -r1.1 tls_client.c
--- tls_client.c31 Oct 2014 13:46:17 -  1.1
+++ tls_client.c31 Oct 2014 16:20:52 -
@@ -123,6 +123,15 @@ err:
 int
 tls_connect_socket(struct tls *ctx, int socket, const char *hostname)
 {
+   ctx-socket = socket;
+
+   return tls_connect_fds(ctx, socket, socket, hostname);
+}
+
+int
+tls_connect_fds(struct tls *ctx, int fd_read, int fd_write,
+const char *hostname)
+{
union { struct in_addr ip4; struct in6_addr ip6; } addrbuf;
X509 *cert = NULL;
int ret;
@@ -132,7 +141,13 @@ tls_connect_socket(struct tls *ctx, int 
goto err;
}
 
-   ctx-socket = socket;
+   if (fd_read  0 || fd_write  0) {
+   tls_set_error(ctx, invalid file descriptors);
+   return (-1);
+   }
+
+   ctx-fd_read = fd_read;
+   ctx-fd_write = fd_write;
 
if ((ctx-ssl_ctx = SSL_CTX_new(SSLv23_client_method())) == NULL) {
tls_set_error(ctx, ssl context failure);
@@ -166,7 +181,8 @@ tls_connect_socket(struct tls *ctx, int 
tls_set_error(ctx, ssl connection failure);
goto err;
}
-   if (SSL_set_fd(ctx-ssl_conn, ctx-socket) != 1) {
+   if (SSL_set_rfd(ctx-ssl_conn, ctx-fd_read) != 1 ||
+   SSL_set_wfd(ctx-ssl_conn, ctx-fd_write) != 1) {
tls_set_error(ctx, ssl file descriptor failure);
goto err;
}
Index: tls_internal.h
===
RCS file: /cvs/src/lib/libtls/tls_internal.h,v
retrieving revision 1.1
diff -u -p -r1.1 tls_internal.h
--- tls_internal.h  31 Oct 2014 13:46:17 -  1.1
+++ tls_internal.h  31 Oct 2014 16:20:52 -
@@ -53,6 +53,8 @@ struct tls {
int err;
char *errmsg;
 
+   int fd_read;
+   int fd_write;
int socket;
 
SSL *ssl_conn;

-- 

Action without study is fatal. Study without action is futile.
-- Mary Ritter Beard

Re: ressl: two way fds extention

[Joel Sing]

On Sat, 1 Nov 2014, Jan Klemkow wrote:
 On Fri, Oct 31, 2014 at 09:18:26PM -0700, Doug Hogan wrote:
  On Sat, Nov 01, 2014 at 03:07:24AM +0100, Jan Klemkow wrote:
   Index: tls_client.c
   ===
   RCS file: /cvs/src/lib/libtls/tls_client.c,v
   retrieving revision 1.1
   diff -u -p -r1.1 tls_client.c
   --- tls_client.c  31 Oct 2014 13:46:17 -  1.1
   +++ tls_client.c  1 Nov 2014 01:50:56 -
   @@ -123,6 +123,13 @@ err:
int
tls_connect_socket(struct tls *ctx, int socket, const char *hostname)
{
   + return tls_connect_fds(ctx, socket, socket, hostname);
   +}
 
  This changes the behavior of tls_connect_socket() and tls_connect().
  Joel's diff set ctx-socket = socket before calling tls_connect_fds() so
  it would behave the same.  When you call tls_close(ctx), it will close
  ctx-socket in the existing code and Joel's diff.
 
  I don't think you want to change the semantics like this.  I think
  either tls_connect_fds() is the special case where you need to manually
  close the sockets or tls_close() should close everything.  With the
  above change, even people calling tls_connect() will need to save one of
  the fd_(read|write) before calling tls_close() and then close the fd
  afterward.

 Oh, sorry.  This is my fault.  I experimented a little bit and forgot
 this part.  I corrected it in the diff below.  Additionally I add some
 extra information about the closing behavior inside of the manpage.

Thanks. I've just committed a slightly different version of this.

 Thanks,
 Jan

 Index: tls.c
 ===
 RCS file: /cvs/src/lib/libtls/tls.c,v
 retrieving revision 1.1
 diff -u -p -r1.1 tls.c
 --- tls.c 31 Oct 2014 13:46:17 -  1.1
 +++ tls.c 1 Nov 2014 11:59:36 -
 @@ -217,6 +217,8 @@ tls_reset(struct tls *ctx)
   ctx-ssl_conn = NULL;
   ctx-ssl_ctx = NULL;

 + ctx-fd_read = -1;
 + ctx-fd_write = -1;
   ctx-socket = -1;

   ctx-err = 0;
 @@ -290,6 +292,8 @@ tls_close(struct tls *ctx)
   tls_set_error(ctx, close);
   goto err;
   }
 + ctx-fd_read = -1;
 + ctx-fd_write = -1;
   ctx-socket = -1;
   }

 Index: tls.h
 ===
 RCS file: /cvs/src/lib/libtls/tls.h,v
 retrieving revision 1.1
 diff -u -p -r1.1 tls.h
 --- tls.h 31 Oct 2014 13:46:17 -  1.1
 +++ tls.h 1 Nov 2014 11:59:36 -
 @@ -66,6 +66,8 @@ void tls_free(struct tls *ctx);

  int tls_accept_socket(struct tls *ctx, struct tls **cctx, int socket);
  int tls_connect(struct tls *ctx, const char *host, const char *port);
 +int tls_connect_fds(struct tls *ctx, int fd_read, int fd_write,
 +const char *hostname);
  int tls_connect_socket(struct tls *ctx, int s, const char *hostname);
  int tls_read(struct tls *ctx, void *buf, size_t buflen, size_t *outlen);
  int tls_write(struct tls *ctx, const void *buf, size_t buflen, size_t
 *outlen); Index: tls_client.c
 ===
 RCS file: /cvs/src/lib/libtls/tls_client.c,v
 retrieving revision 1.1
 diff -u -p -r1.1 tls_client.c
 --- tls_client.c  31 Oct 2014 13:46:17 -  1.1
 +++ tls_client.c  1 Nov 2014 11:59:36 -
 @@ -123,6 +123,15 @@ err:
  int
  tls_connect_socket(struct tls *ctx, int socket, const char *hostname)
  {
 + ctx-socket = socket;
 +
 + return tls_connect_fds(ctx, socket, socket, hostname);
 +}
 +
 +int
 +tls_connect_fds(struct tls *ctx, int fd_read, int fd_write,
 +const char *hostname)
 +{
   union { struct in_addr ip4; struct in6_addr ip6; } addrbuf;
   X509 *cert = NULL;
   int ret;
 @@ -132,7 +141,13 @@ tls_connect_socket(struct tls *ctx, int
   goto err;
   }

 - ctx-socket = socket;
 + if (fd_read  0 || fd_write  0) {
 + tls_set_error(ctx, invalid file descriptors);
 + return (-1);
 + }
 +
 + ctx-fd_read = fd_read;
 + ctx-fd_write = fd_write;

   if ((ctx-ssl_ctx = SSL_CTX_new(SSLv23_client_method())) == NULL) {
   tls_set_error(ctx, ssl context failure);
 @@ -166,7 +181,8 @@ tls_connect_socket(struct tls *ctx, int
   tls_set_error(ctx, ssl connection failure);
   goto err;
   }
 - if (SSL_set_fd(ctx-ssl_conn, ctx-socket) != 1) {
 + if (SSL_set_rfd(ctx-ssl_conn, ctx-fd_read) != 1 ||
 + SSL_set_wfd(ctx-ssl_conn, ctx-fd_write) != 1) {
   tls_set_error(ctx, ssl file descriptor failure);
   goto err;
   }
 Index: tls_init.3
 ===
 RCS file: /cvs/src/lib/libtls/tls_init.3,v
 retrieving revision 1.1
 diff -u -p -r1.1 tls_init.3
 --- tls_init.331 Oct 2014 13:46:17 -  1.1
 +++ tls_init.31 Nov 2014 11:59:36 -
 @@ 

Re: libtls future

[Joel Sing]

On Thu, 6 Nov 2014, Daniel wrote:
 Looking over libtls it struck me that this is the best-designed TLS
 API I've ever seen, so it was a bit disheartening to look at the code
 and find that it was mainly just wrapping libssl and abstracting away
 its fragile, haphazard design choices. Though even just this is
 obviously already an unconditional good, are there plans for enough of
 libssl to be split off, cleaned up, and rolled directly into libtls so
 that the libtls - libssl dependency can be broken for good?

The short answer is yes, that's the long term ideal.

The longer answer is that there is still a huge amount of software that uses 
libssl and even if we took the time to write a brand new TLS implementation, 
it would be of little benefit until everything started using it. The amount 
of effort required to implement a new TLS from scratch is significant. Same 
goes with porting existing software to use it.

For the time being we're far better off investing time into fixing libssl as 
much as we possibly can (while being held to its existing API), while also 
providing a new TLS API that things can start using now. Hopefully we'll 
eventually get to the point where libtls stands on its own...
-- 

Action without study is fatal. Study without action is futile.
-- Mary Ritter Beard

Re: base apache and HonorCipherOrder

[Joel Sing]

On Mon, 8 Jul 2013, Damien Miller wrote:
 On Sun, 7 Jul 2013, Aaron Stellman wrote:
  On Tue, Apr 23, 2013 at 09:08:19AM +0200, Otto Moerbeek wrote:
   If there is any interest, I might add the manual stuff, get ok's and
   commit it.
 
  I find it useful to have SSLHonorCipherOrder in OpenBSD's apache.

 More than that, AFAIK it is necessary to mitigate some of the TLS crypto
 attacks. IMO it is well worth having.

 It would also be good if someone could make a patch to enable ECDHE cipher
 suites in Apache-1.x.
 This nginx patch is a good reference to what needs to 
 be done:

 http://hg.nginx.org/nginx/rev/0832a6997227

The following should do the trick...

$ openssl s_client -connect localhost:443 21 /dev/null | grep Cipher is
New, TLSv1/SSLv3, Cipher is ECDHE-RSA-AES256-GCM-SHA384

Index: conf/httpd.conf
===
RCS file: /cvs/src/usr.sbin/httpd/conf/httpd.conf,v
retrieving revision 1.26
diff -u -p -u -p -r1.26 httpd.conf
--- conf/httpd.conf 3 Jun 2009 18:28:21 -   1.26
+++ conf/httpd.conf 11 Jul 2013 15:28:21 -
@@ -1034,6 +1034,11 @@ SSLEngine on
 #   List the ciphers that the client is permitted to negotiate.
 #   See the mod_ssl documentation for a complete list.
 #SSLCipherSuite ALL:!ADH:RC4+RSA:+HIGH:+MEDIUM:+LOW:+SSLv2:+EXP
+
+#   SSL ECDH Curve:
+#   Named curve to use when generating ephemeral EC keys for an
+#   ECDHE-based cipher suite.
+#SSLECDHCurve prime256v1
 
 #   Server Certificate:
 #   Point SSLCertificateFile at a PEM encoded certificate.  If
Index: conf/httpd.conf-dist
===
RCS file: /cvs/src/usr.sbin/httpd/conf/httpd.conf-dist,v
retrieving revision 1.20
diff -u -p -u -p -r1.20 httpd.conf-dist
--- conf/httpd.conf-dist1 Apr 2009 06:47:34 -   1.20
+++ conf/httpd.conf-dist11 Jul 2013 15:28:21 -
@@ -1045,6 +1045,11 @@ SSLEngine on
 #   See the mod_ssl documentation for a complete list.
 SSLCipherSuite ALL:!ADH:!EXPORT56:RC4+RSA:+HIGH:+MEDIUM:+LOW:+SSLv2:+EXP:+eNULL
 
+#   SSL ECDH Curve:
+#   Named curve to use when generating ephemeral EC keys for an
+#   ECDHE-based cipher suite.
+SSLECDHCurve prime256v1
+
 #   Server Certificate:
 #   Point SSLCertificateFile at a PEM encoded certificate.  If
 #   the certificate is encrypted, then you will be prompted for a
Index: src/modules/ssl/mod_ssl.c
===
RCS file: /cvs/src/usr.sbin/httpd/src/modules/ssl/mod_ssl.c,v
retrieving revision 1.11
diff -u -p -u -p -r1.11 mod_ssl.c
--- src/modules/ssl/mod_ssl.c   11 Jul 2013 12:41:52 -  1.11
+++ src/modules/ssl/mod_ssl.c   11 Jul 2013 15:28:21 -
@@ -113,6 +113,9 @@ static command_rec ssl_config_cmds[] = {
 AP_ALL_CMD(CipherSuite, TAKE1,
Colon-delimited list of permitted SSL Ciphers 
(`XXX:...:XXX' - see manual))
+AP_SRV_CMD(ECDHCurve, TAKE1,
+   Name of ECDH curve to use for ephemeral EC keys 
+   (`curve' - see manual))
 AP_SRV_CMD(CertificateFile, TAKE1,
SSL Server Certificate file 
(`/path/to/file' - PEM or DER encoded))
Index: src/modules/ssl/mod_ssl.h
===
RCS file: /cvs/src/usr.sbin/httpd/src/modules/ssl/mod_ssl.h,v
retrieving revision 1.22
diff -u -p -u -p -r1.22 mod_ssl.h
--- src/modules/ssl/mod_ssl.h   11 Jul 2013 12:41:52 -  1.22
+++ src/modules/ssl/mod_ssl.h   11 Jul 2013 15:28:22 -
@@ -514,6 +514,7 @@ typedef struct {
 char*szCACertificateFile;
 char*szLogFile;
 char*szCipherSuite;
+char*szECDHCurve;
 FILE*fileLogFile;
 int  nLogLevel;
 BOOL cipher_server_pref;
@@ -592,6 +593,7 @@ const char  *ssl_cmd_SSLRandomSeed(cmd_p
 const char  *ssl_cmd_SSLEngine(cmd_parms *, char *, int);
 const char  *ssl_cmd_SSLHonorCipherOrder(cmd_parms *, char *, int);
 const char  *ssl_cmd_SSLCipherSuite(cmd_parms *, SSLDirConfigRec *, char *);
+const char  *ssl_cmd_SSLECDHCurve(cmd_parms *, char *, char *);
 const char  *ssl_cmd_SSLCertificateFile(cmd_parms *, char *, char *);
 const char  *ssl_cmd_SSLCertificateKeyFile(cmd_parms *, char *, char *);
 const char  *ssl_cmd_SSLCertificateChainFile(cmd_parms *, char *, char *);
Index: src/modules/ssl/ssl_engine_config.c
===
RCS file: /cvs/src/usr.sbin/httpd/src/modules/ssl/ssl_engine_config.c,v
retrieving revision 1.20
diff -u -p -u -p -r1.20 ssl_engine_config.c
--- src/modules/ssl/ssl_engine_config.c 11 Jul 2013 12:41:52 -  1.20
+++ src/modules/ssl/ssl_engine_config.c 11 Jul 2013 15:28:22 -
@@ -196,6 +196,7 @@ void *ssl_config_server_create(pool *p, 
 sc-szCertificateChain = NULL;
 sc-szLogFile  = NULL;
 sc-szCipherSuite  = NULL;
+sc-szECDHCurve

Re: threaded prof signals

[Joel Sing]

On Fri, 4 Oct 2013, Philip Guenther wrote:
 On Fri, 16 Aug 2013, Ted Unangst wrote:
  As per http://research.swtch.com/macpprof
 
  We deliver all prof signals to the main thread, which is unlikely to
  result in accurate profiling info. I think the diff below fixes things.

 How about we take an idea from FreeBSD and have hardclock() just set a
 flag on the thread and then have the thread send SIGPROF (and SIGVTALRM)
 to itself from userret().  The signal gets sent by the thread itself right
 before it checks for pending signals when returning to userspace, so
 that's absolutely as soon as possible, and it's done from process context
 instead of from softclock by a timeout, so no which CPU are we on?
 issues that might delay the delivery.

 Ted, Joel, does this solve your profiling problems?

Nope.

I like the concept of this diff, however it is not sufficient to solve the 
multithreaded profiling problem. The reason for this is that you're still 
sending SPROCESS signals, which are delivered to the main thread unless 
another thread has diverted the signal. This means that all of the profiling 
signals end up on the main thread, not the thread that was consuming CPU 
time.

This can be fixed by either using ptsignal(p, SIG{PROF,VTALARM}, STHREAD) 
instead of psignal(), or by including your other diff that changes the signal 
delivery within ptsignal - regardless, I think the previous diff is worth 
pursuing since it potentially reduces the number of signal related context 
switches.

P.S. The diff below misses a chunk from kern_time.c.

 (No, this had nothing at all to do with my splitting the process and
 thread flags, what could possibly make you think that...)


 Philip

 Index: sys/sys/proc.h
 ===
 RCS file: /cvs/src/sys/sys/proc.h,v
 retrieving revision 1.170
 diff -u -p -r1.170 proc.h
 --- sys/sys/proc.h22 Sep 2013 17:28:33 -  1.170
 +++ sys/sys/proc.h4 Oct 2013 01:00:18 -
 @@ -209,8 +209,6 @@ struct process {

   struct  timespec ps_start;  /* starting time. */
   struct  timeout ps_realit_to;   /* real-time itimer trampoline. */
 - struct  timeout ps_virt_to; /* virtual itimer trampoline. */
 - struct  timeout ps_prof_to; /* prof itimer trampoline. */

   int ps_refcnt;  /* Number of references. */
  };
 @@ -362,6 +361,8 @@ struct proc {
   * These flags are per-thread and kept in p_flag
   */
  #define  P_INKTR 0x01/* In a ktrace op, don't 
 recurse */
 +#define  P_PROFPEND  0x02/* SIGPROF needs to be posted */
 +#define  P_ALRMPEND  0x04/* SIGVTALRM needs to be posted 
 */
  #define  P_SIGSUSPEND0x08/* Need to restore 
 before-suspend mask*/
  #define  P_SELECT0x40/* Selecting; wakeup/waiting 
 danger. */
  #define  P_SINTR 0x80/* Sleep is interruptible. */
 @@ -380,7 +381,8 @@ struct proc {
  #define P_CPUPEG 0x4000  /* Do not move to another cpu. */

  #define  P_BITS \
 -(\20\01INKTR\04SIGSUSPEND\07SELECT\010SINTR\012SYSTEM \
 +(\20\01INKTR\02PROFPEND\03ALRMPEND\04SIGSUSPEND\07SELECT \
 + \010SINTR\012SYSTEM \
   \013TIMEOUT\016WEXIT\020OWEUPC\024SUSPSINGLE \
   \025NOZOMBIE\027SYSTRACE\030CONTINUED\033THREAD \
   \034SUSPSIG\035SOFTDEP\036STOPPED\037CPUPEG)
 Index: sys/sys/resourcevar.h
 ===
 RCS file: /cvs/src/sys/sys/resourcevar.h,v
 retrieving revision 1.16
 diff -u -p -r1.16 resourcevar.h
 --- sys/sys/resourcevar.h 3 Jun 2013 16:55:22 -   1.16
 +++ sys/sys/resourcevar.h 4 Oct 2013 01:00:18 -
 @@ -68,8 +68,5 @@ struct plimit *limcopy(struct plimit *);
  void limfree(struct plimit *);

  void  ruadd(struct rusage *, struct rusage *);
 -
 -void virttimer_trampoline(void *);
 -void proftimer_trampoline(void *);
  #endif
  #endif   /* !_SYS_RESOURCEVAR_H_ */
 Index: sys/kern/kern_clock.c
 ===
 RCS file: /cvs/src/sys/kern/kern_clock.c,v
 retrieving revision 1.82
 diff -u -p -r1.82 kern_clock.c
 --- sys/kern/kern_clock.c 13 Aug 2013 05:52:23 -  1.82
 +++ sys/kern/kern_clock.c 4 Oct 2013 01:00:19 -
 @@ -144,40 +144,11 @@ initclocks(void)
  /*
   * hardclock does the accounting needed for ITIMER_PROF and
 ITIMER_VIRTUAL. * We don't want to send signals with psignal from hardclock
 because it makes - * MULTIPROCESSOR locking very complicated. Instead we
 use a small trick - * to send the signals safely and without blocking too
 many interrupts - * while doing that (signal handling can be heavy).
 - *
 - * hardclock detects that the itimer has expired, and schedules a timeout
 - * to deliver the signal. This works because of the following reasons:
 - *  - The timeout can be scheduled with a 1 tick time because we're
 - *doing 

Re: duid support to dump

[Joel Sing]

On Wed, 27 Nov 2013, Cody Cutler wrote:
 hello, the following is my attempt to add duid support to dump.
 thanks!

From a quick glance, you should be able to use opendev(3) to open the disk 
device (rather than using open), in which case you'll get DUID handling for 
free. This also avoids the TOCTOU issues associated with attempting to map a 
DUID back to a name. Even if this is not possible, then it would be 
preferable to use opendev(3) to get the realname (as is done in fsck(8) from 
memory), rather than the hand rolled version below.

 Index: Makefile
 ===
 RCS file: /cvs/src/sbin/dump/Makefile,v
 retrieving revision 1.11
 diff -p -u -r1.11 Makefile
 --- Makefile  6 Jan 2013 21:59:28 -   1.11
 +++ Makefile  26 Nov 2013 17:59:50 -
 @@ -12,6 +12,8 @@
  #TDEBUG  trace out the process forking

  PROG=dump
 +LDADD=   -lutil
 +DPADD=   ${LIBUTIL}
  LINKS=   ${BINDIR}/dump ${BINDIR}/rdump
  CFLAGS+=-DRDUMP
  SRCS=itime.c main.c optr.c dumprmt.c tape.c traverse.c
 Index: dump.h
 ===
 RCS file: /cvs/src/sbin/dump/dump.h,v
 retrieving revision 1.17
 diff -p -u -r1.17 dump.h
 --- dump.h11 Jun 2013 16:42:04 -  1.17
 +++ dump.h26 Nov 2013 17:59:50 -
 @@ -86,6 +86,7 @@ int tp_bshift;  /* log2(TP_BSIZE) */
  /* operator interface functions */
  void broadcast(char *message);
  time_t   do_stats(void);
 +int  duidtorawname(char *duid, char *buf, size_t s);
  void lastdump(int arg);  /* int should be char */
  void msg(const char *fmt, ...)
  __attribute__((__format__ (printf, 1, 2)));
 Index: main.c
 ===
 RCS file: /cvs/src/sbin/dump/main.c,v
 retrieving revision 1.46
 diff -p -u -r1.46 main.c
 --- main.c16 Apr 2013 18:17:39 -  1.46
 +++ main.c26 Nov 2013 17:59:50 -
 @@ -31,6 +31,7 @@
   */

  #include sys/param.h
 +#include sys/dkio.h
  #include sys/mount.h
  #include sys/stat.h
  #include sys/time.h
 @@ -51,6 +52,7 @@
  #include string.h
  #include time.h
  #include unistd.h
 +#include util.h

  #include dump.h
  #include pathnames.h
 @@ -78,6 +80,7 @@ static void usage(void);
  int
  main(int argc, char *argv[])
  {
 + char dbuf[MAXPATHLEN];
   ino_t ino;
   int dirty;
   union dinode *dp;
 @@ -204,7 +207,9 @@ main(int argc, char *argv[])
   for (i = 0; i  argc; i++) {
   struct stat sb;

 - if (lstat(argv[i], sb) == -1) {
 + if (isduid(argv[i], 0))
 + break;
 + else if (lstat(argv[i], sb) == -1) {
   msg(Cannot lstat %s: %s\n, argv[i], strerror(errno));
   exit(X_STARTUP);
   }
 @@ -321,6 +326,10 @@ main(int argc, char *argv[])
*  the special name missing the leading '/',
*  the file system name with or without the leading '/'.
*/
 + if (isduid(disk, 0)) {
 + duidtorawname(disk, dbuf, sizeof(dbuf));
 + disk = dbuf;
 + }
   if (!statfs(disk, fsbuf)  !strcmp(fsbuf.f_mntonname, disk)) {
   /* mounted disk? */
   disk = rawname(fsbuf.f_mntfromname);
 @@ -604,17 +613,42 @@ sig(int signo)
   }
  }

 +int
 +duidtorawname(char *duid, char *buf, size_t s)
 +{
 + struct dk_diskmap dk;
 + int fd, ret;
 +
 + if ((fd = open(/dev/diskmap, O_RDONLY))  0)
 + return 0;
 +
 + strlcpy(buf, duid, s);
 + dk.fd = fd;
 + dk.device = buf;
 + dk.flags = 0;
 + // buf is filled with raw dev
 + ret = ioctl(fd, DIOCMAP, dk);
 + close(fd);
 +
 + return (ret  0) ? 0 : 1;
 +}
 +
  char *
  rawname(char *cp)
  {
   static char rawbuf[MAXPATHLEN];
 - char *dp = strrchr(cp, '/');
 -
 - if (dp == NULL)
 - return (NULL);
 - *dp = '\0';
 - (void)snprintf(rawbuf, sizeof(rawbuf), %s/r%s, cp, dp + 1);
 - *dp = '/';
 + char *dp;
 +
 + if (isduid(cp, 0)) {
 + duidtorawname(cp, rawbuf, sizeof(rawbuf));
 + } else {
 + dp = strrchr(cp, '/');
 + if (dp == NULL)
 + return (NULL);
 + *dp = '\0';
 + (void)snprintf(rawbuf, sizeof(rawbuf), %s/r%s, cp, dp + 1);
 + *dp = '/';
 + }
   return (rawbuf);
  }



-- 

Action without study is fatal. Study without action is futile.
-- Mary Ritter Beard

Re: 5.5 and dual-boot

[Joel Sing]

On Sat, 8 Mar 2014, Theo de Raadt wrote:
  I follow -current for several years but recently a thing puzzles me.
 
  My x200 is a dual-boot system (Seven/OpenBSD -current) and since (I
  think) the amd64/i386 installboot change, each time I upgrade via
  bsd.rd, I have to generate a new openbsd.pbr and  copy it to Seven.
 
  If I miss that, I get an ERR M and can't go further.
 
  First I was thinking about changes but I checked code and last
  modification on boot(8) was made 2 weeks ago (I did 2 updates since it).
 
  I used that setup for several years without any need to do that, what do
  I miss here, is it now normal ?

 The old installboot would reuse the same file, very carefully.

To be clear, the old amd64/i386 installboot never updated /boot - it only ever 
patched biosboot and installed it into the PBR. It was entirely up to the 
user (or the install script) to copy /usr/mdec/boot file to /boot - if you 
used cp or cat the old PBR may have continued to work, but there was no 
guarantee.

 Joel's new installboot does not do that, and chooses to be careful
 about handling a different problem instead.

 Can't easily solve both problems.  I think he should revert to the
 old method.
-- 

Action without study is fatal. Study without action is futile.
-- Mary Ritter Beard

Re: OpenBSD 4.8 RAID 0+1 or 1+0 or 5

[Joel Sing]

On Wednesday 16 February 2011, Steven R. Gerber wrote:
 Sorry for cross posting?
 I am trying to setup a decent RAID (0+1 or 1+0 or 5) and there SEEMS to
 be no approved method.  (4 disks -- I usually like stripe on top of
 mirrors.)
 I believe that I have done my homework.
 What are my options?

 softraid (bioctl) cannot handle stripe on mirrors:
 I can easily create 2 mirrors and they survive reboot.
 I can create stripe on those mirrors (works -- can create files), but it
 does not survive reboot.

Define does not survive reboot. I'm guessing that you probably mean fails
to automatically reassemble at boot, which is accurate - we do not currently
probe volumes that we have just assembled. Things should just work if you
manually assemble it after the mirrors are available. Note that this is not a
supported configuration, however it does seem to work - YMMV.

 Message is device not configured.

 Both ccd and RAIDframe are decprecated (FAQ 14.13):
  Software Options
  OpenBSD supports softraid(4), a framework supporting many kinds of I/O

 transformations, including RAID and encryption disciplines. Softraid(4)
 is managed using bioctl(8).

  OpenBSD also includes RAIDframe (raid(4), requires a custom kernel),

 and ccd(4) as historic ways of implementing RAID, but at this point
 OpenBSD does not suggest implementing either as a RAID solution for new
 installs or reinstalls.
 OpenBSD does not suggest implementing either
 Also, RAIDframe requires a custom kernel and we all know that GENERIC is
 preferred.

 RAID 5 is experimental (man bioctl):
  CAVEATS
   Use of the CRYPTO  RAID 4/5 disciplines are currently considered
   experimental.
 
  OpenBSD 4.9December 22, 2010

 OpenBSD 4.9

 Also, bioctl would not let me create a RAID 5 set:
   # bioctl -iv softraid0
   # bioctl -c 5 -l /dev/sd1a,/dev/sd2a,/dev/sd3a,/dev/sd4a softraid0
 bioctl: BIOCCREATERAID: Invalid argument
   # bioctl -iv softraid0
   # dmesg|tail
   sd11 at scsibus6 targ 0 lun 0: OPENBSD, SR RAID 0, 004 SCSI2 0/direct
 fixed
   sd11: 3815436MB, 512 bytes/sec, 7814014721 sec total
   sd11 detached
   scsibus6 detached
   sd10 detached
   scsibus5 detached
   sd9 detached
   scsibus4 detached
   softraid0: not part of the same volume
   softraid0: can't attach metadata type 0

You previously had a RAID 0 volume on some or all of these partitions, hence
the not part of the same volume and can't attach metadata type 0 messages
(softraid is refusing to make members of a RAID 0 volume into a RAID 5
volume). Either wipe the first 1MB or so of each partition (dd if=/dev/zero
of=/dev/rsd1a bs=1m count=1, etc) or use 'bioctl -C force ... '.
--

bReason is not automatic. Those who deny it cannot be conquered by it.
 Do not count on them. Leave them alone.b -- Ayn Rand

Re: Future of ccd(4) and raid(4)?

[Joel Sing]

On Friday 24 June 2011, Matthew Dempsky wrote:
 On Thu, Jun 23, 2011 at 7:29 PM, Kenneth R Westerback

 kwesterb...@rogers.com wrote:
  I use neither but know people claim to be using one or the other,
  but mostly raid(4), a.k.a. raidframe.

 Then it sounds like the solution is to subtly break them so we can
 smoke out these claimed users! ;)

  In particular until softraid
  can reliably be booted from, I think raid(4) will be useful to have.

 I don't think you can boot from raid(4) either though?

grep -i raid /usr/src/sys/arch/sparc64/stand/bootblk/*

No idea if it actually works or if you can on other architectures...
-- 

Reason is not automatic. Those who deny it cannot be conquered by it.
 Do not count on them. Leave them alone. -- Ayn Rand

Re: Future of ccd(4) and raid(4)?

[Joel Sing]

On Saturday 25 June 2011, Christian Weisgerber wrote:
 Matthew Dempsky matt...@dempsky.org wrote:
  What should be done about ccd(4) and raid(4)?  They both seem
  superseded in functionality by softraid(4), which also has much more
  developer interest and active development.

 Is softraid ready at all?  I thought it was experimental, under
 construction, incomplete, don't-use-unless-you-want-to-contribute
 code.

The RAID 0, RAID 1 and crypto disciplines have been stable for the better part 
of the last two years (RAID 1 got rebuild and hotspare support about 2 years 
ago). The remaining disciplines (RAID 4, RAID 5, RAID 6, AOE) are 
certainly experimental and incomplete.
-- 

Reason is not automatic. Those who deny it cannot be conquered by it.
 Do not count on them. Leave them alone. -- Ayn Rand

Re: Future of ccd(4) and raid(4)?

[Joel Sing]

On Friday 24 June 2011, Benny Lofgren wrote:
 On 2011-06-24 01.39, Matthew Dempsky wrote:
  What should be done about ccd(4) and raid(4)?  They both seem
  superseded in functionality by softraid(4), which also has much more
  developer interest and active development.

 Never used ccd(4) so can't comment on that, but RAIDframe (raid(4)) has
 a lot of functionality that is not yet implemented in softraid(4).

 It has (for good reason given that softraid(4) is in the works) received
 little developer attention and has a few bugs and other shortcomings.

 I've tried in the past to address those I've run up against, but I know
 there are probably more problems with it than is worth fixing (in
 particular I've had problems with very large disks and raid sets) so I
 have high hopes for softraid(4) in the future.

  Are there any users still using ccd(4) and/or raid(4) and unable to
  upgrade to softraid(4)?  Will anyone be up a creek if ccd(4)/raid(4)
  were removed?

 I for one will be up the worst of creeks if raid(4) was removed, that
 would force me to stay on 4.9 until softraid(4) have evolved enough
 (which I have no doubt will happen eventually), so please please don't
 remove raid(4) just yet. :-)

 My wish list for softraid(4) to enable me to say goodbye to RAIDframe
 is something like this (not exhaustive and in no particular order):

 - More complete RAID support overall, including
   - ability to tune stripe sizes

Easily doable - not sure about the benefit since MAXPHYS should be close to 
optimal.

   - parity initialization / rebuilding, preferrably with background
 mode

The RAID 4/5/6 disciplines are still lacking this (scrubbing), along with 
other things.

   - Hot spare support

We've had that for almost 2 years.

   - Better handling of stripe (disk) failures

Not sure what you're wanting here.

   - Better handling of recovery from failed stripes (ability to hot
 plug a replacement disk and rebuild on the spot for example)

We've had that for almost 2 years as well.

   - Full stripe writes for perfomance

Meaning?

   - Usable status reporting

Are you talking about error messages, or bioctl(8) output?

   - Stripe on stripe (on stripe ...) support to be able to build
 RAID 0+1 and RAID50 sets, as well as crypto on raid (this may
 work now, haven't tried lately)

This works, although is not officially supported at this stage.

   - RAID6 support (way way back in priority though)

 - Bootable/rootable raid sets (I know this is close now)

 - More consistent sdn unit allocation (perhaps this is achievable
   with DUID, I haven't had time to explore that yet)

sd(4) unit allocation will always be inconsistent and unpredicatable - DUIDs 
will let you avoid this entirely.

 - Probably other small features as well, that I'll probably think
   of the moment I've sent this mail off...


 Regards,
 /Benny
-- 

Reason is not automatic. Those who deny it cannot be conquered by it.
 Do not count on them. Leave them alone. -- Ayn Rand

dkcsum: do not sleep whilst walking alldevs

[Joel Sing]

Currently dkcsumattach() walks the alldevs list whilst performing
operations that will sleep. This is rather bad if the list happens to be
modified (i.e. a device detachs) whilst it is sleeping.

The following diff resolves this issue. The process is very similar to
that used for softraid - we maintain a list of disks that we have checked
and each time we do something that may have slept we restart the
scan from the top of the list.

A couple of other things come along for the ride:

1. We can walk disklist rather than walking alldevs.

2. We can benefit from the more recent changes to disk_attach() and
use the devno that is stored in the disk struct.

3. The DKF_NOREADLABEL flag is used to skip disks to dkcsum, rather
than using dev_rawpart() - the list should be similar and we already
exclude cd(4) and fd(4) devices. If we do this dev_rawpart() can probably
be removed.

I have lightly tested this, however further testing (on i386 and amd64)
would be appreciated.

Comments or oks?

Index: arch/i386/i386/dkcsum.c
===
RCS file: /cvs/src/sys/arch/i386/i386/dkcsum.c,v
retrieving revision 1.29
diff -u -p -r1.29 dkcsum.c
--- arch/i386/i386/dkcsum.c 16 Apr 2011 03:21:15 -  1.29
+++ arch/i386/i386/dkcsum.c 2 Jul 2011 16:01:57 -
@@ -33,8 +33,10 @@
 #include sys/buf.h
 #include sys/conf.h
 #include sys/device.h
+#include sys/disk.h
 #include sys/disklabel.h
 #include sys/fcntl.h
+#include sys/malloc.h
 #include sys/proc.h
 #include sys/reboot.h
 #include sys/stat.h
@@ -44,16 +46,23 @@
 
 #include lib/libz/zlib.h
 
-dev_t dev_rawpart(struct device *);/* XXX */
-
 extern u_int32_t bios_cksumlen;
 extern bios_diskinfo_t *bios_diskinfo;
 extern dev_t bootdev;
 
+struct dkcsum_disk {
+   dev_t dkc_devno;
+   SLIST_ENTRY(dkcsum_disk) dkc_link;
+};
+SLIST_HEAD(dkcsum_disklist_head, dkcsum_disk);
+
 void
 dkcsumattach(void)
 {
+   struct dkcsum_disklist_head dkc_disklist;
+   struct dkcsum_disk *dkc;
struct device *dv;
+   struct disk *dk;
struct buf *bp;
struct bdevsw *bdsw;
dev_t dev, pribootdev, altbootdev;
@@ -88,12 +97,39 @@ dkcsumattach(void)
 */
bp = geteblk(bios_cksumlen * DEV_BSIZE);/* XXX error check?  */
 
-   TAILQ_FOREACH(dv, alldevs, dv_list) {
-   if (dv-dv_class != DV_DISK)
+   SLIST_INIT(dkc_disklist);
+   dk = TAILQ_FIRST(disklist);
+   while (dk != TAILQ_END(disklist)) {
+
+   dv = dk-dk_device;
+   if (dv == NULL || dk-dk_devno == NODEV ||
+   (dk-dk_flags  DKF_NOLABELREAD)) {
+   dk = TAILQ_NEXT(dk, dk_link);
continue;
-   bp-b_dev = dev = dev_rawpart(dv);
-   if (dev == NODEV)
+   }
+
+   SLIST_FOREACH(dkc, dkc_disklist, dkc_link)
+   if (dkc-dkc_devno == dk-dk_devno)
+   break;
+
+   if (dkc != NULL) {
+   dk = TAILQ_NEXT(dk, dk_link);
continue;
+   }
+
+#ifdef DEBUG
+   printf(dkcsum: adding %s to the list of checked disks\n,
+   dv-dv_xname);
+#endif
+   dkc = malloc(sizeof(struct dkcsum_disk), M_DEVBUF,
+   M_NOWAIT | M_CANFAIL | M_ZERO);
+   if (dkc == NULL)
+   panic(dkcsum: failed to allocate memory);
+   dkc-dkc_devno = dk-dk_devno;
+   SLIST_INSERT_HEAD(dkc_disklist, dkc, dkc_link);
+
+   bp-b_dev = dev = MAKEDISKDEV(major(dk-dk_devno),
+   DISKUNIT(dk-dk_devno), RAW_PART);
bdsw = bdevsw[major(dev)];
 
/*
@@ -107,6 +143,7 @@ dkcsumattach(void)
printf(dkcsum: %s open failed (%d)\n,
dv-dv_xname, error);
 #endif
+   dk = TAILQ_FIRST(disklist);
continue;
}
 
@@ -129,6 +166,7 @@ dkcsumattach(void)
printf(dkcsum: %s close failed (%d)\n,
dv-dv_xname, error);
 #endif
+   dk = TAILQ_FIRST(disklist);
continue;
}
error = (*bdsw-d_close)(dev, FREAD, S_IFCHR, curproc);
@@ -138,6 +176,7 @@ dkcsumattach(void)
printf(dkcsum: %s closed failed (%d)\n,
dv-dv_xname, error);
 #endif
+   dk = TAILQ_FIRST(disklist);
continue;
}
 
@@ -173,6 +212,7 @@ dkcsumattach(void)
printf(dkcsum: %s has no matching BIOS drive\n,
dv-dv_xname);
 #endif 
+   dk = TAILQ_FIRST(disklist);
continue;
}
 
@@ -223,9 +263,18 @@ dkcsumattach(void)

sysctl__string fails to return old length

[Joel Sing]

According to the sysctl(3) man page, calling sysctl with a NULL value for
oldp should result in the current size being returned. This works correctly
for sysctl_rdstring(), but not for sysctl__string().

ok?

Index: kern_sysctl.c
===
RCS file: /cvs/src/sys/kern/kern_sysctl.c,v
retrieving revision 1.206
diff -u -p -u -p -r1.206 kern_sysctl.c
--- kern_sysctl.c   5 Jul 2011 04:48:02 -   1.206
+++ kern_sysctl.c   3 Aug 2011 16:52:39 -
@@ -917,15 +917,15 @@ sysctl__string(void *oldp, size_t *oldle
if (oldp) {
if (trunc  *oldlenp  len) {
/* save  zap NUL terminator while copying */
+   len = *oldlenp;
c = str[*oldlenp-1];
str[*oldlenp-1] = '\0';
error = copyout(str, oldp, *oldlenp);
str[*oldlenp-1] = c;
-   } else {
-   *oldlenp = len;
+   } else
error = copyout(str, oldp, len);
-   }
}
+   *oldlenp = len;
if (error == 0  newp) {
error = copyin(newp, str, newlen);
str[newlen] = 0;

-- 

Reason is not automatic. Those who deny it cannot be conquered by it.
 Do not count on them. Leave them alone. -- Ayn Rand

Make amd64/i386 boot(8) work when 64KB

[Joel Sing]

The amd64/i386 boot(8) code runs in protected mode, however switches back
to real mode for BIOS calls. The real mode code uses a scratch area located
in the BSS section to load/store registers across BIOS calls. However, once
the BSS moves beyond an offset of 0x (a logical address of 0x5
since the base is 0x4) it is inaccessible from the real mode code and
results in access failures.

The following diff removes this restriction by loading the necessary registers
(ES, BX) from the scratch area while still in protected mode, then patching
them into the real mode instructions. The same is done when returning from
real mode to protected mode, allowing the registers to be preserved and then
stored into the scratch area from protected mode.

This has been tested on amd64 and i386, however it needs further testing to
ensure that it does not introduce any unexpected regressions. Please report
successes or failures directly to me.

ok?

Index: arch/amd64/stand/libsa/gidt.S
===
RCS file: /cvs/src/sys/arch/amd64/stand/libsa/gidt.S,v
retrieving revision 1.6
diff -u -p -r1.6 gidt.S
--- arch/amd64/stand/libsa/gidt.S   29 Dec 2006 11:44:01 -  1.6
+++ arch/amd64/stand/libsa/gidt.S   29 Sep 2012 16:15:11 -
@@ -312,6 +312,10 @@ IEMUENT(44); IEMUENT(45); IEMUENT(46); I
  * entry point for BIOS real-mode interface
  * all the magic for real-prot mode switching is here
  *
+ * Note: Once in real mode access to .data or .bss should be avoided since it
+ * may not be reachable within the current segment. The code also assumes that
+ * .text is writeable.
+ *
  * Call:   %eax, %ecx, %edx, %ebx, %ebp, %esi, %edi, %es, %ds
  * Return: %eax, %edx, %ecx, %eflags (as returned from BIOS)
  *
@@ -320,7 +324,7 @@ IEMUENT(44); IEMUENT(45); IEMUENT(46); I
.align  8, 0x90
 EMUh:
/* save %eax */
-   mov %eax, 3f
+   mov %eax, 5f
pop %eax
 
pusha
@@ -332,18 +336,29 @@ EMUh:
/* save BIOS int vector */
mov %al, intno
 
+   /* Load BIOS registers prior to switching to real mode. */
+   movl _C_LABEL(BIOS_regs)+BIOSR_ES, %eax
+   mov %eax, 7f
+   movl _C_LABEL(BIOS_regs)+BIOSR_DS, %eax
+   mov %eax, 6f
+
prot2real
 
push%ds
 
-   addr32  movw (_C_LABEL(BIOS_regs)+(BIOSR_ES) - LINKADDR), %ax
-   movw%ax, %es
-   addr32  movw (_C_LABEL(BIOS_regs)+(BIOSR_DS) - LINKADDR), %ax
-   movw%ax, %ds
+   # data32 movl $Leax, %eax
+   .byte   0x66, 0xb8
+7: .long   0x90909090
+   mov %ax, %es
+
+   # data32 movl $Leax, %eax
+   .byte   0x66, 0xb8
+6: .long   0x90909090
+   mov %ax, %ds
 
# data32 movl $Leax, %eax
.byte   0x66, 0xb8
-3: .long   0x90909090
+5: .long   0x90909090
 
;sti
int $0
@@ -352,19 +367,35 @@ intno = . - 1
 
pop %ds
 
-   addr32 movl %ebx, (_C_LABEL(BIOS_regs)+(BIOSR_BX) - LINKADDR)
-   movw%es, %bx
-   addr32 movw %bx, (_C_LABEL(BIOS_regs)+(BIOSR_ES) - LINKADDR)
+   /* Preserve BX and ES for protected mode. */
+   addr32 movl %eax, (2f - LINKADDR)
+   movl%ebx, %eax
+   addr32 movl %eax, (4f - LINKADDR)
+   movl%es, %eax
+   addr32 movl %eax, (3f - LINKADDR)
+   addr32 movl (2f - LINKADDR), %eax
+
movb%ah, %bh
lahf
xchgb   %ah, %bh
 
+   /* Preserve AX for protected mode. */
addr32 movl %eax, (2f - LINKADDR)
 
real2prot
 
# movl $Leax, %eax
.byte 0xb8
+4: .long 0x90909090
+   movl%eax, _C_LABEL(BIOS_regs)+BIOSR_BX
+
+   # movl $Leax, %eax
+   .byte 0xb8
+3: .long 0x90909090
+   movl%eax, _C_LABEL(BIOS_regs)+BIOSR_ES
+
+   # movl $Leax, %eax
+   .byte 0xb8
 2: .long 0x90909090
 
/* pass BIOS return values back to caller */
Index: arch/i386/stand/libsa/gidt.S
===
RCS file: /cvs/src/sys/arch/i386/stand/libsa/gidt.S,v
retrieving revision 1.32
diff -u -p -r1.32 gidt.S
--- arch/i386/stand/libsa/gidt.S26 Dec 2006 19:30:44 -  1.32
+++ arch/i386/stand/libsa/gidt.S29 Sep 2012 16:15:11 -
@@ -314,6 +314,10 @@ IEMUENT(44); IEMUENT(45); IEMUENT(46); I
  * entry point for BIOS real-mode interface
  * all the magic for real-prot mode switching is here
  *
+ * Note: Once in real mode access to .data or .bss should be avoided since it
+ * may not be reachable within the current segment. The code also assumes that
+ * .text is writeable.
+ *
  * Call:   %eax, %ecx, %edx, %ebx, %ebp, %esi, %edi, %es, %ds
  * Return: %eax, %edx, %ecx, %eflags (as returned from BIOS)
  *
@@ -322,7 +326,7 @@ IEMUENT(44); IEMUENT(45); IEMUENT(46); I
.align  8, 0x90
 EMUh:
/* save %eax */
-   mov %eax, 3f
+   mov %eax, 5f
pop %eax
 

Re: bioctl patch testing

[Joel Sing]

On Sun, 10 Feb 2013, Scott McEachern wrote:
 Moving this to tech.

 I tested the patch found at
 http://marc.info/?l=openbsd-techm=133513662106783w=2 and can give you
 some results.

As you've already discovered, that diff is broken. BIOCVOL does not behave how 
the diff assumes - it will return the volume specified by the bv_volid field 
in the bioc_vol struct. Since this is never initialised you'll effectively 
get a random volume returned (based on stack garbage). This is arguably a 
design flaw in bio(4), which softraid could work around (basically we want to 
identify the volume whose device we opened, not the softraid controller).
-- 

Reason is not automatic. Those who deny it cannot be conquered by it.
 Do not count on them. Leave them alone. -- Ayn Rand

Re: bioctl should retry passphrase

[Joel Sing]

On Friday 14 January 2011, Ted Unangst wrote:
 If I type the wrong password into bioctl at boot, disks don't exist,
 filesystems don't get mounted, and generally lots of things go wrong.  All
 I need is a second chance to remind me to type the right password.

Huh? Both you and Marco rejected this last year and when I last checked there 
was no bioctl included in /etc/rc... I guess we need to decide if bioctl 
should behave like su/passwd, sudo or like something else.

==

Re: bioctl patch (inline) diff -uNp
From: Ted Unangst ted.unan...@gmail.com
To: merlyn merlyn...@gmail.com
CC: Marco Peereboom sl...@peereboom.us, tech@openbsd.org
Date: 2010-09-15 05:21
   
On Tue, Sep 14, 2010 at 3:46 PM, merlyn merlyn...@gmail.com wrote:
 I am not a fan of this.  Why wouldn't you do this in the wrapping
 script?

 Just because I think such a basic thing should be presend.
 And I'm not a fan of doing this in wrapping script.
 However I respect your decision.

I'm with Marco here.  Other command line tools don't ask questions
like this.  You just rerun the command.

==

 Index: bioctl.c
 ===
 RCS file: /home/tedu/cvs/src/sbin/bioctl/bioctl.c,v
 retrieving revision 1.98
 diff -u -r1.98 bioctl.c
 --- bioctl.c  1 Dec 2010 19:40:18 -   1.98
 +++ bioctl.c  13 Jan 2011 23:47:24 -
 @@ -699,6 +699,7 @@
   int rv, no_dev, fd;
   dev_t   *dt;
   u_int16_t   min_disks = 0;
 + int retry = 0;

   if (!dev_list)
   errx(1, no devices specified);
 @@ -738,6 +739,7 @@
   if (level == 'C'  no_dev != min_disks)
   errx(1, not exactly one partition);

 +again:
   memset(create, 0, sizeof(create));
   create.bc_cookie = bl.bl_cookie;
   create.bc_level = level;
 @@ -802,8 +804,14 @@
   memset(kdfinfo, 0, sizeof(kdfinfo));
   memset(create, 0, sizeof(create));
   if (rv == -1) {
 - if (errno == EPERM)
 + if (errno == EPERM) {
 + if (!retry) {
 + warnx(Incorrect passphrase. Try again.);
 + retry = 1;
 + goto again;
 + }
   errx(1, Incorrect passphrase);
 + }
   err(1, BIOCCREATERAID);
   }

-- 

   Stop assuming that systems are secure unless demonstrated insecure;
start assuming that systems are insecure unless designed securely.
  - Bruce Schneier

softraid: factor out block I/O code

[Joel Sing]

The following diff factors out the block I/O code that is used within
softraid(4) and also allows it to handle I/Os that exceeds MAXPHYS in
size. This is necessary for some upcoming work.

This diff needs extensive testing since the main purpose is to read and
write the softraid metadata. Bugs in this area will eat softraid metadata
and therefore destroy softraid volumes. If you are testing please ensure
that you have backed up your data or that the volume does not have
critical information. Please report test successes/failures directly to me.

Index: softraidvar.h
===
RCS file: /cvs/src/sys/dev/softraidvar.h,v
retrieving revision 1.96
diff -u -p -r1.96 softraidvar.h
--- softraidvar.h   6 Nov 2010 23:01:56 -   1.96
+++ softraidvar.h   6 Jan 2011 12:41:26 -
@@ -624,6 +624,8 @@ int sr_check_io_collision(struct sr_wo
 void   sr_scsi_done(struct sr_discipline *,
struct scsi_xfer *);
 intsr_chunk_in_use(struct sr_softc *, dev_t);
+intsr_rw(struct sr_softc *, dev_t, char *, size_t,
+   daddr64_t, long);
 
 /* discipline functions */
 intsr_raid_inquiry(struct sr_workunit *);
Index: softraid.c
===
RCS file: /cvs/src/sys/dev/softraid.c,v
retrieving revision 1.217
diff -u -p -r1.217 softraid.c
--- softraid.c  20 Dec 2010 17:47:48 -  1.217
+++ softraid.c  6 Jan 2011 12:41:26 -
@@ -387,57 +387,93 @@ sr_meta_getdevname(struct sr_softc *sc, 
 }
 
 int
-sr_meta_rw(struct sr_discipline *sd, dev_t dev, void *md, size_t sz,
-daddr64_t ofs, long flags)
+sr_rw(struct sr_softc *sc, dev_t dev, char *buf, size_t size, daddr64_t 
offset,
+long flags)
 {
-   struct sr_softc *sc = sd-sd_sc;
+   struct vnode*vp;
struct buf  b;
+   size_t  bufsize;
int rv = 1;
 
-   DNPRINTF(SR_D_META, %s: sr_meta_rw(0x%x, %p, %d, %llu 0x%x)\n,
-   DEVNAME(sc), dev, md, sz, ofs, flags);
-
-   bzero(b, sizeof(b));
+   DNPRINTF(SR_D_MISC, %s: sr_rw(0x%x, %p, %d, %llu 0x%x)\n,
+   DEVNAME(sc), dev, buf, size, offset, flags);
 
-   if (md == NULL) {
-   printf(%s: read invalid metadata pointer\n, DEVNAME(sc));
+   if (bdevvp(dev, vp)) {
+   printf(%s: sr_rw: failed to allocate vnode\n, DEVNAME(sc));
goto done;
}
-   b.b_flags = flags | B_PHYS;
-   b.b_blkno = ofs;
-   b.b_bcount = sz;
-   b.b_bufsize = sz;
-   b.b_resid = sz;
-   b.b_data = md;
-   b.b_error = 0;
-   b.b_proc = curproc;
-   b.b_dev = dev;
-   b.b_iodone = NULL;
-   if (bdevvp(dev, b.b_vp)) {
-   printf(%s: sr_meta_rw: can't allocate vnode\n, DEVNAME(sc));
-   goto done;
-   }
-   if ((b.b_flags  B_READ) == 0)
-   b.b_vp-v_numoutput++;
-
-   LIST_INIT(b.b_dep);
-   VOP_STRATEGY(b);
-   biowait(b);
-
-   if (b.b_flags  B_ERROR) {
-   printf(%s: 0x%x i/o error on block %llu while reading 
-   metadata %d\n, DEVNAME(sc), dev, b.b_blkno, b.b_error);
-   goto done;
+
+   while (size  0) {
+   
+   DNPRINTF(SR_D_MISC, %s: buf %p, size %d, offset %llu)\n,
+   DEVNAME(sc), buf, size, offset);
+
+   bufsize = (size  MAXPHYS) ? MAXPHYS : size;
+
+   bzero(b, sizeof(b));
+
+   b.b_flags = flags | B_PHYS;
+   b.b_proc = curproc;
+   b.b_dev = dev;
+   b.b_iodone = NULL;
+   b.b_error = 0;
+   b.b_blkno = offset;
+   b.b_data = buf;
+   b.b_bcount = bufsize;
+   b.b_bufsize = bufsize;
+   b.b_resid = bufsize;
+   b.b_vp = vp;
+
+   if ((b.b_flags  B_READ) == 0)
+   vp-v_numoutput++;
+
+   LIST_INIT(b.b_dep);
+   VOP_STRATEGY(b);
+   biowait(b);
+
+   if (b.b_flags  B_ERROR) {
+   printf(%s: I/O error %d on dev 0x%x at block %llu\n,
+   DEVNAME(sc), b.b_error, dev, b.b_blkno);
+   goto done;
+   }
+
+   size -= bufsize;
+   buf += bufsize;
+   offset += howmany(bufsize, DEV_BSIZE);
+
}
+
rv = 0;
+
 done:
-   if (b.b_vp)
-   vput(b.b_vp);
+   if (vp)
+   vput(vp);
 
return (rv);
 }
 
 int
+sr_meta_rw(struct sr_discipline *sd, dev_t dev, void *md, size_t size,
+daddr64_t offset, long flags)
+{
+   int rv = 1;
+
+   DNPRINTF(SR_D_META, %s: sr_meta_rw(0x%x, %p, %d, %llu 0x%x)\n,
+   DEVNAME(sd-sd_sc), dev, 

Re: /bsd: splassert: assertwaitok: want -1 have 1

[Joel Sing]

On Thursday 20 January 2011, Gregory Edigarov wrote:
 On Wed, 19 Jan 2011 20:14:01 +1100

 Joel Sing j...@sing.id.au wrote:
  On Wednesday 19 January 2011, Gregory Edigarov wrote:
   Hello,
  
   I have my home system connected via pppoe(4) to a provider and
   connection disapears very frequently some once an hour.
   Just before connection is gone I always see the following in my
   logs:
  
   /bsd: splassert: assertwaitok: want -1 have 1
 
  Please set kern.splassert = 2 and provide a stack trace.
 
   My first thought was that something happens on provider's side but I
   eliminate this reason connecting one of my other boxes(with linux)
   directly to my provider. The linux box is working correctly.
   I've also tryed to change the nic. It was rl(4) now it is vr(4).
   Result is the same.
  
   System is:
   # uname -a
   OpenBSD edigarov.sa.net.ua 4.9 GENERIC#11 amd64
   rebuilt on Sun 16 Jan.

 --- interrupt ---
 end trace frame: 0x0, count: 245
 0x8:
 End of stack trace.
 pppoe0: received unexpected PADO
 pppoe0: chap failure
 pppoe: GENERIC ERROR: RP-PPPoE: Child pppd process terminated

This message is not being generated by from pppoe(4), rather it is originating 
from the remote end. Looks like the remote end is running Roaring Penguin 
(RP) PPPoE and that for some reason the pppd process is terminating. The 
preceeding unexpected PADO (PPPoE Active Discovery Offer) and chap failure 
suggest that the other end is making an unsolicity offer that then fails 
authentication and therefore results in session disconnection.

 pppoe0: received unexpected PADO
 pppoe0: chap failure
 pppoe: GENERIC ERROR: RP-PPPoE: Child pppd process terminated
 pppoe0: received unexpected PADO
 pppoe0: chap failure
 pppoe: GENERIC ERROR: RP-PPPoE: Child pppd process terminated
 pppoe0: received unexpected PADO
 splassert: assertwaitok: want -1 have 1

This part is a bug in OpenBSD - an IPCP message is trigging the addition of an 
interface address from interrupt context, which is no longer permitted. The 
dropout and reconnection is however triggering it.

 Starting stack trace...
 assertwaitok() at assertwaitok+0x1c
 pool_get() at pool_get+0x95
 ifa_item_insert() at ifa_item_insert+0x35
 ifa_add() at ifa_add+0x43
 in_ifinit() at in_ifinit+0x16f
 sppp_set_ip_addrs() at sppp_set_ip_addrs+0x107
 sppp_ipcp_tlu() at sppp_ipcp_tlu+0x4e
 sppp_input() at sppp_input+0x594
 pppoeintr() at pppoeintr+0x41d
 netintr() at netintr+0x97
 softintr_dispatch() at softintr_dispatch+0x5d
 Xsoftnet() at Xsoftnet+0x28
 --- interrupt ---
 end trace frame: 0x0, count: 245
 0x8:
 End of stack trace.

-- 

   Stop assuming that systems are secure unless demonstrated insecure;
start assuming that systems are insecure unless designed securely.
  - Bruce Schneier

Re: /bsd: splassert: assertwaitok: want -1 have 1

[Joel Sing]

On Thursday 20 January 2011, Mike Belopuhov wrote:
 On Thu, Jan 20, 2011 at 10:31 +0200, Gregory Edigarov wrote:
  --- interrupt ---
  end trace frame: 0x0, count: 245
  0x8:
  End of stack trace.
  pppoe0: received unexpected PADO
  pppoe0: chap failure
  pppoe: GENERIC ERROR: RP-PPPoE: Child pppd process terminated
  pppoe0: received unexpected PADO
  pppoe0: chap failure
  pppoe: GENERIC ERROR: RP-PPPoE: Child pppd process terminated
  pppoe0: received unexpected PADO
  pppoe0: chap failure
  pppoe: GENERIC ERROR: RP-PPPoE: Child pppd process terminated
  pppoe0: received unexpected PADO
  splassert: assertwaitok: want -1 have 1
  Starting stack trace...
  assertwaitok() at assertwaitok+0x1c
  pool_get() at pool_get+0x95
  ifa_item_insert() at ifa_item_insert+0x35
  ifa_add() at ifa_add+0x43
  in_ifinit() at in_ifinit+0x16f
  sppp_set_ip_addrs() at sppp_set_ip_addrs+0x107
  sppp_ipcp_tlu() at sppp_ipcp_tlu+0x4e
  sppp_input() at sppp_input+0x594
  pppoeintr() at pppoeintr+0x41d
  netintr() at netintr+0x97
  softintr_dispatch() at softintr_dispatch+0x5d
  Xsoftnet() at Xsoftnet+0x28
  --- interrupt ---
  end trace frame: 0x0, count: 245
  0x8:
  End of stack trace.

 seems like this is the only plausible way to fix it:

 Index: net/if.c
 ===
 RCS file: /home/cvs/src/sys/net/if.c,v
 retrieving revision 1.231
 diff -u -p -r1.231 if.c
 --- net/if.c  29 Nov 2010 19:38:59 -  1.231
 +++ net/if.c  20 Jan 2011 11:11:53 -
 @@ -2213,7 +2213,7 @@ ifa_item_insert(struct sockaddr *sa, str
  {
   struct ifaddr_item  *ifai, *p;

 - ifai = pool_get(ifaddr_item_pl, PR_WAITOK);
 + ifai = pool_get(ifaddr_item_pl, PR_NOWAIT);
   ifai-ifai_addr = sa;
   ifai-ifai_ifa = ifa;
   ifai-ifai_rdomain = ifp-if_rdomain;

pool_get() with PR_NOWAIT... and then not checking the return value? That's 
got null pointer dereference written all over it... :)

However, the bigger problem is what can you then do if the pool_get() fails? 
This then results in the interface address not being allocated and in most 
cases there is no way to propagate/handle the error. The solution here is to 
add the interface address from process context and not from interrupt 
context.
-- 

   Stop assuming that systems are secure unless demonstrated insecure;
start assuming that systems are insecure unless designed securely.
  - Bruce Schneier

Re: LibreSSL-portable 2.1.1 s_client supports connecting to SSLv3 servers

[Joel Sing]

On Tue, 25 Nov 2014, Bernard Spil wrote:
 Hi,

 Running LibreSSL portable 2.1.1 from FreeBSD ports on FreeBSD 10.1
 $ /usr/local/bin/openssl version
 LibreSSL 2.1
 $ uname -a
 FreeBSD meterkast3.example.org 10.1-RELEASE FreeBSD 10.1-RELEASE #0
 r264324M: Tue Nov 11 13:46:58 CET 2014
 r...@meterkast3.example.org:/usr/obj/usr/src/sys/BEASTIE101  amd64

 To my surprise, the LibreSSL openssl binary does not see the -sslv3
 option as an error. (examples and captures with google.com server)
$ /usr/local/bin/openssl s_client -connect 173.194.65.147:443 -ssl3
CONNECTED(0003)
 where I would expect the same behaviour as e.g. openssl 0.9.8 when
 calling it with the -tls1_2 option.

 Next to that I see that it succefully negotiates a connection using an
 ssl3-capable server.
 Client Hello and Server Hello both have 0x0300 as can be seen in
 attached capture and at end of this mail.

 Is this expected behaviour?

Yes.

 I.e. has LibreSSL only removed the sslv3 server capability?

SSLv3 has only been disabled by default - if you explicitly ask for it then 
you still get it. In the case of s_client, the -ssl3 option explicitly 
switches to the SSLv3 client method, hence it will *only* negotiate SSLv3.

 When I setup an SSL server with OpenSSL 1.0.1j from base, I can not
 connect to it straight away but I can connect when I use -ssl3 (both in
 log below)

Are you saying that running 'openssl s_client' fails to connect to 'openssl 
s_server'? I do not see any example where you are not specifying -ssl3 with 
s_server - by doing that you can only ever connect to it with SSLv3 (-ssl3 
does not enable the negotiation of SSLv3, it makes it SSLv3 *only*).

 $ openssl version
 OpenSSL 1.0.1j-freebsd 15 Oct 2014
 $ openssl s_server -ssl3 -accept 4443
 Using default temp DH parameters
 Using default temp ECDH parameters
 ACCEPT
 ERROR
 shutting down SSL
 CONNECTION CLOSED
 ACCEPT
 -BEGIN SSL SESSION PARAMETERS-
 snip
 -END SSL SESSION PARAMETERS-
 snipCIPHER is ECDHE-RSA-AES256-SHA
 Secure Renegotiation IS supported
 DONE
 shutting down SSL
 CONNECTION CLOSED
 ACCEPT

 $ /usr/local/bin/openssl s_client -connect localhost:4443
 CONNECTED(0003)
 34378806536:error:14077102:SSL
 routines:SSL23_GET_SERVER_HELLO:unsupported protocol:s23_clnt.c:497:
 ---
 no peer certificate available
 ---
 No client certificate CA names sent
 ---
 SSL handshake has read 7 bytes and written 280 bytes
 ---
 New, (NONE), Cipher is (NONE)
 Secure Renegotiation IS NOT supported
 Compression: NONE
 Expansion: NONE
 ---
 $ /usr/local/bin/openssl s_client -connect localhost:4443 -ssl3
 snip
 ---
 SSL handshake has read 1524 bytes and written 262 bytes
 ---
 New, TLSv1/SSLv3, Cipher is ECDHE-RSA-AES256-SHA
 Server public key is 2048 bit
 Secure Renegotiation IS supported
 Compression: NONE
 Expansion: NONE
 SSL-Session:
  Protocol  : SSLv3
  Cipher: ECDHE-RSA-AES256-SHA
  Session-ID:
 468B5F3CE1CF1CDA9F49312EE9424BD985B22FC1A9EA92692C9C6EB818F0C725
  Session-ID-ctx:
  Master-Key:
 78D830C15F518C6FC9C5D9760B8B3F09D58F516944E72C9F2A89D3B3E6DD6D78189B1B0A702
D4FBB8CDDEBF83B19A433 Start Time: 1416914867
  Timeout   : 7200 (sec)
  Verify return code: 21 (unable to verify the first certificate)
 ---

 Thanks!
 Bernard (Barnerd) Spil.

 depth=2 C = US, O = GeoTrust Inc., CN = GeoTrust Global CA
 verify error:num=20:unable to get local issuer certificate
 verify return:0
 ---
 Certificate chain
   0 s:/C=US/ST=California/L=Mountain View/O=Google Inc/CN=www.google.com
 i:/C=US/O=Google Inc/CN=Google Internet Authority G2
   1 s:/C=US/O=Google Inc/CN=Google Internet Authority G2
 i:/C=US/O=GeoTrust Inc./CN=GeoTrust Global CA
   2 s:/C=US/O=GeoTrust Inc./CN=GeoTrust Global CA
 i:/C=US/O=Equifax/OU=Equifax Secure Certificate Authority
 ---
 Server certificate
 -BEGIN CERTIFICATE-
 snip
 -END CERTIFICATE-
 subject=/C=US/ST=California/L=Mountain View/O=Google
 Inc/CN=www.google.com
 issuer=/C=US/O=Google Inc/CN=Google Internet Authority G2
 ---
 No client certificate CA names sent
 ---
 SSL handshake has read 3578 bytes and written 258 bytes
 ---
 New, TLSv1/SSLv3, Cipher is ECDHE-RSA-RC4-SHA
 Server public key is 2048 bit
 Secure Renegotiation IS supported
 Compression: NONE
 Expansion: NONE
 SSL-Session:
  Protocol  : SSLv3
  Cipher: ECDHE-RSA-RC4-SHA
  Session-ID:
 D807A5102140A0D0F5DF4562E961C485F7C0D506572FF7852D61207576F3C5A5
  Session-ID-ctx:
  Master-Key:
 175DDE1E866E41DC8F9D64779B0BBB5F4AA663F2DBF1EB1C312036CFE9E580997653A73CB6C
7AEB2310B6D5793F13C55 Start Time: 1416913094
  Timeout   : 7200 (sec)
  Verify return code: 20 (unable to get local issuer certificate)
 ---



-- 

Action without study is fatal. Study without action is futile.
-- Mary Ritter Beard

Re: libtls: Secure default cipher list and dtls support

[Joel Sing]

On Thu, 27 Nov 2014, Manuel Schoelling wrote:
 Hi,

 I hope this is the right mailing list for discussing this issue. I could
 not find any information about a mailing list on libressl.org.

Here is fine.

 It currently looks like the libtls version does not set a list of secure
 ciphers by default (e.g. that does not include MD5 or SHA-1).
 Would it be a reasonable idea to include secure defaults in libtls?

Yes - there are plans for this.

 I also noticed that libtls is currently supporting SOCK_STREAM (TLS)
 connections only. Is the support of SOCK_DGRAM (DTLS) connections within
 the scope of this library and would patches be accepted?

I do not have any objection to supporting datagram sockets, however it is not 
a primary interest/focus and there are many things that would likely get 
implemented prior to looking at it. That said, if you have a use case for it 
and can make it fit with the API, we'll happy review diffs.
-- 

Action without study is fatal. Study without action is futile.
-- Mary Ritter Beard

Re: LibreSSL 2.1.2 linking issues

[Joel Sing]

On Wed, 10 Dec 2014, Lukas Tribus wrote:
  On 2014/12/09 07:37, Brent Cook wrote:
  If an app calls a function, it should probably check if that function
  exists during configuration time, rather than inferring if define A
  exists, function B and C must exist. Especially things that are just
  protocol constant definitions. If they are using autoconf, a call to
  AC_CHECK_FUNCS would be better, IMHO.
 
  Neither nginx nor haproxy use autoconf. nginx has it's own config
  testing infrastructure where this could be added, though they appear
  to be deliberately trying to keep the maze of #ifdef and configuration
  tests to a minimum. haproxy has a simple Makefile with USE_* flags
  set for different system types, it's quite clean and straightforward
  but doesn't allow for this type of test.

 Thats correct indeed.

 I believe a not to be underestimated amount of applications #ifdef's
 certain functionality of openssl out, for example NPN
 (SSL_CTRL_SET_TLSEXT_HOSTNAME) or server preferential cipher ordering
 (SSL_OP_CIPHER_SERVER_PREFERENCE).

That's rather different to checking using defines with TLSEXT_TYPE_* - these 
are just definitions of TLS extension types and assuming that the presence of 
these implies support is plain crazy. For example, TLSEXT_TYPE_user_mapping 
is defined but AFAIK neither LibreSSL or OpenSSL have any support for it.

The above mentioned defines are ones that exist as part of an API - if those 
exist then it is likely a fair assumption that the API exists and is useable.

 In fact if I'm not mistaken that is exactly the reason why both
 LibreSSL [1] and BoringSSL [2] have opensslfeatures.h.

As has been documented elsewhere, the feature negation approach is seriously 
broken - presumably we should have defined OPENSSL_NO_ALPN to indicate that 
we did not have ALPN support, but you then have a race between knowing what 
you need to indicate a lack of support for and actually saying you do not 
support it. On the other hand defining OPENSSL_ALPN as a feature would have 
allowed software to check for that instead of trying to do half baked checks 
based on things like TLS extension types that do not imply an API exists.

  But, I believe jsing@ has a patch in-review to merge the rest ALPN
  support any way.

It has just been committed so hopefully one less problem to deal with (or I 
just created another one, depending on how you look at it :)

 Great, thanks!



 [1]
 https://github.com/libressl-portable/openbsd/blob/master/src/lib/libssl/src
/crypto/opensslfeatures.h [2]
 https://boringssl.googlesource.com/boringssl/+/987b8f1e715414b0b278a4a0c64e
c9c97ad65f58%5E!/



-- 

Action without study is fatal. Study without action is futile.
-- Mary Ritter Beard

Re: Too much SUID/SGID files!

[Joel Sing]

On Tuesday 06 January 2015, whoami toask wrote:
 Hello,

 isn't there too much SUID/SGID files on a default OpenBSD install?

 Can this number be reduced?

Of course it can!

$ find / -perm -4000 -o -perm -2000 -exec chmod 0 {} \;

 Example: why does wall, write, modstat need an SGID?

 # uname -a
 OpenBSD notebook.lan 5.6 GENERIC.MP#333 amd64
 # find / -perm -4000 -o -perm -2000 -ls -print
  78047 5856 -rwxr-sr-x1 root auth  2970920 Aug  6 21:45
 /usr/X11R6/bin/xlock/usr/X11R6/bin/xlock 78068 1216 -rwxr-sr-x1 root   
  utmp   592056 Aug  6 22:09 /usr/X11R6/bin/xterm/usr/X11R6/bin/xterm
 1147497   60 -r-xr-sr-x1 root kmem30200 Jul 31 11:50
 /usr/local/bin/libgtop_server2/usr/local/bin/libgtop_server2 78031   32
 -r-xr-sr-x1 root utmp15864 Jul 31 09:57
 /usr/local/libexec/gnome-pty-helper/usr/local/libexec/gnome-pty-helper
 155910   84 -r-xr-sr-x4 root crontab 41752 Aug  8 08:06
 /usr/bin/at/usr/bin/at 155910   84 -r-xr-sr-x4 root crontab
 41752 Aug  8 08:06 /usr/bin/atq/usr/bin/atq 155910   84 -r-xr-sr-x4
 root crontab 41752 Aug  8 08:06 /usr/bin/atrm/usr/bin/atrm 155910  
 84 -r-xr-sr-x4 root crontab 41752 Aug  8 08:06
 /usr/bin/batch/usr/bin/batch 155943   72 -r-xr-sr-x1 root crontab  
   36504 Aug  8 08:06 /usr/bin/crontab/usr/bin/crontab 156014   24
 -r-xr-sr-x1 root auth11672 Aug  8 08:06
 /usr/bin/lock/usr/bin/lock 156019   60 -r-xr-sr-x1 root daemon 
 28952 Aug  8 08:06 /usr/bin/lpq/usr/bin/lpq 156033   20 -r-xr-sr-x1
 root _lkm 8952 Aug  8 08:06 /usr/bin/modstat/usr/bin/modstat
 156035  292 -r-xr-sr-x1 root kmem   148216 Aug  8 08:06
 /usr/bin/netstat/usr/bin/netstat 156093   24 -r-xr-sr-x1 root auth 
   11544 Aug  8 08:06 /usr/bin/skeyaudit/usr/bin/skeyaudit 156094   16
 -r-xr-sr-x1 root auth 8184 Aug  8 08:06
 /usr/bin/skeyinfo/usr/bin/skeyinfo 156095   44 -r-xr-sr-x1 root
 auth20632 Aug  8 08:06 /usr/bin/skeyinit/usr/bin/skeyinit 156105 
 704 -r-xr-sr-x1 root _sshagnt   333656 Aug  8 08:07
 /usr/bin/ssh-agent/usr/bin/ssh-agent 156112  284 -r-xr-sr-x1 root
 kmem   144568 Aug  8 08:06 /usr/bin/systat/usr/bin/systat 156146   32
 -r-xr-sr-x1 root tty 15928 Aug  8 08:06
 /usr/bin/wall/usr/bin/wall 156152   28 -r-xr-sr-x1 root tty
 13080 Aug  8 08:06 /usr/bin/write/usr/bin/write 103939   40 -r-xr-sr-x4
 root _token  20344 Aug  8 08:06
 /usr/libexec/auth/login_activ/usr/libexec/auth/login_activ 103939   40
 -r-xr-sr-x4 root _token  20344 Aug  8 08:06
 /usr/libexec/auth/login_crypto/usr/libexec/auth/login_crypto 103943   40
 -r-xr-sr-x1 root _radius 19928 Aug  8 08:06
 /usr/libexec/auth/login_radius/usr/libexec/auth/login_radius 103945   24
 -r-xr-sr-x1 root auth11608 Aug  8 08:06
 /usr/libexec/auth/login_skey/usr/libexec/auth/login_skey 103939   40
 -r-xr-sr-x4 root _token  20344 Aug  8 08:06
 /usr/libexec/auth/login_snk/usr/libexec/auth/login_snk 103939   40
 -r-xr-sr-x4 root _token  20344 Aug  8 08:06
 /usr/libexec/auth/login_token/usr/libexec/auth/login_token 103947   40
 -r-xr-sr-x1 root auth20408 Aug  8 08:06
 /usr/libexec/auth/login_yubikey/usr/libexec/auth/login_yubikey 103987 1568
 -r-xr-sr-x1 root smmsp  783576 Aug  8 08:08
 /usr/libexec/sendmail/sendmail/usr/libexec/sendmail/sendmail 52023   80
 -r-xr-sr-x1 root daemon  39736 Aug  8 08:06
 /usr/sbin/lpc/usr/sbin/lpc 52024  160 -r-xr-s---1 root daemon 
 80952 Aug  8 08:06 /usr/sbin/lpd/usr/sbin/lpd 52073   52 -r-xr-sr-x1
 root kmem24664 Aug  8 08:06 /usr/sbin/pstat/usr/sbin/pstat
 5196804 drwxrws---2 root wheel 512 Aug  8 08:05
 /var/audit/var/audit # find / -perm -4000 -o -perm -2000 -ls -print | wc -l
 32

 Thanks,

 have a secure day!



-- 

Action without study is fatal. Study without action is futile.
-- Mary Ritter Beard

Re: the libressl wikipedia article is awful.

[Joel Sing]

On Sunday 22 March 2015, Jiří Navrátil wrote:
 Good morning Bob,

 I did a quick fix

 OpenBSD, FreeBSD[2] and many others

 Where I can get list of supported operating systems, please? I will add
 them.

The current list of platforms supported by LibreSSL portable is available at:

  http://www.libressl.org/releases.html

 I can also add list of removed operating systems in the text, if someone
 will see it valuable there.

 In general - I can go through the article and the check the accuracy. I’m
 not sure, if will be able to check all details. Which our documents can be
 used as my inputs?

 Thank you,
 Jiri

 --
 Jiri Navratil, http://kouc.navratil.cz,  +420 222 767 131

  22. 3. 2015 v 2:51, Bob Beck b...@obtuse.com:
 
  Someone who wikipedias should fix it. It runs on a lot more than
  OpenBSD and FreeBSD.

-- 

Action without study is fatal. Study without action is futile.
-- Mary Ritter Beard

Re: tls_accept_socket() error message

[Joel Sing]

On Tuesday 31 March 2015, Tim van der Molen wrote:
 httpd/server.c contains the following:

   ret = tls_accept_socket(srv-srv_tls_ctx, clt-clt_tls_ctx,
   clt-clt_s);
   [...]
   } else if (ret != 0) {
   log_warnx(%s: TLS accept failed - %s, __func__,
   tls_error(srv-srv_tls_ctx));
   return;

 Here the return value of tls_error(srv-srv_tls_ctx) may be incorrect if
 tls_accept_socket() sets the error message in clt-clt_tls_ctx.

 For instance, in my case, the above code snippet produces the following
 log entries:

 Mar 29 22:53:22 alpha httpd[6684]: server_accept_tls: TLS accept failed -
 (null)

Thanks for flagging this - the real issue is that the error was being assigned 
to the connection context, rather than the server context. I've just 
submitted a diff that changes this behaviour. This means we now get actual 
errors:

server_accept_tls: TLS accept failed - accept failed: error:1407609C:SSL 
routines:SSL23_GET_CLIENT_HELLO:http request

 Perhaps the following diff would be a good way to fix this.

 Index: tls_server.c
 ===
 RCS file: /cvs/src/lib/libtls/tls_server.c,v
 retrieving revision 1.5
 diff -p -U5 -r1.5 tls_server.c
 --- tls_server.c  7 Feb 2015 09:50:09 -   1.5
 +++ tls_server.c  30 Mar 2015 17:28:33 -
 @@ -133,10 +133,11 @@ tls_accept_socket(struct tls *ctx, struc
   if ((ret = SSL_accept(conn_ctx-ssl_conn)) != 1) {
   err = tls_ssl_error(conn_ctx, ret, accept);
   if (err == TLS_READ_AGAIN || err == TLS_WRITE_AGAIN) {
   return (err);
   }
 + tls_set_error(ctx, %s, tls_error(conn_ctx));
   goto err;
   }

   return (0);



-- 

Action without study is fatal. Study without action is futile.
-- Mary Ritter Beard

Re: libtls manpage diff

[Joel Sing]

On Tuesday 31 March 2015, Tim van der Molen wrote:
 - Correct title.
 - tls_accept_socket() also may return TLS_{READ,WRITE}_AGAIN.

I've committed a slightly different version of this and fixed the title - 
thanks for the diff.

 Index: tls_init.3
 ===
 RCS file: /cvs/src/lib/libtls/tls_init.3,v
 retrieving revision 1.18
 diff -u -r1.18 tls_init.3
 --- tls_init.322 Feb 2015 15:09:54 -  1.18
 +++ tls_init.330 Mar 2015 16:36:47 -
 @@ -15,7 +15,7 @@
  .\ OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
  .\
  .Dd $Mdocdate: February 22 2015 $
 -.Dt TLS 3
 +.Dt TLS_INIT 3
  .Os
  .Sh NAME
  .Nm tls_init ,
 @@ -391,9 +391,10 @@
  Functions that return a pointer will return NULL on error.
  .Pp
  The
 -.Fn tls_read
 -and
 +.Fn tls_read ,
  .Fn tls_write
 +and
 +.Fn tls_accept_socket
  functions and the
  .Fn tls_connect
  family of functions have two special return values.



-- 

Action without study is fatal. Study without action is futile.
-- Mary Ritter Beard

Re: crypto softraid and keydisk on same harddrive

[Joel Sing]

On Saturday 25 April 2015, Patrik Lundin wrote:
 On Wed, Oct 29, 2014 at 01:24:30AM +1100, Joel Sing wrote:
  On Wed, 29 Oct 2014, Joel Sing wrote:
   A CRYPTO key disk is slightly special in that it has softraid metadata
   but is not technically part of the same volume (well, it is in some
   ways but it is not in others). The problem in question occurs since
   installboot(8) installs the first stage boot loader on each chunk that
   is a member of the volume - in this case it installs first stage boot
   loader twice (once for wd0a and again for wd0d). The second stage boot
   loader is installed in the softraid metadata area for the sd0 volume,
   however in the case of a CRYPTO key disk its metadata area does not end
   up with a copy of the boot of the second stage loader (unlike, say a
   RAID 1 chunk). If the first stage boot blocks are installed in the
   CRYPTO volume then the key disk, the boot loader (in the PBR of wd0)
   will end up pointing at a boot storage area (of the key disk) that does
   not contain the second stage boot loader. The fix is to probably avoid
   installing the boot loader on the key disk.
 
  You could try this (only compile tested) diff:
 
  Index: i386_softraid.c
  ===
  RCS file: /cvs/src/usr.sbin/installboot/i386_softraid.c,v
  retrieving revision 1.2
  diff -u -p -r1.2 i386_softraid.c
  --- i386_softraid.c 9 Jun 2014 13:13:48 -   1.2
  +++ i386_softraid.c 28 Oct 2014 14:21:27 -
  @@ -42,6 +42,7 @@ void  sr_install_bootldr(int, char *);
   void
   sr_install_bootblk(int devfd, int vol, int disk)
   {
  +   struct bioc_vol bv;
  struct bioc_disk bd;
  struct disklabel dl;
  struct partition *pp;
  @@ -56,6 +57,15 @@ sr_install_bootblk(int devfd, int vol, i
  bd.bd_diskid = disk;
  if (ioctl(devfd, BIOCDISK, bd) == -1)
  err(1, BIOCDISK);
  +
  +   /* Skip CRYPTO key disks. */
  +   /* XXX - pass volume in rather than volume ID. */
  +   memset(bv, 0, sizeof(bv));
  +   bv.bv_volid = vol;
  +   if (ioctl(devfd, BIOCVOL, bv) == -1)
  +   err(1, BIOCVOL);
  +   if (bv.bv_level == 'C'  bd.bd_size == 0)
  +   return;
 
  /* Check disk status. */
  if (bd.bd_status != BIOC_SDONLINE  bd.bd_status != BIOC_SDREBUILD) {
 

 Any developer feel like looking at merging this diff? It seems jsing is
 busy with other work. My interest in this is that it is helpful for
 OpenBSD machines used at Dreamhack (http://dreamhack.se) to quickly
 decommision them when the festival is over.

 The original thread can be found here:
 http://marc.info/?l=openbsd-miscm=141435482820277w=2

Apologies for not getting back to look at this - the above diff is in part a 
hack and it needs to be more cleanly implemented before it is committed. 
Additionally, it needs to be implemented and tested for all platforms that 
support softraid boot. I'll attempt to get this done over the next couple of 
weeks.

In the meantime, as previously mentioned, the more effective way of clearing a 
softraid crypto volume is to recreate the volume with 'bioctl -C force' (or 
even just dd over the first 1MB of the RAID partition) - that nukes the keys 
that were used to encrypt the data, making the key disk or passphrase 
completely useless. Also, keep in mind that anyone who has root access on 
these machines has the ability to copy the key disk, the softraid metadata 
with the encrypted disk keys and even the unencrypted disk encryption keys 
once they're in memory...
-- 

Action without study is fatal. Study without action is futile.
-- Mary Ritter Beard

Re: [patch] Turn on Server Cipher Preference

[Joel Sing]

On Friday 15 May 2015, Kyle Thompson wrote:
 Very basic patch to turn on server cipher preference in libtls. This
 will allow us to always use our cipher preference over what the client
 thinks is best. Tested with httpd as the server and openssl as the
 client with two ciphers selected.

 Should we make this a configurable option (possibly on by default)?

Thanks - this has been on my todo list for a while. I think it needs to be a 
configuration option so that it can be disabled (in possibly strange use 
cases), however it should be on by default.

 Index: lib/libtls/tls_server.c
 ===
 RCS file: /cvs/src/lib/libtls/tls_server.c,v
 retrieving revision 1.7
 diff -u -p -r1.7 tls_server.c
 --- lib/libtls/tls_server.c   31 Mar 2015 14:03:38 -  1.7
 +++ lib/libtls/tls_server.c   15 May 2015 04:12:43 -
 @@ -81,6 +81,8 @@ tls_configure_server(struct tls *ctx)
   EC_KEY_free(ecdh_key);
   }

 + SSL_CTX_set_options(ctx-ssl_ctx, SSL_OP_CIPHER_SERVER_PREFERENCE);
 +
   /*
* Set session ID context to a random value.  We don't support
* persistent caching of sessions so it is OK to set a temporary

-- 

Action without study is fatal. Study without action is futile.
-- Mary Ritter Beard

softraid(4) RAID 5 - call for testing

[Joel Sing]

For those not following source-changes@, I have just re-enabled the RAID 5 
discipline for softraid(4).

During the last two hackathons in Dunedin, the RAID 5 implementation was 
largely rewritten. As far as I am aware, the last missing part was the lack 
of ability to resume a partial rebuild, which has been fixed - it now needs 
further testing and usage so that any remaining issues can be found.

Unfortunately I do not currently have access to my D2 disk array, which has 
limited the amount of testing that I can do - while the basics of creating 
and using a RAID 5 volume certainly help, the really useful testing also 
includes disk failures, rebuilds and partial rebuilds, along with data 
verification.

At this point I would probably trust it enough to put real data on it, however 
I cannot promise that no data eating bugs still exist - obviously real world 
usage should have real world backups (and probably far more frequently than 
normal).

If you do test and find problems, please report them either on this list or 
directly if you prefer. I would also be more than happy to hear of any 
successes (providing sufficient beating has been applied first!).
-- 

   Stop assuming that systems are secure unless demonstrated insecure;
start assuming that systems are insecure unless designed securely.
  - Bruce Schneier

Re: Do you need/prefer the non-DUID option in the installer?

[Joel Sing]

On Wednesday 01 April 2015, frantisek holop wrote:
 Theo de Raadt, 30 Mar 2015 18:09:
  IIRC 'bioctl -d' cannot deal with DUID's.
  not a showstopper, just sayin'
 
  Sounds like you might use this.  Want to trial a diff that adds
  support?  If it is wrong, don't worry, someone will hate your bad
  diff, and do it right.  (That is pretty much the history of DUID
  support in the tools)

 yes, i'd like to use it :)  i have numerous softraid
 encrypted usb dongles that i'd like to mount/unmount
 using DUID's.

 i started looking into this when i first reported it:
 http://marc.info/?l=openbsd-miscm=138198909623938w=2
 but it was not the one liner i hoped for :)
 i'll try to revisit.

 -f

FWIW this is now fixed in r1.351 of src/sys/dev/softraid.c.
-- 

Action without study is fatal. Study without action is futile.
-- Mary Ritter Beard

Re: LibreSSL 2.2 fails to connect to webdav.yandex.com

[Joel Sing]

On Saturday 06 June 2015, 1edhaz+9sj4olxjt6...@guerrillamail.com wrote:
 Hello,

 LibreSSL 2.2 (openbsd-current) fails to connect to
 https://webdav.yandex.com.

 OpenSSL 1.0.1m from OpenBSD packages does succeed.

 Yandex is the largest search engine in Russia. The webdav.yandex.com
 site is for accessing their file-hosting service.

 System info:

 $ uname -a
 OpenBSD roger.my.domain 5.7 GENERIC.MP#1039 amd64
 $ dmesg | head -n 1
 OpenBSD 5.7-current (GENERIC.MP) #1039: Wed Jun  3 12:09:31 MDT 2015

[snip]

The issue is due to the remote end not being RFC compliant and failing to 
complete a TLS handshake when it does not recognise TLS signature algorithms 
(sigalgs) that are being advertised by the client. In this case the new 
signature algorithms are related to GOST - almost the definition of irony... 

If you want to verify this for yourself, you can comment out the GOST related 
entries in the tls12_sigalgs array in t1_lib.c. HTTPS connections to 
www.yandex.com work without issue, so it would seemingly be the particular 
HTTP server that is being used for this service - I would recommend 
contacting Yandex and reporting the issue to them.
-- 

Action without study is fatal. Study without action is futile.
-- Mary Ritter Beard

Re: softraid checksumming discipline.

[Joel Sing]

On Wednesday 17 June 2015, Karel Gardas wrote:
 Hello,

 I'm curious if anybody is working on implementing block-level
 checksumming on softraid?

Not that I'm aware of.

 Backgroud: I'm comming from Solaris 11/ZFS world and I like ZFS's
 focus on data integrity from drive level up to the RAM. I've been
 thinking about OpenBSD and how to get the same with minimalistic
 effort (not porting ZFS) and I've though that having checksumming
 implemented in a virtual drive (softraid) may be the most easiest way.
 I think more easier than for example enhance ffs to include file-based
 checksums. Another issue is how to propagate block failures up to the
 file level, but for me it would be enough to just know something bad
 happening with the drive. At least for now. I hope discipline may be
 stacked on top of another so there is a possibility of using RAID1
 with checksumming disciplines on two drives, hence getting something
 similar to what I use now with ZFS (zpool with two drives in mirroring
 setup). If stacking is not possible for whatever reason, then I would
 probably go and clone and modify RAID1 to add checksum support (if
 feasible of course).

Stacking in softraid does work, but it is not officially supported (there are 
number of gotchas that you need to be aware of, such as the need to manually 
reassemble the volumes). It was never really designed to work this way and it 
results in I/O going through multiple layers unnecessarily. At some point it 
needs to be rearchitected so that it is stackable internally, which will then 
allow for a set of fixed but flexible disciplines.

Re adding some form of checksumming, it only seems to make sense in the case 
of RAID 1 where you can decide that the data on a disk is invalid, then fail 
the read and pull the data from another drive. That coupled with block 
level healing or similar could be interesting. Otherwise checksumming on 
its own is not overly useful at this level - you would simply fail a read, 
which then results in potentially worse than bit-flipping at higher layers.

If you wanted to investigate this I would suggest considering it as an option 
to the existing RAID 1 implementation. The bulk of it would be calculating 
and adding a checksum to each write and offsetting each block accordingly, 
along with verification on read. The failure modes would need to be thought 
through and handled - the re-reading from a different disk is already there, 
however what you then do with the failure is an open question (failing the 
chunk entirely is the heavy handed but already supported approach).

 Any comment on this topic welcome.

 Thanks,
 Karel

-- 

Action without study is fatal. Study without action is futile.
-- Mary Ritter Beard

Re: LibreSSL 2.2 fails to connect to webdav.yandex.com

[Joel Sing]

On Tuesday 09 June 2015, Alexey Ivanov wrote:
  On Jun 6, 2015, at 5:31 AM, Joel Sing j...@sing.id.au wrote:
 
  On Saturday 06 June 2015, 1edhaz+9sj4olxjt6...@guerrillamail.com wrote:
  Hello,
 
  LibreSSL 2.2 (openbsd-current) fails to connect to
  https://webdav.yandex.com.
 
  OpenSSL 1.0.1m from OpenBSD packages does succeed.
 
  Yandex is the largest search engine in Russia. The webdav.yandex.com
  site is for accessing their file-hosting service.
 
  System info:
 
  $ uname -a
  OpenBSD roger.my.domain 5.7 GENERIC.MP#1039 amd64
  $ dmesg | head -n 1
  OpenBSD 5.7-current (GENERIC.MP) #1039: Wed Jun  3 12:09:31 MDT 2015
 
  [snip]
 
  The issue is due to the remote end not being RFC compliant and failing to
  complete a TLS handshake when it does not recognise TLS signature
  algorithms (sigalgs) that are being advertised by the client. In this
  case the new signature algorithms are related to GOST - almost the
  definition of irony...

 GOST… lol indeed =)

  If you want to verify this for yourself, you can comment out the GOST
  related entries in the tls12_sigalgs array in t1_lib.c. HTTPS connections
  to www.yandex.com work without issue, so it would seemingly be the
  particular HTTP server that is being used for this service - I would
  recommend contacting Yandex and reporting the issue to them.

 He just did - Yandex is heavy BSD user, so many people there are reading
 tech@ and freebsd-hackers@. Some brave souls even subscribed to
 trolls@^Wmisc@!

 Back to the problem itself, as far as I know they are aware of it. In the
 meantime, while they are busy solving it on their side, you may want to
 limit ciphersuites client is using by calling `SSL_CTX_set_cipher_list`
 before `SSL_do_handshake`.

Except that would not have made any difference - currently the list of 
signature algorithms is static and not dependent on the cipher suites 
selected.

 PS. Anyway, next time you probably want to report libressl-related problems
 to recently announced libre...@openbsd.org [1].

 [1] http://comments.gmane.org/gmane.os.openbsd.tech/42319
-- 

Action without study is fatal. Study without action is futile.
-- Mary Ritter Beard

Re: [PATCH] libcrypto: initialize pointer

[Joel Sing]

On Friday 29 May 2015, Benjamin Baier wrote:
 Hello tech@

 buf.data is not initialized up front, which may lead to free(3)'ing a
 garbage pointer. Found by llvm/scan-build.
 Also free(3) handles NULL. No need to check.

At first glance this is not actually a real problem - free_cont is initialised 
to zero, then only set to one after buf.data has been initialised. That said, 
I'll take a closer look.

 Index: tasn_dec.c
 ===
 RCS file: /cvs/src/lib/libssl/src/crypto/asn1/tasn_dec.c,v
 retrieving revision 1.26
 diff -u -p -r1.26 tasn_dec.c
 --- tasn_dec.c19 Mar 2015 14:00:22 -  1.26
 +++ tasn_dec.c27 May 2015 18:40:34 -
 @@ -669,6 +669,8 @@ asn1_d2i_ex_primitive(ASN1_VALUE **pval,
   const unsigned char *cont = NULL;
   long len;

 + buf.data = NULL;
 +
   if (!pval) {
   ASN1err(ASN1_F_ASN1_D2I_EX_PRIMITIVE, ASN1_R_ILLEGAL_NULL);
   return 0; /* Should never happen */
 @@ -783,7 +785,7 @@ asn1_d2i_ex_primitive(ASN1_VALUE **pval,
   ret = 1;

  err:
 - if (free_cont  buf.data)
 + if (free_cont)
   free(buf.data);
   return ret;
  }



-- 

Action without study is fatal. Study without action is futile.
-- Mary Ritter Beard

Re: httpd: patch to close TLS sockets that fail before TLS handshake

[Joel Sing]

On Wednesday 15 July 2015 23:38:33 Jack Burton wrote:
 In 5.7-stable  -current, httpd, when listening for TLS, does not close
 the client socket when tls_accept_socket() returns any non-recoverable
 error. The problem manifests most often when a client connects but does
 not attempt TLS handshake.
 
 Steps to reproduce:
 
 * Configure httpd to listen for TLS requests
 * From a remote host, open TCP session to httpd on port 443
 * Close the TCP connection without sending any data
 * Wait at least 2 * MSL to avoid false positives
 * fstat continues to show the open socket for as long as httpd runs
 
 This causes the number of sockets httpd has opne to keep growing until
 it reaches (openfiles-cur - 7), at which point httpd stops responding
 to HTTPS requests altogether.
 
 Real world occurrence:
 
 We've seen this fairly regularly on a host with only modest HTTPS load,
 where httpd stops responding to HTTPS requests anywhere between 2 hours
 and 4 days after httpd restart.
 
 See the thread httpd stops accepting connections after a few hours on
 current on misc@ for further background and several independent reports
 of the observed behaviour.
 
 Despite the title of that thread, the same behaviour is seen in
 5.7-stable.
 
 Sorry, I don't have any hosts running -current at the moment, but I've
 written a trivial patch against 5.7-stable to treat that particular
 failure mode in the same way as was already being done for EV_TIMEOUTs.
 That fixes the issue for us here (been in place on one production host
 with a modest [2req/sec avg] load for 4 hours with no obvious
 regressions and no stale sockets -- previously we were getting at least
 several stale sockets appearing every hour). The good folks on misc@
 suggested I should post my patch to tech@, so here it is:
 
 --- usr.sbin/httpd/server.c.origWed Jul 15 20:40:16 2015
 +++ usr.sbin/httpd/server.c Wed Jul 15 20:50:15 2015
 @@ -932,6 +932,7 @@ server_accept_tls(int fd, short event, void *arg)
 struct client *clt = (struct client *)arg;
 struct server *srv = (struct server *)clt-clt_srv;
 int ret;
 + char *errmsg;
 
 if (event == EV_TIMEOUT) {
 server_close(clt, TLS accept timeout);
 @@ -952,8 +953,13 @@ server_accept_tls(int fd, short event, void *arg)
 server_accept_tls, clt-clt_tv_start,
 srv-srv_conf.timeout, clt);
 } else if (ret != 0) {
 - log_warnx(%s: TLS accept failed - %s, __func__,
 - tls_error(srv-srv_tls_ctx));
 + if (asprintf(errmsg, %s: TLS accept failed - %s,
 + __func__, tls_error(srv-srv_tls_ctx))  0) {
 + server_close(clt, server_accept_tls: TLS accept failed);
 + } else {
 + server_close(clt, errmsg);
 + free(errmsg);
 + }
 return;
 }
 
 This is the first time I've posted a patch for OpenBSD, so if I've erred
 in style or method please correct me and I'll try to do better next
 time.

Thanks for finding this and providing a diff. I've just committed a slightly 
different version, which simply adds the missing server_close() call - the 
version above will result in additional noise in the logs (both the function 
name and the full details from tls_error()), which we'd rather avoid.

Re: [PATCH] fix write error handling on SR RAID1

[Joel Sing]

On Friday 10 July 2015 22:01:43 Karel Gardas wrote:
 On Fri, Jul 10, 2015 at 9:34 PM, Chris Cappuccio ch...@nmedia.net wrote:
  My first impression, offlining the drive after a single chunk failure
  may be too aggressive as some errors are a result of issues other than
  drive failures.
 
 Indeed, it may look as too aggressive, but is my analysis written in
 comment correct? I mean: if there is a write error for whatever reason
 to one or more chunk(s) and if we completely ignore it since at least
 one write succeed, then arrays is in incorrect state where some
 drive(s) hold(s) correct data and another drive(s) hold(s) previous
 data. Since reading is done in round-robin fashion, then there is a
 chance that you will read old data in the future. If this is correct,
 then I think it calls for fix.

Your analysis is incorrect - offlining of chunks is handled via sr_ccb_done(). 
If lower level I/O indicates an error occurred then the chunk is marked 
offline, 
providing that the discipline has redundancy (for example, we do not offline 
chunks for RAID 0 or CRYPTO - it usually just makes things worse). This 
applies to both read and write operations. 

 If you do not like off-lining drive(s) just after 1 failed read, then
 perhaps correct may be to restart whole work unit and enforce writing
 again? We can even have some threshold where we may stop and consider
 the problematic block really not writeable at the end. Is something
 like that better solution?

We already offline after a single read or write failure occurs - it would be 
possible to implement some form of retry algorithm, however at some point we 
have to trust the lower layers (VFS, disk controller driver, disk hardware, 
etc).

Re: httpd: patch to close TLS sockets that,fail before TLS handshake

[Joel Sing]

On Tuesday 25 August 2015 19:19:58 Edgar Pettijohn wrote:
 I was curious if this issue is fixed in -current or if there is going to
 be a patch available on the errata page?

Yes, this is fixed in -current (and will be in 5.8) - see r1.68 of server.c. 
There may be back ports/commits of various httpd fixes at some point, but at 
this stage there I'm not sure there will be errata.

libtls: ALPN support

[Joel Sing]

The following diff adds ALPN support to libtls via:

tls_config_set_alpn() - set the ALPN protocols supported by this client/server
tls_conn_alpn_selected() - get the ALPN protocol selected for this connection

ok?

Index: tls.c
===
RCS file: /cvs/src/lib/libtls/tls.c,v
retrieving revision 1.41
diff -u -p -r1.41 tls.c
--- tls.c   7 Jul 2016 14:09:03 -   1.41
+++ tls.c   27 Jul 2016 16:57:06 -
@@ -310,6 +310,14 @@ tls_configure_ssl(struct tls *ctx)
if ((ctx->config->protocols & TLS_PROTOCOL_TLSv1_2) == 0)
SSL_CTX_set_options(ctx->ssl_ctx, SSL_OP_NO_TLSv1_2);
 
+   if (ctx->config->alpn != NULL) {
+   if (SSL_CTX_set_alpn_protos(ctx->ssl_ctx, ctx->config->alpn,
+   ctx->config->alpn_len) != 0) {
+   tls_set_errorx(ctx, "failed to set alpn");
+   goto err;
+   }
+   }
+
if (ctx->config->ciphers != NULL) {
if (SSL_CTX_set_cipher_list(ctx->ssl_ctx,
ctx->config->ciphers) != 1) {
Index: tls.h
===
RCS file: /cvs/src/lib/libtls/tls.h,v
retrieving revision 1.29
diff -u -p -r1.29 tls.h
--- tls.h   27 May 2016 14:21:24 -  1.29
+++ tls.h   27 Jul 2016 16:57:06 -
@@ -52,6 +52,7 @@ const char *tls_error(struct tls *_ctx);
 struct tls_config *tls_config_new(void);
 void tls_config_free(struct tls_config *_config);
 
+int tls_config_set_alpn(struct tls_config *_config, const char *_alpn);
 int tls_config_set_ca_file(struct tls_config *_config, const char *_ca_file);
 int tls_config_set_ca_path(struct tls_config *_config, const char *_ca_path);
 int tls_config_set_ca_mem(struct tls_config *_config, const uint8_t *_ca,
@@ -116,8 +117,9 @@ const char *tls_peer_cert_subject(struct
 time_t tls_peer_cert_notbefore(struct tls *_ctx);
 time_t tls_peer_cert_notafter(struct tls *_ctx);
 
-const char *tls_conn_version(struct tls *_ctx);
+const char *tls_conn_alpn_selected(struct tls *_ctx);
 const char *tls_conn_cipher(struct tls *_ctx);
+const char *tls_conn_version(struct tls *_ctx);
 
 uint8_t *tls_load_file(const char *_file, size_t *_len, char *_password);
 
Index: tls_config.c
===
RCS file: /cvs/src/lib/libtls/tls_config.c,v
retrieving revision 1.22
diff -u -p -r1.22 tls_config.c
--- tls_config.c13 Jul 2016 16:30:48 -  1.22
+++ tls_config.c27 Jul 2016 16:57:06 -
@@ -166,6 +166,7 @@ tls_config_free(struct tls_config *confi
 
free(config->error.msg);
 
+   free(config->alpn);
free((char *)config->ca_file);
free((char *)config->ca_mem);
free((char *)config->ca_path);
@@ -247,6 +248,72 @@ tls_config_parse_protocols(uint32_t *pro
free(s);
 
return (0);
+}
+
+static int
+tls_config_parse_alpn(struct tls_config *config, const char *alpn,
+char **alpn_data, size_t *alpn_len)
+{
+   size_t buf_len, i, len;
+   char *buf = NULL;
+   char *s = NULL;
+   char *p, *q;
+
+   if ((buf_len = strlen(alpn) + 1) > 65535) {
+   tls_config_set_errorx(config, "alpn too large");
+   goto err;
+   }
+
+   if ((buf = malloc(buf_len)) == NULL) {
+   tls_config_set_errorx(config, "out of memory");
+   goto err;
+   }
+
+   if ((s = strdup(alpn)) == NULL) {
+   tls_config_set_errorx(config, "out of memory");
+   goto err;
+   }
+
+   i = 0;
+   q = s;
+   while ((p = strsep(, ",")) != NULL) {
+   if ((len = strlen(p)) == 0) {
+   tls_config_set_errorx(config,
+   "alpn protocol with zero length");
+   goto err;
+   }
+   if (len > 255) {
+   tls_config_set_errorx(config,
+   "alpn protocol too long");
+   goto err;
+   }
+   buf[i++] = len & 0xff;
+   memcpy([i], p, len);
+   i += len;
+   }
+
+   free(s);
+
+   *alpn_data = buf;
+   *alpn_len = buf_len;
+
+   return (0);
+
+ err:
+   free(buf);
+   free(s);
+
+   *alpn_data = NULL;
+   *alpn_len = 0;
+
+   return (-1);
+}
+
+int
+tls_config_set_alpn(struct tls_config *config, const char *alpn)
+{
+   return tls_config_parse_alpn(config, alpn, >alpn,
+   >alpn_len);
 }
 
 int
Index: tls_conninfo.c
===
RCS file: /cvs/src/lib/libtls/tls_conninfo.c,v
retrieving revision 1.5
diff -u -p -r1.5 tls_conninfo.c
--- tls_conninfo.c  7 Oct 2015 23:33:38 -   1.5
+++ tls_conninfo.c  27 Jul 2016 16:57:06 -
@@ -150,6 +150,26 @@ tls_get_peer_cert_times(struct tls *ctx,
return (rv);
 }
 

httpd: Add SNI support

[Joel Sing]

The following enables SNI support within httpd.

It requires libtls to have server side support for SNI (diff previously
posted).

Index: server.c
===
RCS file: /cvs/src/usr.sbin/httpd/server.c,v
retrieving revision 1.85
diff -u -p -r1.85 server.c
--- server.c28 Apr 2016 17:18:06 -  1.85
+++ server.c13 Aug 2016 17:18:51 -
@@ -159,6 +159,8 @@ server_tls_load_keypair(struct server *s
 int
 server_tls_init(struct server *srv)
 {
+   struct server_config *srv_conf;
+
if ((srv->srv_conf.flags & SRVFLAG_TLS) == 0)
return (0);
 
@@ -207,6 +209,19 @@ server_tls_init(struct server *srv)
return (-1);
}
 
+   TAILQ_FOREACH(srv_conf, >srv_hosts, entry) {
+   if (srv_conf->tls_cert == NULL || srv_conf->tls_key == NULL)
+   continue;
+   log_debug("%s: adding keypair for server %s", __func__,
+   srv->srv_conf.name);
+   if (tls_config_add_keypair_mem(srv->srv_tls_config,
+   srv_conf->tls_cert, srv_conf->tls_cert_len,
+   srv_conf->tls_key, srv_conf->tls_key_len) != 0) {
+   log_warnx("%s: failed to add tls keypair", __func__);
+   return (-1);
+   }
+   }
+
if (tls_configure(srv->srv_tls_ctx, srv->srv_tls_config) != 0) {
log_warnx("%s: failed to configure TLS - %s", __func__,
tls_error(srv->srv_tls_ctx));
@@ -261,6 +276,9 @@ server_launch(void)
struct server   *srv;
 
TAILQ_FOREACH(srv, env->sc_servers, srv_entry) {
+   log_debug("%s: configuring server %s", __func__,
+   srv->srv_conf.name);
+
server_tls_init(srv);
server_http_init(srv);
 

httpd: be stricter with TLS configuration

[Joel Sing]

The following diff makes httpd stricter with respect to TLS configuration:

- Do not allow TLS and non-TLS to be configured on the same port.
- Do not allow TLS options to be specified without a TLS listener.
- Ensure that TLS options are the same when a server is specified on the
  same address/port.

Currently, these configurations are permitted but do not work as intended.

This also factors out (and reuses) the server matching code that was already
duplicated.

ok?

Index: parse.y
===
RCS file: /cvs/src/usr.sbin/httpd/parse.y,v
retrieving revision 1.78
diff -u -p -r1.78 parse.y
--- parse.y 21 Jun 2016 21:35:24 -  1.78
+++ parse.y 12 Aug 2016 16:48:28 -
@@ -107,8 +107,10 @@ int host_if(const char *, struct addre
 int host(const char *, struct addresslist *,
int, struct portrange *, const char *, int);
 voidhost_free(struct addresslist *);
+struct server  *server_match(struct server *, int);
 struct server  *server_inherit(struct server *, struct server_config *,
struct server_config *);
+int server_tls_cmp(struct server *, struct server *);
 int getservice(char *);
 int is_if_in_group(const char *, const char *);
 
@@ -283,24 +285,13 @@ server: SERVER optmatch STRING{
 
TAILQ_INSERT_TAIL(>srv_hosts, srv_conf, entry);
} '{' optnl serveropts_l '}'{
-   struct server   *s = NULL, *sn;
+   struct server   *s, *sn;
struct server_config*a, *b;
 
srv_conf = >srv_conf;
 
-   TAILQ_FOREACH(s, conf->sc_servers, srv_entry) {
-   if ((s->srv_conf.flags &
-   SRVFLAG_LOCATION) == 0 &&
-   strcmp(s->srv_conf.name,
-   srv->srv_conf.name) == 0 &&
-   s->srv_conf.port == srv->srv_conf.port &&
-   sockaddr_cmp(
-   (struct sockaddr *)>srv_conf.ss,
-   (struct sockaddr *)>srv_conf.ss,
-   s->srv_conf.prefixlen) == 0)
-   break;
-   }
-   if (s != NULL) {
+   /* Check if the new server already exists. */
+   if (server_match(srv, 1) != NULL) {
yyerror("server \"%s\" defined twice",
srv->srv_conf.name);
serverconfig_free(srv_conf);
@@ -315,16 +306,39 @@ server: SERVER optmatch STRING{
YYERROR;
}
 
+   if ((s = server_match(srv, 0)) != NULL) {
+   if ((s->srv_conf.flags & SRVFLAG_TLS) != 
+   (srv->srv_conf.flags & SRVFLAG_TLS)) {
+   yyerror("server \"%s\": TLS and "
+   "non-TLS on same address/port",
+   srv->srv_conf.name);
+   serverconfig_free(srv_conf);
+   free(srv);
+   YYERROR;
+   }
+   if (server_tls_cmp(s, srv) != 0) {
+   yyerror("server \"%s\": TLS "
+   "configuration mismatch on same "
+   "address/port",
+   srv->srv_conf.name);
+   serverconfig_free(srv_conf);
+   free(srv);
+   YYERROR;
+   }
+   }
+
if ((srv->srv_conf.flags & SRVFLAG_TLS) &&
srv->srv_conf.tls_protocols == 0) {
-   yyerror("no TLS protocols");
+   yyerror("server \"%s\": no TLS protocols",
+   srv->srv_conf.name);
+   serverconfig_free(srv_conf);
free(srv);
YYERROR;
}
 
if (server_tls_load_keypair(srv) == -1) {
-   yyerror("failed to load public/private keys "
-   "for server %s", srv->srv_conf.name);
+   yyerror("server \"%s\": failed to load "
+  

Re: ntpd && pledge

[Joel Sing]

On Thursday 07 July 2016 00:32:04 Ian Mcwilliam wrote:
> Seems changes to pledge have made ntpd abort.
> 
> ntpd(67855): syscall 5 "rpath"
> ntpd(81479): syscall 5 "rpath"
> 
> Jul  7 10:29:23 ianm-openbsd ntpd[76119]: constraint
> 2404:6800:4006:800::2004; terminated with signal 6 (Abort trap)

Thanks - this was actually due to a change to libtls, which has been reverted.

Re: httpd: be stricter with TLS configuration

[Joel Sing]

On Monday 15 August 2016 13:04:43 Reyk Floeter wrote:
> On Sat, Aug 13, 2016 at 02:57:14AM +1000, Joel Sing wrote:
> > The following diff makes httpd stricter with respect to TLS configuration:
> > 
> > - Do not allow TLS and non-TLS to be configured on the same port.
> > - Do not allow TLS options to be specified without a TLS listener.
> > - Ensure that TLS options are the same when a server is specified on the
> > 
> >   same address/port.
> > 
> > Currently, these configurations are permitted but do not work as intended.
> > 
> > This also factors out (and reuses) the server matching code that was
> > already duplicated.
> > 
> > ok?
> 
> - I think server_match() and server_tls_cmp() can both live in
> server.c (server_match() somewhere close to server_foreach() - this
> match function can be used for at least one other case outside of
> parse.y).

I've moved server_tls_cmp() to server.c, however I'm not sure it makes sense 
for server_match() since it operates on conf (a global declared in parse.y) 
and I do not see any identical matching (the closest seems to be the code in 
server_privinit(), but it still differs).
 
> - As discussed before, for consistency with the config, please use
> "tls" instead of "TLS" in the log messages.

Agreed, I'll handle this in a separate commit.
 
> FYI, The SNI diff doesn't like the tls_cert_file and tls_key_file
> checks in server_tls_cmp(), as they now become valid, but they can be
> removed/changed later.

Yes, they are independent diffs and we'll need to relax the restrictions 
slightly when we enable SNI.

> Otherwise OK reyk@

Thanks.

Re: specify curves via ecdhe statement in httpd.conf

[Joel Sing]

On Sunday 05 February 2017 17:05:40 Andreas Bartelt wrote:
> > - What type of public certificate are you using (RSA or ECDSA)?
> 
> ECDSA with P-256. Certificate signed by letsencrypt (via RSA).
> Must-staple is enabled - that's why I'm also using the ocsp line for
> testing.

Ah, this was the missing piece of information.

In order to use ECDSA the client must support the curve used for the server 
certificate, otherwise when the server signs the server key exchange, the 
client will not be able to verify the signature. In the case where you 
announce that the client only supports P-384, any ECDSA ciphers are considered 
to be invalid for this session, effectively resulting in no shared ciphers and 
the handshake failure alert.

In order for this configuration to work you need to include P-256 in the client 
supported groups. Specifying groups as "P-384:P-256" should still get you 
P-384, depending on the server configuration and whether the preference for a 
curve is based on the client or server preference (for libtls and hence httpd, 
it will be server preference).

Re: specify curves via ecdhe statement in httpd.conf

[Joel Sing]

On Saturday 04 February 2017 15:51:02 Andreas Bartelt wrote:
> On 02/04/17 05:26, Joel Sing wrote:
> > On Wednesday 01 February 2017 15:41:29 Andreas Bartelt wrote:
> >> Hello,
> >> 
> >> after reading the LibreSSL accouncement from today, I assumed that
> >> specifying ecdhe "auto" in /etc/httpd.conf would enable X25519, P-256
> >> and P-384 on current.
> > 
> > This is correct.
> > 
> >> I've noticed that "auto" enables only curves x25519 and P-256 (which is
> >> what I'd want to use - but somehow unexpected with regard to the
> >> announcement).
> > 
> > Why do you believe this is the case?
> 
> Tested with a build of today's current:
> - httpd started with ecdhe "auto" in /etc/httpd.conf
> - then trying to connect via openssl s_client with -groups P-384 option
> doesn't negotiate a cipher suite.
> 
> However, specifying -groups P-256 works. I don't know how to specify
> x25519 with OpenBSD's openssl s_client (it's not yet listed in openssl
> ecparam -list_curves output) but SSL Labs successfully negotiates via
> x25519 and P-256 (but not P-384). P-384 doesn't seem to be enabled with
> "auto".

You can just specify X25519 as a group - it will not appear in `openssl 
ecparam -list_curves' since it is not a standard EC curve.

> Another confusing test result:
> - httpd started with ecdhe "secp384r1" (P-384)
> - then trying to connect via openssl s_client with -groups P-384 option
> also doesn't negotiate a cipher suite!
> 
> However, SSL Labs successfully connects to httpd and confirms support
> for secp384r1.
> 
> Can you reproduce this?

No, it works correctly for me (OpenBSD -current, amd64).

With "tls ecdhe auto":

$ for group in X25519 P-256 P-384; do openssl s_client -connect localhost:443 
-groups $group &1 | grep 'Server Temp Key:'; done
Server Temp Key: ECDH, X25519, 253 bits
Server Temp Key: ECDH, P-256, 256 bits
Server Temp Key: ECDH, P-384, 384 bits

With "tls ecdhe secp384r1":

 $ for group in X25519 P-256 P-384; do openssl s_client -connect localhost:443 
-groups $group &1 | grep 'Server Temp Key:'; done  
Server Temp Key: ECDH, P-384, 384 bits

> >> Diff is attached which clarifies the meaning of "auto" in httpd.conf.5.
> > 
> > There are some documentation improvements that could be used here, however
> > the meaning of auto for httpd.conf.5 needs to refer to the meaning of
> > "auto" for libtls (currently tls_config_set_ecdhecurve()). Otherwise
> > libtls changes and httpd becomes out of date.
> > 
> >> There currently seems to be no way to explicitly specify x25519, or to
> >> specify multiple colon separated curves with the ecdhe statement. Would
> >> it make sense to change semantics and make the ecdhe statement in
> >> httpd.conf consistent with the recent changes to openssl s_client
> >> -groups (e.g., to also allow more common names like P-256 instead of
> >> prime256v1)?
> > 
> > Yes - tls_config_set_ecdhecurve() needs to change to accept the same colon
> > separate list of priority ordered curve names, that SSL_set1_curves_list()
> > accepts.

Re: specify curves via ecdhe statement in httpd.conf

[Joel Sing]

On Monday 06 February 2017 20:18:48 Andreas Bartelt wrote:
> Yes, right - thanks. I wasn't aware that this is actually a MUST
> requirement from RFC 4492. I'm quite surprised that the "Supported
> Elliptic Curves Extension" is also used in order to specify any allowed
> curves for use in the context of certificates. I think this is quite
> inconsistent but it's what this RFC seems to mandate. It's inconsistent
> because there is no such binding with regard to -ECDHE-RSA- or -DHE-RSA-
> cipher suites.

Both DHE and RSA are either supported or they are not. If they are not then 
the client should not include cipher suites that use these key exchange and/or 
authentication methods. ECDHE and ECDSA are somewhat different, the main reason 
being the use of named curves vs arbitrary curves. If named curves were not in 
use then more data has to be provided in the Server Key Exchange and it is 
more like a DHE key exchange (see RFC 4492 section 5.4 for more details if 
you're interested). The trade off is effectively providing EC parameters to the 
client versus requiring the client and server to support the same named 
curves.
 
> I've successfully tested a P-384-based certificate which allowed me to
> explicitly specify -groups secp384r1 for ECDHE on the client side.

Excellent.

> I think it would be beneficial to allow to explicitly specify multiple
> groups with the ecdhe statement in httpd.conf, and also to respect their
> order.

Absolutely - as noted in an earlier reply, this is what is planned.

Re: specify curves via ecdhe statement in httpd.conf

[Joel Sing]

On Sunday 05 February 2017 11:13:16 Andreas Bartelt wrote:
> On 02/05/17 07:41, Joel Sing wrote:
> > You can just specify X25519 as a group - it will not appear in `openssl
> > ecparam -list_curves' since it is not a standard EC curve.
> 
> thanks - I didn't notice that capitalization is important here. Maybe
> x25519 and ecdh_x25519 [IANA] should also be accepted as valid names
> [http://www.iana.org/assignments/tls-parameters/tls-parameters.xhtml].

I'll consider this, however all of the RFCs refer to it as X25519 (e.g. 
RFC7748 and draft-ietf-curdle-pkix-03). This is also no different to the fact 
that you cannot use 'Prime256v1' or 'p-384'.

> The definition of the curve itself is provided in RFC 7748 - RFCs for
> some other listed curves (e.g., brainpool) are also only tagged as
> Informational. What is missing with regard to X25519?

There is nothing missing as such - X25519 is a function that performs scalar 
multiplication on a curve known as curve25519. The X25519 function is in turn 
used to perform Diffie-Hellman key exchange. Neither X25519 nor curve25519 fit 
the OpenSSL Elliptic Curve API (largely due to design), hence it does not make 
sense for it to appear in 'openssl ecparam -list_curves', which would require 
it to be treated and manipulated as if it was a standard EC curve.
 
> This is really weird. Although my test results for X25519 were similarly
> confusing -- for simplicity, I'll provide some more results which were
> restricted to explicitly testing secp384r1.
[snip]
> Error messages when failing against httpd:
>  > openssl s_client -connect :443 -servername 
> 
> -groups secp384r1
> CONNECTED(0003)
> 1225438578:error:14FFF410:SSL routines:SSL_internal:sslv3 alert
> handshake failure:/usr/src/lib/libssl/ssl_pkt.c:1205:SSL alert number 40
> 1225438578:error:14FFF0E5:SSL routines:SSL_internal:ssl handshake
> failure:/usr/src/lib/libssl/ssl_pkt.c:585:

This is the server-side responding with a fatal SSL handshake failure alert - 
there are only a few cases where this will happen, the most likely of which is 
when there is no matching cipher suite.

- Is there any other configuration that would limit the cipher suites in use?

- What cipher suite is used if you connect without specifying -groups?

- What type of public certificate are you using (RSA or ECDSA)?

- If you're still unable to get to the bottom of it, try running 'openssl 
s_client' with -debug and provide the output.

Re: libressl-2.5.1 patches

[Joel Sing]

Thanks for providing some patches, however a few things to note:

- Could you please resend the diffs inline - it makes them much easier to 
review and provide feedback/discussion on.

- When generating diffs please create unified diffs (generally `diff -uNp').

- When sending a change, please include a description of the change (for 
example, if this was actually committed, what should the commit message be) 
and even more importantly explain why you're proposing the change be made.

On Tuesday 07 February 2017 08:06:10 John Boyd wrote:
> 

Re: specify curves via ecdhe statement in httpd.conf

[Joel Sing]

On Wednesday 01 February 2017 15:41:29 Andreas Bartelt wrote:
> Hello,
> 
> after reading the LibreSSL accouncement from today, I assumed that
> specifying ecdhe "auto" in /etc/httpd.conf would enable X25519, P-256
> and P-384 on current.

This is correct.

> I've noticed that "auto" enables only curves x25519 and P-256 (which is
> what I'd want to use - but somehow unexpected with regard to the
> announcement).

Why do you believe this is the case?

> Diff is attached which clarifies the meaning of "auto" in httpd.conf.5.

There are some documentation improvements that could be used here, however the 
meaning of auto for httpd.conf.5 needs to refer to the meaning of "auto" for 
libtls (currently tls_config_set_ecdhecurve()). Otherwise libtls changes and 
httpd becomes out of date.

> There currently seems to be no way to explicitly specify x25519, or to
> specify multiple colon separated curves with the ecdhe statement. Would
> it make sense to change semantics and make the ecdhe statement in
> httpd.conf consistent with the recent changes to openssl s_client
> -groups (e.g., to also allow more common names like P-256 instead of
> prime256v1)?

Yes - tls_config_set_ecdhecurve() needs to change to accept the same colon 
separate list of priority ordered curve names, that SSL_set1_curves_list() 
accepts.

Re: attach SR drive by force even if not all chunks provide native metadata

[Joel Sing]

On Saturday 24 September 2016 00:13:47 Karel Gardas wrote:
> Hello,
> 
> following patch fixes issue while attempting to attach SR RAID1 drive
> where not all chunks provide native metadata. I.e. one chunk is dd
> zeroed. The complain of SR is good one, but I'd think that force
> parameter should overcome it and really enforce SR to attach such
> drive.

I'll need to look more closely, but I'm pretty certain this is not correct - 
if there is no native metadata on the chunk, then it should not be considered 
to be part of the volume. In the case of an SR RAID1 volume, if you have a 
chunk that was zeroed, then you should be rebuilding on to it, rather than 
bringing it up as an existing part of the volume.
 
> Thanks,
> Karel
> 
> diff -u -p -u -r1.377 softraid.c
> --- softraid.c  20 Jul 2016 20:45:13 -  1.377
> +++ softraid.c  23 Sep 2016 22:06:55 -
> @@ -1658,7 +1661,7 @@ sr_meta_native_attach(struct sr_discipli
> not_sr++;
> }
> 
> -   if (sr && not_sr) {
> +   if (sr && not_sr && !force) {
> sr_error(sc, "not all chunks are of the native metadata "
> "format");
> goto bad;

OpenSSL 1.1 API migration path (or the lack thereof...)

[Joel Sing]

As many of you will already be aware, the OpenSSL 1.1.0 release intentionally
introduced significant API changes from the previous release[0][1]. In summary,
a large number of data structures that were previously publically visible have
been made opaque, with accessor functions being added in order to get and
set some of the fields within these now opaque structs. It is worth noting that
the use of opaque data structures is generally beneficial for libraries, since
changes can be made to these data structures without breaking the ABI. As 
such, the overall direction of these changes is largely reasonable.

However, while API change is generally necessary for progression, in this case
it would appear that there is NO transition plan and a complete disregard for
the impact that these changes would have on the overall open source ecosystem.

So far it seems that the only approach is to place the migration burden onto
each and every software project that uses OpenSSL, pushing significant code
changes to each project that migrates to OpenSSL 1.1, while maintaining
compatibility with the previous API (such as [2] and [3]). This is forcing
each project to provide their own backwards compatibility shims, which is
practically guaranteeing that there will be a proliferation of variable
quality implementations; it is almost a certainty that some of these will
contain bugs, potentially introducing security issues or memory leaks.

Due to a number of factors, software projects that make use of OpenSSL cannot
simply migrate to the 1.1 API and drop support for the 1.0 API - in most cases
they will need to continue to support both. Firstly, I am not aware of any
platform that has shipped a production release with OpenSSL 1.1 - any software
that supported OpenSSL 1.1 only, would effectively be unusable on every
platform for the time being. Secondly, the OpenSSL 1.0.2 release is supported
until the 31st of December 2019[4], while OpenSSL 1.1.0 is only supported
until the 31st of August 2018[4] - any LTS style release is clearly going to
consider shipping with 1.0.2 as a result.

Platforms that are attempting to ship with OpenSSL 1.1 are already
encountering significant challenges - for example, Debian currently has 257
packages (out of 518) that do not build against OpenSSL 1.1[5][6]. There are
also hidden gotchas for situations where different libraries are linked against
different OpenSSL versions and then share OpenSSL data structures between
them[7] - many of these problems will be difficult to detect since they only
fail at runtime.

But here's the thing - there are at least two options that OpenSSL had (and
still have) that would have avoided this situation and allowed for software
projects to make a smooth transition from the old API to the new API, without
every single project carrying backwards compatibility glue for at least the
next three years (and in reality, much longer).
 
I would implore the OpenSSL project to seriously reconsider their approach
to this API change and more importantly the associated migration, especially
keeping in mind what is going to be best for the overall ecosystem and not
just for the OpenSSL project. I would also ask software projects that make use
of OpenSSL to think carefully about how they approach this situation and in
particular consider how long they will need to carry compatibility code and
#ifs for.
 
Last but not least I would like to note that this is not a LibreSSL problem -
if we added all of the OpenSSL 1.1 accessors tomorrow, software written for
OpenSSL 1.0 or OpenSSL 1.1 would work seamlessly with LibreSSL. However,
software will still have to maintain compatibility with both of the OpenSSL
APIs, regardless of what we do.

[0] https://wiki.openssl.org/index.php/1.1_API_Changes

[1] https://abi-laboratory.pro/tracker/timeline/openssl/

[2] 
https://github.com/php/php-src/commit/2ecce94756bebda9eca3084b586f5bc821174c50

[3] https://github.com/openssh/openssh-portable/pull/48/files

[4] https://www.openssl.org/policies/releasestrat.html

[5] https://wiki.debian.org/OpenSSL-1.1

[6] https://breakpoint.cc/openssl-trans/html/openssl.html

[7] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=844018

Re: Fix memory leak in LibreSSL/tls_conninfo_free()

[Joel Sing]

On Sunday 08 January 2017 07:59:34 Shuo Chen wrote:
> Valgrind finds out that conninfo->servername is not free()d by
> tls_conninfo_free().
> 
> == HEAP SUMMARY:
> == in use at exit: 83,069 bytes in 2,690 blocks
> ==   total heap usage: 4,107 allocs, 1,417 frees,
> == 339,660 bytes allocated
> ==
> == 17 bytes in 1 blocks are definitely lost in loss record 1 of 266
> ==at 0x4C28C20: malloc (vg_replace_malloc.c:296)
> ==by 0x58F5989: strdup (strdup.c:42)
> ==by 0x40B2C4: tls_conninfo_populate
> ==by 0x408C4F: tls_handshake
> ==by 0x403691: TlsContext::handshake()
> ==by 0x403343: TlsStream::connect(TlsConfig*, char const*,
> ==by 0x407781: main (in /home/schen/recipes/ssl/client)
> ==
> == LEAK SUMMARY:
> ==definitely lost: 17 bytes in 1 blocks
> ==indirectly lost: 0 bytes in 0 blocks
> ==  possibly lost: 0 bytes in 0 blocks
> ==still reachable: 83,052 bytes in 2,689 blocks
> == suppressed: 0 bytes in 0 blocks
> 
> Here's a quick fix.

Committed, thanks!
 
> 
> diff --git a/tls/tls_conninfo.c b/tls/tls_conninfo.c
> --- a/tls/tls_conninfo.c
> +++ b/tls/tls_conninfo.c
> @@ -248,6 +248,8 @@ tls_conninfo_free(struct tls_conninfo *conninfo)
>   conninfo->alpn = NULL;
>   free(conninfo->cipher);
>   conninfo->cipher = NULL;
> + free(conninfo->servername);
> + conninfo->servername = NULL;
>   free(conninfo->version);
>   conninfo->version = NULL;

Re: OpenBSD 6.0 relayd TLS Loadbalancer failing SSL Lab tests (Cipher Support)

[Joel Sing]

On Thursday 06 April 2017 16:38:26 Tom Smyth wrote:
> Hello all,
> 
> I was installing relayd as a loadbalancer (and ssl terminator)  on
> OpenBSD6.0
> amd64 base install,
> 
> I used the following configuration for my /etc/relayd.conf file
> 
> http protocol https {
> match request header append "X-Forwarded-For" value "$REMOTE_ADDR"
> match request header append "X-Forwarded-By" \
> value "$SERVER_ADDR:$SERVER_PORT"
> match request header append "X-Forwarded-Proto" value "https"
> match request header set "Connection" value "close"
> tls { no tlsv1.0, ciphers HIGH }
> }
> 
> The Site I used to test was
> https://www.ssllabs.com/ssltest/
> 
> according to qualys the result for my site was a fail (F)
> due to the following ciphers being supported by relayd / LibreTLS

The relayd cipher string is passed through to libssl, hence you'll get 
whatever you specify. There are potential use cases for anonymous ciphers and 
for various historical reasons OpenSSL includes (almost) everything by 
default. What you want is "HIGH:!aNULL", rather than just "HIGH" which 
includes aNULL (null authentication/anonymous) ciphers.

You can check what ciphers you're actually specifying via openssl(1):

$ openssl ciphers HIGH:aNULL

As an aside, if you did not specify your own ciphers and used the relayd 
defaults, you would get an appropriate/correct configuration.

If you want something that is even more secure use the libtls default of 
"TLSv1.2+AEAD+ECDHE:TLSv1.2+AEAD+DHE", which will give you TLSv1.2 only cipher 
suites with AEAD and PFS.

> TLS_ECDH_anon_WITH_AES_256_CBC_SHA (0xc019)   INSECURE 256
> TLS_ECDH_anon_WITH_AES_128_CBC_SHA (0xc018)   INSECURE 128
> TLS_ECDH_anon_WITH_3DES_EDE_CBC_SHA (0xc017)   INSECURE 112
> 
> I was wondering if these ciphers could be disabled by default
> in the upcoming release (if not already done so) I will investigate
> selecting ciphers manually to exclude those ciphers in the mean time.
> 
> Thanks for your Time,

Re: [diff] httpd: tls client cert & CRL checks

[Joel Sing]

On Saturday 29 July 2017 20:49:18 Jan Klemkow wrote:
> Hi Jack,
> 
> On Fri, Jul 28, 2017 at 02:05:34AM +0930, Jack Burton wrote:
> > On Thu, 27 Jul 2017 13:10:14 +0200
> > 
> > > But, I found a bug in the part of the FastCGI variables.  The
> > > following condition is always false.
> > > 
> > > > Index: usr.sbin/httpd/server_fcgi.c
> > > > ===
> > > > RCS file: /cvs/src/usr.sbin/httpd/server_fcgi.c,v
> > > > retrieving revision 1.74
> > > > diff -u -p -r1.74 server_fcgi.c
> > > > --- usr.sbin/httpd/server_fcgi.c21 Jan 2017 11:32:04
> > > > -   1.74 +++ usr.sbin/httpd/server_fcgi.c   21 Jul
> > > > 2017 08:25:57 - @@ -282,11 +283,57 @@ server_fcgi(struct httpd
> > > > *env, struct cl
> > > 
> > > ...
> > > 
> > > > +   if (srv_conf->tls_ca != NULL) {
> > > 
> > > ...
> > 
> > That's odd -- I'm not seeing that behaviour here -- in my tests
> > srv_conf->tls_ca always behaved just as expected (it's NULL iff the "tls
> > client ca" directive is not given for that server).
> 
> In the End, I found and fixed the real bug here:
> 
> @@ -430,7 +438,11 @@ config_getserver_config(struct httpd *en
> }
> 
> f = SRVFLAG_TLS;
> -   srv_conf->flags |= parent->flags & f;
> +   if ((srv_conf->flags & f) == 0) {
> +   srv_conf->flags |= parent->flags & f;
> +   srv_conf->tls_ca = parent->tls_ca;
> +   srv_conf->tls_crl = parent->tls_crl;

I'd have to double check, however I'm pretty sure that this will result in a 
double-free since you're copying the pointer (without a reference count) 
across server config structs. Both will likely be passed to 
serverconfig_free(), 
which means free() will then be passed the same pointer twice.

> +   }
> 
> f = SRVFLAG_ACCESS_LOG;
> if ((srv_conf->flags & f) == 0) {
> 
> This additional copy fixes the bug I have seen by this config:
> 
> server "default" {
> listen on 127.0.0.1 tls port 443
> 
> # TLS certificate and key files created with acme-client(1)
> tls certificate "/root/ca/server.crt"
> tls key "/root/ca/server.key"
> #tls client ca "/root/ca/ca.crt" crl "/root/ca/ca.crl"
> tls client ca "/root/ca/ca.crt"
> 
> location "*.cgi" {
> fastcgi
> root "/var/www/cgi-bin/env.cgi"
> }
> 
> root "/htdocs/"
> }
> 
> You find the whole diff below.
> I tested:
>  - TLS without client certs
>  - TLS with client certs and without CRL
>  - TLS with client certs and with CRL
>  - as well as environment variables in CGI-Scripts
> 
> Everything should work now.
> 
> Bye,
> Jan

Re: [PATCH 1/2] nc: support -T tlscompat option

[Joel Sing]

On Thursday 18 May 2017 07:03:31 Kyle J. McKay wrote:
> Some services are still provided using TLS 1.0 and older ciphers.
> It is possible to use the nc command to connect to these services
> using the "-T tlsall" option, but that also enables legacy and
> insecure ciphers and is not desirable.
> 
> Instead add a new "-T tlscompat" option that can be used to access
> older servers while not also enabling insecure and very old legacy
> ciphers possibly allowing them to be unintentionally used (perhaps
> because of a server misconfiguration).

I'm not a fan of the continued alphabet soup options - I suspect we need to 
revisit this so that you can actually specify a cipher string and/or the list 
of protocols, rather than just adding more options that map to different 
things. That said, this is no worse than the status quo - see comment inline.
 
> Signed-off-by: Kyle J. McKay 
> ---
> 
> For those using the libressl-2.5.4.tar.gz distribution, an equivalent
> patch that updates the tarball files instead can be found here (#0001):
> 
>   https://gist.github.com/11ab5545aaa431b6cecda2188cbda73d
> 
>  src/usr.bin/nc/nc.1 |  2 ++
>  src/usr.bin/nc/netcat.c | 12 +++-
>  2 files changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/src/usr.bin/nc/nc.1 b/src/usr.bin/nc/nc.1
> index b1f96488..dd8bc70e 100644
> --- a/src/usr.bin/nc/nc.1
> +++ b/src/usr.bin/nc/nc.1
> @@ -233,6 +233,8 @@ For TLS options
>  may be one of
>  .Ar tlsall ;
>  which allows the use of all supported TLS protocols and ciphers,
> +.Ar tlscompat ;
> +which allows the use of all supported TLS protocols and "compat" ciphers,
>  .Ar noverify ;
>  which disables certificate verification;
>  .Ar noname ,
> diff --git a/src/usr.bin/nc/netcat.c b/src/usr.bin/nc/netcat.c
> index e222e1e7..cae85594 100644
> --- a/src/usr.bin/nc/netcat.c
> +++ b/src/usr.bin/nc/netcat.c
> @@ -72,6 +72,7 @@
>  #define TLS_NONAME   (1 << 3)
>  #define TLS_CCERT(1 << 4)
>  #define TLS_MUSTSTAPLE   (1 << 5)
> +#define TLS_COMPAT   (1 << 6)
> 
>  /* Command Line Options */
>  int  dflag;  /* detached, no stdin */
> @@ -381,6 +382,8 @@ main(int argc, char *argv[])
>   errx(1, "cannot use -c and -F");
>   if (TLSopt && !usetls)
>   errx(1, "you must specify -c to use TLS options");
> + if ((TLSopt & (TLS_ALL|TLS_COMPAT)) == (TLS_ALL|TLS_COMPAT))
> + errx(1, "cannot use -T tlsall and -T tlscompat");
>   if (Cflag && !usetls)
>   errx(1, "you must specify -c to use -C");
>   if (Kflag && !usetls)
> @@ -478,7 +481,13 @@ main(int argc, char *argv[])
>   errx(1, "%s", tls_config_error(tls_cfg));
>   if (oflag && tls_config_set_ocsp_staple_file(tls_cfg, oflag) == 
> -1)
>   errx(1, "%s", tls_config_error(tls_cfg));
> - if (TLSopt & TLS_ALL) {
> + if (TLSopt & TLS_COMPAT) {
> + if (tls_config_set_protocols(tls_cfg,
> + TLS_PROTOCOLS_ALL) != 0)
> + errx(1, "%s", tls_config_error(tls_cfg));
> + if (tls_config_set_ciphers(tls_cfg, "compat") != 0)
> + errx(1, "%s", tls_config_error(tls_cfg));
> + } else if (TLSopt & TLS_ALL) {

These two are essentially duplicates - you might as well use the same code for 
both and just select the appropriate value to pass to tls_config_set_ciphers() 
based on the flag in question.

>   if (tls_config_set_protocols(tls_cfg,
>   TLS_PROTOCOLS_ALL) != 0)
>   errx(1, "%s", tls_config_error(tls_cfg));
> @@ -1536,6 +1545,7 @@ map_tls(char *s, int *val)
>   { "noname", TLS_NONAME },
>   { "clientcert", TLS_CCERT},
>   { "muststaple", TLS_MUSTSTAPLE},
> + { "tlscompat",  TLS_COMPAT },
>   { NULL, -1 },
>   };
> 
> ---

Re: Reported problem: postfix, libressl 2.6.x, DHE-RSA-AES256-SHA

[Joel Sing]

On Friday 10 November 2017 11:58:04 Stuart Henderson wrote:
>
> From an irc contact using LibreSSL 2.6.3 on FreeBSD:
> 
> 11:14 < matt> Nov 10 11:06:06 tao postfix/smtpd[77685]: Anonymous TLS 
> connection established from email.morrisons.com[192.86.55.223]: TLSv1 with 
> cipher DHE-RSA-AES256-SHA (256/256 bits)
> 11:14 < matt> had to switch postfix to openssl temporarily to get that
> ...
> 11:26 < matt> using libressl 2.6.x I get this from morrisons:
> 11:27 < matt> Nov 10 10:55:57 tao postfix/smtpd[5996]: SSL_accept error from 
> email.morrisons.com[192.86.55.223]: -1
> 11:27 < matt> Nov 10 10:55:57 tao postfix/smtpd[5996]: warning: TLS library 
> problem: error:1403710B:SSL routines:ACCEPT_SR_KEY_EXCH:wrong version 
> number:ssl_pkt.c:376:
> 11:27 < matt> Nov 10 10:55:57 tao postfix/smtpd[5996]: lost connection after 
> STARTTLS from email.morrisons.com[192.86.55.223]
> 11:27 < matt> worked fine on 2.5.x
> ...
> 11:55 < matt> odd then. but yeah. works fine in 2.5.x, breaks in 2.6.x
> 11:56 < matt> it was actually broken on 2.6.0
>
> And Bernard mentioned similar yesterday.
> 
> 18:55 < Barnerd> Trusted TLS connection established from 
> russian-caravan.cloud9.net[2604:8d00:0:1::4]: TLSv1 with cipher 
> DHE-RSA-AES256-SHA (256/256 bits) is all I really know
> 18:58 < Barnerd> Cipher works OK with OpenSMTPd :D
> 
> matt has the mail accepted now and they're not triggerable remotely
> (most of their mails are sent via messagelabs, only certain marketing
> mails are sent this way) so I can't get a pcap or test on-demand. 
> 
> Code generating the error message here:
> 
> 374 /* Lets check version */
> 375 if (!s->internal->first_packet && ssl_version != 
> s->version) {
> 376 SSLerror(s, SSL_R_WRONG_VERSION_NUMBER);
> 377 if ((s->version & 0xFF00) == (ssl_version & 
> 0xFF00) &&
> 378 !s->internal->enc_write_ctx && 
> !s->internal->write_hash)
> 379 /* Send back error using their minor 
> version number :-) */
> 380 s->version = ssl_version;
> 381 al = SSL_AD_PROTOCOL_VERSION;
> 382 goto f_err;
> 383 }
> 
> It hasn't really changed recently, the SSLerror line was touched due to
> refactoring but no real changes there.
> 
> Any ideas?

This effectively suggests that during the TLS handshake (while we're expecting
the Client Key Exchange) the client is sending a record that has a version
number that does not match what we sent in the Server Hello, which is rather
strange. Out of the changes between 2.5.3 and 2.6.0, the only version related
change was the addition of SSL_{,CTX_}_set_{min,max}_proto_version(). However,
that seems unlikely to result in specific client breakage.

I suspect this is going to be difficult to track down without being able to see
what is on the wire (tcpdump or 'smtpd_tls_loglevel = 3' in postfix) or being
able to reproduce/trigger TLS sessions from the client.

Re: Reported problem: postfix, libressl 2.6.x, DHE-RSA-AES256-SHA

[Joel Sing]

On Monday 04 December 2017 15:54:35 Giovanni Bechis wrote:
> On 12/04/17 13:19, Giovanni Bechis wrote:
> > On 11/10/17 17:46, Joel Sing wrote:
> > [...]
> > 
> >> I suspect this is going to be difficult to track down without being able
> >> to see what is on the wire (tcpdump or 'smtpd_tls_loglevel = 3' in
> >> postfix) or being able to reproduce/trigger TLS sessions from the
> >> client.
> > 
> > postfix log file with 'smtpd_tls_loglevel = 3' attached.
> > 
> >  Thanks & Cheers
> >  
> >   Giovanni
> 
> dmesg(8) attached, running postfix-3.2.2 on 6.2, latest postfix version does
> not fix the problem. Giovanni

Thanks - Eric Elena was able to provide a packet capture, resulting in an 
issue being found and fixed in libssl. This has already been committed to -
current and the backport should hopefully be committed to 6.2 soon:

https://marc.info/?l=libressl=151188863421496=2

Re: Reported problem: postfix, libressl 2.6.x, DHE-RSA-AES256-SHA

[Joel Sing]

On Monday 04 December 2017 13:19:41 Giovanni Bechis wrote:
> On 11/10/17 17:46, Joel Sing wrote:
> [...]
> 
> > I suspect this is going to be difficult to track down without being able
> > to see what is on the wire (tcpdump or 'smtpd_tls_loglevel = 3' in
> > postfix) or being able to reproduce/trigger TLS sessions from the client.
> 
> postfix log file with 'smtpd_tls_loglevel = 3' attached.
>  Thanks & Cheers
>   Giovanni

Looking at this more closely, it is actually a different problem from the
originally reported issue (wrong version number):

Dec  4 13:09:30 thor postfix/smtpd[91646]: connect from 
sonic301-3.consmr.mail.bf2.yahoo.com[74.6.129.42]
Dec  4 13:09:31 thor postfix/smtpd[91646]: setting up TLS connection from 
sonic301-3.consmr.mail.bf2.yahoo.com[74.6.129.42]
Dec  4 13:09:31 thor postfix/smtpd[91646]: 
sonic301-3.consmr.mail.bf2.yahoo.com[74.6.129.42]: TLS cipher list 
"aNULL:-aNULL:HIGH:MEDIUM:+RC4:@STRENGTH"
Dec  4 13:09:31 thor postfix/smtpd[91646]: SSL_accept:before/accept 
initialization
...
Dec  4 13:09:31 thor postfix/smtpd[91646]: SSL_accept:SSLv3 read client hello B
Dec  4 13:09:31 thor postfix/smtpd[91646]: SSL_accept:SSLv3 write server hello A
Dec  4 13:09:31 thor postfix/smtpd[91646]: SSL_accept:SSLv3 write certificate A
Dec  4 13:09:31 thor postfix/smtpd[91646]: SSL_accept:SSLv3 write key exchange A
Dec  4 13:09:31 thor postfix/smtpd[91646]: SSL_accept:SSLv3 write server done A
...
Dec  4 13:09:31 thor postfix/smtpd[91646]: SSL_accept:SSLv3 flush data
Dec  4 13:09:31 thor postfix/smtpd[91646]: SSL_accept:SSLv3 read client 
certificate A
Dec  4 13:09:31 thor postfix/smtpd[91646]: read from 4F66840F900 [4F6048AA003] 
(5 bytes => -1 (0x))
Dec  4 13:09:31 thor postfix/smtpd[91646]: read from 4F66840F900 [4F6048AA003] 
(5 bytes => 5 (0x5))
Dec  4 13:09:31 thor postfix/smtpd[91646]:  15 03 03 00 02  
 .
Dec  4 13:09:31 thor postfix/smtpd[91646]: read from 4F66840F900 [4F6048AA008] 
(2 bytes => 2 (0x2))
Dec  4 13:09:31 thor postfix/smtpd[91646]:  02 2e   
 ..
Dec  4 13:09:31 thor postfix/smtpd[91646]: SSL3 alert read:fatal:certificate 
unknown
Dec  4 13:09:31 thor postfix/smtpd[91646]: SSL_accept:failed in SSLv3 read 
client key exchange A
Dec  4 13:09:31 thor postfix/smtpd[91646]: SSL_accept error from 
sonic301-3.consmr.mail.bf2.yahoo.com[74.6.129.42]: 0
Dec  4 13:09:31 thor postfix/smtpd[91646]: warning: TLS library problem: 
error:14037416:SSL routines:ACCEPT_SR_KEY_EXCH:sslv3 alert certificate 
unknown:/usr/src/lib/libssl/ssl_pkt.c:1205:SSL alert number 46:
Dec  4 13:09:31 thor postfix/smtpd[91646]: lost connection after STARTTLS from 
sonic301-3.consmr.mail.bf2.yahoo.com[74.6.129.42]
Dec  4 13:09:31 thor postfix/smtpd[91646]: disconnect from 
sonic301-3.consmr.mail.bf2.yahoo.com[74.6.129.42] ehlo=1 starttls=0/1 
commands=1/2

In this case the client hello has been received and the server
hello/certificate/key exchange/done has been sent, before the other side
responds with a "certificate unknown" alert - this suggests that the TLS
client is actually expecting to do some form of certificate verification
and this is failing. 

Was this working prior to OpenBSD 6.2?

Re: [patch] httpd: add tls client certificate authentication

[Joel Sing]

On Wednesday 16 May 2018 17:32:56 Jack Burton wrote:
> My attempts to get this accepted last year stalled.
> 
> As best I recall, the main stumbling block was agreeing on how much /
> exactly which client cert data to pass through to fastcgi (my view was
> that passing the whole client cert chain would be ideal).
> 
> So, here's a stripped down version that passes *no* client cert
> data through to fastcgi at all (but still passes the relevant status
> flags in TLS_PEER_VERIFY).
> 
> This diff provides everything necessary for tls client *authentication*,
> but *none* of what's required to use tls client certs for authorisation
> or accounting.
> 
> I figured that if we can agree on this much, so httpd can be used for
> the authentication-only case (which is all non-fastcgi sites would want)
> straight away, that's be a good first step -- then we can come back and
> argue the toss over how much client cert data is necessary/sufficient
> to pass through for authorisation/accounting purposes.
> 
> There's also a trivial regression test (unchanged from last year), which
> I'll post again separately next.

Thanks - I've just committed this, with some minor tweaks and minus one chunk:

> Index: server.c
> ===
> RCS file: /cvs/src/usr.sbin/httpd/server.c,v
> retrieving revision 1.113
> diff -u -p -r1.113 server.c
> --- server.c  29 Nov 2017 16:55:08 -  1.113
> +++ server.c  16 May 2018 07:59:11 -
> @@ -264,6 +300,27 @@ server_tls_init(struct server *srv)
>   return (-1);
>   }
> 
> + if (srv->srv_conf.tls_ca != NULL) {
> + if (tls_config_set_ca_mem(srv->srv_tls_config,
> + srv->srv_conf.tls_ca, srv->srv_conf.tls_ca_len) != 0) {
> + log_warnx("%s: failed to add ca cert(s)", __func__);
> + return (-1);
> + }
> + if (srv->srv_conf.tls_flags & TLSFLAG_OPTIONAL)
> + tls_config_verify_client_optional(srv->srv_tls_config);
> + else
> + tls_config_verify_client(srv->srv_tls_config);
> +
> + if (srv->srv_conf.tls_crl != NULL) {
> + if (tls_config_set_crl_mem(srv->srv_tls_config,
> + srv->srv_conf.tls_crl,
> + srv->srv_conf.tls_crl_len) != 0) {
> + log_warnx("%s: failed to add crl(s)", __func__);
> + return (-1);
> + }
> + }
> + }
> +
>   TAILQ_FOREACH(srv_conf, >srv_hosts, entry) {
>   if (srv_conf->tls_cert == NULL || srv_conf->tls_key == NULL)
>   continue;
> @@ -277,6 +334,26 @@ server_tls_init(struct server *srv)
>   log_warnx("%s: failed to add tls keypair", __func__);
>   return (-1);
>   }
> +
> + if (srv->srv_conf.tls_ca == NULL)
> + continue;
> + log_debug("%s: adding ca cert(s) for server %s", __func__,
> + srv->srv_conf.name);
> + if (tls_config_set_ca_mem(srv->srv_tls_config,
> + srv_conf->tls_ca, srv_conf->tls_ca_len) != 0) {
> + log_warnx("%s: failed to add ca cert(s)", __func__);
> + return (-1);
> + }
> +
> + if (srv->srv_conf.tls_crl == NULL)
> + continue;
> +
> + log_debug("%s: adding crl(s) for server %s", __func__,
> + srv->srv_conf.name);
> + if (tls_config_set_crl_mem(srv->srv_tls_config,
> + srv_conf->tls_crl, srv_conf->tls_crl_len) != 0) {
> + return (-1);
> + }
>   }
> 
>   /* set common session ID among all processes */

The above chunk does not make sense, since in the case of multiple httpd 
servers configured on the same port, we'll just keep on setting the CA and CRL 
which overwrites the one set in the previous chunk (which means the last 
configured CA and CRL win). The SNI support in libtls does not currently allow 
for multiple CAs/CRLs to be provided - if we wanted to support this we'd need 
to add support their first. For the time being we should add the CA and CRL 
configuration to the server_tls_cmp() check so that we force it to be identical 
across HTTPS servers configured on the same address/port.

openssl(1): convert genpkey options handling

[Joel Sing]

The following diff converts the openssl(1) genpkey option handling to the
options handling framework.

ok?

Index: genpkey.c
===
RCS file: /cvs/src/usr.bin/openssl/genpkey.c,v
retrieving revision 1.11
diff -u -p -r1.11 genpkey.c
--- genpkey.c   7 Feb 2018 05:47:55 -   1.11
+++ genpkey.c   7 Feb 2018 09:03:50 -
@@ -65,27 +65,165 @@
 #include 
 #include 
 
-static int
-init_keygen_file(BIO * err, EVP_PKEY_CTX ** pctx, const char *file);
+static int init_keygen_file(BIO * err, EVP_PKEY_CTX **pctx, const char *file);
 static int genpkey_cb(EVP_PKEY_CTX * ctx);
 
+struct {
+   const EVP_CIPHER *cipher;
+   EVP_PKEY_CTX **ctx;
+   int do_param;
+   char *outfile;
+   int outformat;
+   char *passarg;
+   int text;
+} genpkey_config;
+
+static int
+genpkey_opt_algorithm(char *arg)
+{
+   if (!init_gen_str(bio_err, genpkey_config.ctx, arg,
+   genpkey_config.do_param))
+   return (1);
+
+   return (0);
+}
+
+static int
+genpkey_opt_cipher(int argc, char **argv, int *argsused)
+{
+   char *name = argv[0];
+
+   if (*name++ != '-')
+   return (1);
+
+   if (genpkey_config.do_param == 1)
+   return (1);
+
+   if (strcmp(name, "none") == 0) {
+   genpkey_config.cipher = NULL;
+   *argsused = 1;
+   return (0);
+   }
+
+   if ((genpkey_config.cipher = EVP_get_cipherbyname(name)) != NULL) {
+   *argsused = 1;
+   return (0);
+   }
+
+   return (1);
+}
+
+static int
+genpkey_opt_paramfile(char *arg)
+{
+   if (genpkey_config.do_param == 1)
+   return (1);
+   if (!init_keygen_file(bio_err, genpkey_config.ctx, arg))
+   return (1);
+
+   return (0);
+}
+
+static int
+genpkey_opt_pkeyopt(char *arg)
+{
+   if (*genpkey_config.ctx == NULL) {
+   BIO_puts(bio_err, "No keytype specified\n");
+   return (1);
+   }
+
+   if (pkey_ctrl_string(*genpkey_config.ctx, arg) <= 0) {
+   BIO_puts(bio_err, "parameter setting error\n");
+   ERR_print_errors(bio_err);
+   return (1);
+   }
+
+   return (0);
+}
+
+struct option genpkey_options[] = {
+   {
+   .name = "algorithm",
+   .argname = "name",
+   .desc = "Public key algorithm to use (must precede -pkeyopt)",
+   .type = OPTION_ARG_FUNC,
+   .opt.argfunc = genpkey_opt_algorithm,
+   },
+   {
+   .name = "genparam",
+   .desc = "Generate a set of parameters instead of a private key",
+   .type = OPTION_FLAG,
+   .opt.flag = _config.do_param,
+   },
+   {
+   .name = "out",
+   .argname = "file",
+   .desc = "Output file to write to (default stdout)",
+   .type = OPTION_ARG,
+   .opt.arg = _config.outfile,
+   },
+   {
+   .name = "outform",
+   .argname = "format",
+   .desc = "Output format (DER or PEM)",
+   .type = OPTION_ARG_FORMAT,
+   .opt.value = _config.outformat,
+   },
+   {
+   .name = "paramfile",
+   .argname = "file",
+   .desc = "File to load public key algorithm parameters from\n"
+   "(must precede -pkeyopt)",
+   .type = OPTION_ARG_FUNC,
+   .opt.argfunc = genpkey_opt_paramfile,
+   },
+   {
+   .name = "pass",
+   .argname = "arg",
+   .desc = "Output file password source",
+   .type = OPTION_ARG,
+   .opt.arg = _config.passarg,
+   },
+   {
+   .name = "pkeyopt",
+   .argname = "opt:value",
+   .desc = "Set public key algorithm option to the given value",
+   .type = OPTION_ARG_FUNC,
+   .opt.argfunc = genpkey_opt_pkeyopt,
+   },
+   {
+   .name = "text",
+   .desc = "Print the private/public key in human readable form",
+   .type = OPTION_FLAG,
+   .opt.flag = _config.text,
+   },
+   {
+   .name = NULL,
+   .type = OPTION_ARGV_FUNC,
+   .opt.argvfunc = genpkey_opt_cipher,
+   },
+   {NULL},
+};
+
+static void
+genpkey_usage()
+{
+   fprintf(stderr,
+   "usage: genpkey [-algorithm alg] [cipher] [-genparam] [-out file]\n"
+   "[-outform der | pem] [-paramfile file] [-pass arg]\n"
+   "[-pkeyopt opt:value] [-text]\n\n");
+   options_usage(genpkey_options);
+}
+
 int
 genpkey_main(int argc, char **argv)
 {
-   char **args, *outfile = NULL;
-   char *passarg = NULL;
BIO *in = NULL, *out = NULL;
-   const EVP_CIPHER *cipher = NULL;
-   int outformat;
-   int text = 0;
-  

Re: CID #183499: don't leak db in RSA_padding_check_PKCS1_OAEP()

[Joel Sing]

On Sunday 19 August 2018 08:44:24 Theo Buehler wrote:
> Coverity complains about the case where EVP_Digest() fails, but there
> are a couple more.

One thing worth mentioning... previously it would return -1 without setting an 
error, whereas now it will always set  RSA_R_OAEP_DECODING_ERROR (even if the 
underlying failure was something unrelated like a malloc failure). I'm not 
overly concerned by this, but we could (if we wanted) keep the previous 
behaviour by adding an 'err' label that jumps to the 'free(db);' line and use 
that for non-decoding failures.

Either way, ok jsing@

> Index: rsa/rsa_oaep.c
> ===
> RCS file: /var/cvs/src/lib/libcrypto/rsa/rsa_oaep.c,v
> retrieving revision 1.27
> diff -u -p -r1.27 rsa_oaep.c
> --- rsa/rsa_oaep.c5 Aug 2018 13:30:04 -   1.27
> +++ rsa/rsa_oaep.c19 Aug 2018 06:38:52 -
> @@ -126,8 +126,7 @@ RSA_padding_check_PKCS1_OAEP(unsigned ch
>   }
> 
>   dblen = num - SHA_DIGEST_LENGTH;
> - db = malloc(dblen + num);
> - if (db == NULL) {
> + if ((db = malloc(dblen + num)) == NULL) {
>   RSAerror(ERR_R_MALLOC_FAILURE);
>   return -1;
>   }
> @@ -143,17 +142,17 @@ RSA_padding_check_PKCS1_OAEP(unsigned ch
>   maskeddb = padded_from + SHA_DIGEST_LENGTH;
> 
>   if (MGF1(seed, SHA_DIGEST_LENGTH, maskeddb, dblen))
> - return -1;
> + goto decoding_err;
>   for (i = 0; i < SHA_DIGEST_LENGTH; i++)
>   seed[i] ^= padded_from[i];
> 
>   if (MGF1(db, dblen, seed, SHA_DIGEST_LENGTH))
> - return -1;
> + goto decoding_err;
>   for (i = 0; i < dblen; i++)
>   db[i] ^= maskeddb[i];
> 
>   if (!EVP_Digest((void *)param, plen, phash, NULL, EVP_sha1(), NULL))
> - return -1;
> + goto decoding_err;
> 
>   if (timingsafe_memcmp(db, phash, SHA_DIGEST_LENGTH) != 0 || bad)
>   goto decoding_err;
> @@ -177,7 +176,7 @@ RSA_padding_check_PKCS1_OAEP(unsigned ch
>   free(db);
>   return mlen;
> 
> -decoding_err:
> + decoding_err:
>   /*
>* To avoid chosen ciphertext attacks, the error message should not
>* reveal which kind of decoding error happened

Re: Recent "elliptic curve" -> "supported groups" change in libssl

[Joel Sing]

On Tuesday 06 November 2018 00:39:11 Luigi30 wrote:
> Hi,
> 
> As someone with interests in kernel development and a lot of spare
> time, I want to work on OS patches. I just installed OpenBSD 6.4 in a
> clean development VM and started building the -current branch from CVS
> to get up to date with the latest commits.
> 
> I noticed that the build was failing with an error in
> usr.bin/openssl/c_sb.c line 703 caused by a missing #define. I traced
> the cause back to this commit earlier today updating libssl's TLS
> support for RFC 7919 compliance:
> https://github.com/openbsd/src/commit/2cdb2b1d3f3f9272c0a1acf5fe1f067f3db09e
> 29#diff-e050d3ba43ebfa12f82b36086dca3ea3
> 
> It renames the Elliptic Curves extensions to Supported Groups,
> including the TLSEXT_TYPE_elliptic_curves #define which became
> TLSEXT_TYPE_supported_groups. Simple, right? I updated the #define and
> extname to match the new supported groups name and continued building.
> Everything was fine and I was able to access HTTPS web pages and
> retrieve packages.

Thanks - fixed.
 
> However, when I went to create the diff afterward, I got an error from
> CVS...
> 
> --
> ssh_dispatch_run_fatal: Connection to 129.128.197.20 port 22: invalid
> elliptic curve value
> --
> 
> Uh-oh. I'm going to assume that this is connected to the elliptic
> curve diff. I tried a couple different anoncvs mirrors with no effect.
> Just wondering if this was a known problem with -current or something
> hokey going on with my system.

You've probably run into another bug that was introduced and reverted. Please 
update and try again.

Re: BIOCINSTALLBOOT/sparc64 installboot: EFBIG on too big boot loaders

[Joel Sing]

On 20-06-05 22:42:17, Klemens Nanni wrote:
> On Mon, Jun 01, 2020 at 11:48:05PM +0200, Klemens Nanni wrote:
> > Installing an unstripped boot loader on softraid on sparc64 fails
> > without proper error message.
> > 
> > Make BIOCINSTALLBOOT return a proper errno, make installboot(8) use it
> > to provide proper usage errors plus unique code paths for debugging.
> > 
> > At first, I made sr_ioctl_installboot() use sr_error() in the relevant
> > spot and this made the kernel print my errors, however the following
> > piece in softraid.c:sr_bio_handler() means using sr_error() will hide
> > the error code from the ioctl(2) call, i.e. installboot(8) would
> > report no error message on stderr and exit zero:
> > 
> > if (sc->sc_status.bs_msg_count > 0)  
> > rv = 0;
> > 
> > So instead, use a proper errno that yields a simple
> > 
> > # ./obj/installboot sd2 /usr/mdec/bootblk /ofwboot.new
> > installboot.sr: softraid installboot failed: File too large
> > 
> > 
> > 
> > Background:  I built ofwboot on one machine ("make" without
> > "make install", then copy obj/ofwboot over), the resulting executable
> > was not stripped during build (happens during "make install") and was
> > about twice as big:
> > 
> > # ls -l /ofwboot*   
> > -rw-r--r--  1 root  wheel  106688 May 23 22:42 /ofwboot
> > -rwxr-xr-x  1 knwheel  272452 May 24 00:20 /ofwboot.new
> > -rwxr-xr-x  1 root  wheel  106752 May 24 01:21 /ofwboot.new.strip
> > 
> > It took me longer than anticipated to find out that installboot(8)
> > fails beause my new boot loader was too big:
> > 
> > # installboot sd2 /usr/mdec/bootblk /ofwboot.new
> > installboot: softraid installboot failed
> > 
> > # installboot -v sd2 /usr/mdec/bootblk /ofwboot.new
> > Using / as root
> > installing bootstrap on /dev/rsd2c
> > using first-stage /usr/mdec/bootblk, second-stage /ofwboot.new
> > boot block is 6882 bytes (14 blocks @ 512 bytes = 7168 bytes)
> > sd2: softraid volume with 1 disk(s)
> > sd2: installing boot loader on softraid volume
> > installboot: softraid installboot failed
> > 
> > That's it, no details or additional messages from the kernel.
> > While this was primarily my own fault, perhaps there are more legitimate
> > reasons foor bootblk/ofwboot builds to exceed their maximum size.
> 
> In a i386 VM with root on crypto softraid, I built a much bigger second
> stage boot loader and performed the same tests as on sparc64: i386 does
> not try to install the bogus boot code due to checks in i386_softraid.c
> up front:
> 
>   # ls -l /usr/mdec/boot /boot.efbig
>   -r-xr-xr-x  1 root  bin 89728 Jun  5 03:02 /usr/mdec/boot
>   -rwxr-xr-x  1 root  wheel  176172 Jun  5 22:16 /boot.efbig
>   # installboot -v sd1 /usr/mdec/biosboot /boot.efbig
>   Using / as root
>   installing bootstrap on /dev/rsd1c
>   using first-stage /usr/mdec/biosboot, second-stage /boot.efbig
>   sd1: softraid volume with 1 disk(s)
>   installboot: boot code will not fit
> 
> So for installboot(8) on sparc64 and i386 as the only two users of
> BIOCINSTALLBOOT, this makes root on softraid on sparc64 the only use
> that actually hits size checks in the ioctl code.

Keep in mind that the i386 installboot code is used on both i386
and amd64.

> sr_ioctl_installboot() seems inconsistent to me in how it reports some
> errors through sr_error() (causing ioctl() to return zero) and returing
> proper error codes (causing ioctl() and therefore installboot to fail);

sr_error() is used to add detail - the installboot code should be
checking and handling the case where bs->bs_status is non-zero.
IIRC the reason the ioctl has to succeed for this to work, is that
on failure there is no copyout().

> Assuming my EFBIG approach is still sensible (for sparc64), diff below
> adjusts the errx() call to err() in installboot for both platforms, even
> though i386 never reaches it.
> 
> Feedback? OK?

While this works, you would be better off making use of the error
reporting mechanism that exists. A compile tested only diff for
the kernel side is below. A diff to installboot would be needed to
graft some code similar to that in bioctl's bio_status() function.

Index: softraid.c
===
RCS file: /cvs/src/sys/dev/softraid.c,v
retrieving revision 1.401
diff -u -p -u -p -r1.401 softraid.c
--- softraid.c  14 Apr 2020 07:38:21 -  1.401
+++ softraid.c  6 Jun 2020 14:32:56 -
@@ -3704,11 +3704,17 @@ sr_ioctl_installboot(struct sr_softc *sc
goto done;
}
 
-   if (bb->bb_bootblk_size > SR_BOOT_BLOCKS_SIZE * DEV_BSIZE)
+   if (bb->bb_bootblk_size > SR_BOOT_BLOCKS_SIZE * DEV_BSIZE) {
+   sr_error(sc, "boot block is too large (%d > %d)",
+   bb->bb_bootblk_size, SR_BOOT_BLOCKS_SIZE * DEV_BSIZE);
goto done;
+   }
 
-   if 

Re: LibreSSL regressions

[Joel Sing]

On 21-02-15 14:49:46, Jan Klemkow wrote:
> On Sat, Feb 13, 2021 at 03:53:48PM +0100, Theo Buehler wrote:
> > On Sat, Feb 13, 2021 at 11:58:04AM +0100, Jan Klemkow wrote:
> > > A coworker of mine has made tests with LibreSSL [1] and found some
> > > regressions.  I took his test descriptions and created the following
> > > automated regression test.  In the repository he described his findings
> > > in detail.  I kept the numbers of the files and subtests in the target
> > > names for now.  So, its easier to match it with his files.
> > > 
> > > I don't know how to handle the result of "test-01-ssl".  Thats why its
> > > just a comment.  Someone may have an idea to handle this properly.
> > > 
> > > Any comments, wishes or OK's?
> > > 
> > > [1]: https://github.com/noxxi/libressl-tests
> > 
> > First of all thanks for the effort!
> > 
> > The perl script and probably also the Makefile should have a license.
> > 
> > Please add a check that tests whether the required perl modules are
> > installed (p5-IO-Socket-SSL and p5-Net-SSLeay) and otherwise prints
> > SKIPPED and their names, so I can install them if they're not present.
> > I never remember their exact capitalization and hyphenation...
> > 
> > Various comments inline, and a patch for openssl(1) at the end that may
> > simplify some things.
> 
> This is an updated version of the test including comments and wishes
> from tb@ and bluhm@.
> 
> OK?

This currently drives openssl(1) for tests, which means that it is
testing openssl(1), libssl and libcrypto, when what you're really
wanting to test is libcrypto's verifier. While this works, the
problem is that a change or breakage in libssl or openssl(1) results
in a regress failure for libcrypto. If this is to land in its
current form it really should be in regress/usr.bin/openssl -
alternatively, it could be reworked to explicitly test libcrypto's
APIs and remain here.

Some additional comments inline.

> Index: regress/lib/libcrypto/validate/Makefile
> ===
> RCS file: regress/lib/libcrypto/validate/Makefile
> diff -N regress/lib/libcrypto/validate/Makefile
> --- /dev/null 1 Jan 1970 00:00:00 -
> +++ regress/lib/libcrypto/validate/Makefile   15 Feb 2021 13:38:22 -
> @@ -0,0 +1,133 @@
> +# $OpenBSD$
> +
> +# Copyright (c) 2021 Jan Klemkow 
> +#
> +# Permission to use, copy, modify, and distribute this software for any
> +# purpose with or without fee is hereby granted, provided that the above
> +# copyright notice and this permission notice appear in all copies.
> +#
> +# THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
> +# WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
> +# MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
> +# ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
> +# WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
> +# ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
> +# OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
> +
> +# This regression test is based on manual test descriptions from:
> +# https://github.com/noxxi/libressl-tests
> +
> +# The following port must be installed for the regression tests:
> +# p5-IO-Socket-SSL   perl interface to SSL sockets
> +
> +PERL =   perl
> +OPENSSL ?=   openssl
> +
> +PERL_REQUIRE !=  perl -Mstrict -Mwarnings -e ' \
> +eval { require IO::Socket::SSL } or print $@; \
> +'
> +.if ! empty (PERL_REQUIRE)
> +regress:
> + @echo "${PERL_REQUIRE}"
> + @echo install these perl packages for additional tests
> + @echo SKIPPED
> +.endif
> +
> +REGRESS_TARGETS +=   test-unusual-wildcard-cert-no-CA-client
> +REGRESS_TARGETS +=   test-unusual-wildcard-cert-CA-client

I'd would s/unusual-wildcard/inlabel-wildcard/g

> +REGRESS_TARGETS +=   test-common-wildcard-cert-no-CA-client
> +REGRESS_TARGETS +=   test-common wildcard-cert-CA-client

There's a space between "test-common" and "wildcard-cert-CA-client"
(presumably meant to be a hyphen) - same in two places below.

> +REGRESS_TARGETS +=   test-verify-unusual-wildcard-cert
> +REGRESS_TARGETS +=   test-openssl-verify-common-wildcard-cert
> +REGRESS_TARGETS +=   test-chain-certificates-s_server
> +REGRESS_TARGETS +=   test-alternative-chain
> +REGRESS_CLEANUP =cleanup-ssl
> +REGRESS_SETUP_ONCE = create-libressl-test-certs
> +
> +REGRESS_EXPECTED_FAILURES += test-unusual-wildcard-cert-no-CA-client
> +REGRESS_EXPECTED_FAILURES += test-common-wildcard-cert-no-CA-client
> +REGRESS_EXPECTED_FAILURES += test-common wildcard-cert-CA-client
> +REGRESS_EXPECTED_FAILURES += test-verify-unusual-wildcard-cert
> +REGRESS_EXPECTED_FAILURES += test-alternative-chain

I suspect that some or all of these are expected behaviour, rather
than failures. We can review and address this once it lands though.

> +create-libressl-test-certs: create-libressl-test-certs.pl
> + ${PERL} 

Re: bioctl: do not confirm new passphrases on stdin

[Joel Sing]

On 23-08-17 02:21:18, Klemens Nanni wrote:
> On Fri, Aug 11, 2023 at 03:44:46PM +, Klemens Nanni wrote:
> > On Wed, Aug 02, 2023 at 10:37:36AM +, Klemens Nanni wrote:
> > > Creating new volumes prompts
> > >   Passphrase:
> > >   Re-type passphrase:
> > > which is sane for interative usage, but -s (which omits prompts) to read
> > > from stdin also prompts twice.
> > > 
> > > I think that's neither intuitive nor ergonomical and as intended for
> > > non-interactive scripts, -s should take a new passphase just once.
> > > 
> > > Until a month ago, the manual errorneously said -s could not be used 
> > > during
> > > initial creation anyway, so I worry little about existing scripts like
> > > 
> > >   printf '%s\n%s\n' "$pw" "$pw" | bioctl -s -cC -lsd0a softraid0
> > > 
> > > Diff below makes -s create new volumes with a single passphase:
> > > 
> > >   # echo secret |   bioctl -s -Cforce -cC -lvnd0a softraid0
> > >   bioctl: Passphrases did not match
> > >   # echo secret | ./obj/bioctl -s -Cforce -cC -lvnd0a softraid0
> > >   softraid0: CRYPTO volume attached as sd3
> > > 
> > > Feedback? Objection? OK?
> > 
> > Ping.
> 
> I'll commit this in a few days unless I hear objections.
> 
> The current -s behaviour is stupid;  nothing else I know *silently* prompts
> for passphrase *and* confirmation without anything in between.
> 
> This stuff must be either interactive or quiet and one-shot, not in between.

I agree with the intent, however the man page should probably reflect this
change (i.e. -s makes it non-interactive and you will not get confirmation).

> Index: bioctl.c
> ===
> RCS file: /cvs/src/sbin/bioctl/bioctl.c,v
> retrieving revision 1.151
> diff -u -p -r1.151 bioctl.c
> --- bioctl.c  18 Oct 2022 07:04:20 -  1.151
> +++ bioctl.c  17 Aug 2023 02:16:37 -
> @@ -989,7 +989,7 @@ bio_kdf_generate(struct sr_crypto_kdfinf
>   derive_key(kdfinfo->pbkdf.generic.type, kdfinfo->pbkdf.rounds,
>   kdfinfo->maskkey, sizeof(kdfinfo->maskkey),
>   kdfinfo->pbkdf.salt, sizeof(kdfinfo->pbkdf.salt),
> - "New passphrase: ", 1);
> + "New passphrase: ", rpp_flag == RPP_REQUIRE_TTY ? 1 : 0);

I think it would be less ugly to have an iteractive global (or similar)
and clear that when -s is given (the correct way to write the above would
require masking rpp_flag). 

>  }
>  
>  int

Re: libcrypto: fix leak in x509_name_ex_d2i()

[Joel Sing]

On 22-11-08 18:48:44, Tobias Heider wrote:
> nm.a is initialized to NULL until it gets alloced by x509_name_ex_new().
> The following 'goto err' should free nm.a before returning.
> 
> ok?

Unless I'm missing something, I do not believe this is correct -
nm is a union and nm.a is the same pointer as nm.x - nm.x is already
freed via X509_NAME_free(), which would make this a double free.

> Index: asn1/x_name.c
> ===
> RCS file: /cvs/src/lib/libcrypto/asn1/x_name.c,v
> retrieving revision 1.37
> diff -u -p -r1.37 x_name.c
> --- asn1/x_name.c 25 Dec 2021 13:17:48 -  1.37
> +++ asn1/x_name.c 8 Nov 2022 17:45:08 -
> @@ -340,6 +340,7 @@ x509_name_ex_d2i(ASN1_VALUE **val, const
>   err:
>   if (nm.x != NULL)
>   X509_NAME_free(nm.x);
> + free(nm.a);
>   ASN1error(ERR_R_NESTED_ASN1_ERROR);
>   return 0;
>  }