Re: bioctl: do not confirm new passphrases on stdin
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()
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; > }
Re: LibreSSL regressions
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} ${.CUR
Re: BIOCINSTALLBOOT/sparc64 installboot: EFBIG on too big boot loaders
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 (bb->bb_bo
Re: Recent "elliptic curve" -> "supported groups" change in libssl
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: CID #183499: don't leak db in RSA_padding_check_PKCS1_OAEP()
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: [patch] httpd: add tls client certificate authentication
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->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
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 = &genpkey_config.do_param, + }, + { + .name = "out", + .argname = "file", + .desc = "Output file to write to (default stdout)", + .type = OPTION_ARG, + .opt.arg = &genpkey_config.outfile, + }, + { + .name = "outform", + .argname = "format", + .desc = "Output format (DER or PEM)", + .type = OPTION_ARG_FORMAT, + .opt.value = &genpkey_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 = &genpkey_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 = &genpkey_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; -
Re: Reported problem: postfix, libressl 2.6.x, DHE-RSA-AES256-SHA
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: Reported problem: postfix, libressl 2.6.x, DHE-RSA-AES256-SHA
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&m=151188863421496&w=2
Re: Reported problem: postfix, libressl 2.6.x, DHE-RSA-AES256-SHA
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: [diff] httpd: tls client cert & CRL checks
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] objects: add EV subject OID names
On Wednesday 17 May 2017 12:02:48 Kyle J. McKay wrote: > The "EV SSL Certificate Guidelines" available from: > > https://cabforum.org/extended-validation/ > > defines three OIDs commonly seen in leaf certificates: > > jurisdictionLocalityName > 1.3.6.1.4.1.311.60.2.1.1 > > jurisdictionStateOrProvinceName > 1.3.6.1.4.1.311.60.2.1.2 > > jurisdictionCountryName > 1.3.6.1.4.1.311.60.2.1.3 > > Add these OID names so that certificate subjects containing > them display nicely. > > Note that prior to version 1.4.6 of the EV Guidelines (which > was adopted and effective on 2014-03-24) the OID names started > with "jurisdictionOfIncorporation" instead of just "jurisdiction". > > The newer, shorter, names are used here. > > Signed-off-by: Kyle J. McKay Committed, thanks. > --- > > For those using the libressl-2.5.4.tar.gz distribution, an equivalent > patch that updates the generated files instead can be found here: > > https://gist.github.com/60b6236458c8e318412b99069dca8ed0 > > src/lib/libcrypto/objects/obj_mac.num | 3 +++ > src/lib/libcrypto/objects/objects.txt | 6 ++ > 2 files changed, 9 insertions(+) > > diff --git a/src/lib/libcrypto/objects/obj_mac.num > b/src/lib/libcrypto/objects/obj_mac.num index d839b396..3214090a 100644 > --- a/src/lib/libcrypto/objects/obj_mac.num > +++ b/src/lib/libcrypto/objects/obj_mac.num > @@ -953,3 +953,6 @@ Ed25519 952 > Ed448953 > Ed25519ph954 > Ed448ph 955 > +jurisdictionLocalityName 956 > +jurisdictionStateOrProvinceName 957 > +jurisdictionCountryName 958 > diff --git a/src/lib/libcrypto/objects/objects.txt > b/src/lib/libcrypto/objects/objects.txt index 28d77218..6efabf7d 100644 > --- a/src/lib/libcrypto/objects/objects.txt > +++ b/src/lib/libcrypto/objects/objects.txt > @@ -830,6 +830,12 @@ Private 1: enterprises : > Enterprises > # RFC 2247 > Enterprises 1466 344 : dcobject : dcObject > > +# Extended Validation > +!Alias extendedValidation Enterprises 311 60 > +extendedValidation 2 1 1 : : jurisdictionLocalityName > +extendedValidation 2 1 2 : : > jurisdictionStateOrProvinceName > +extendedValidation 2 1 3 : : jurisdictionCountryName > + > # RFC 1495 > Mail 1 : mime-mhs : MIME MHS > mime-mhs 1 : mime-mhs-headings : mime-mhs-headings > ---
Re: [PATCH 1/2] nc: support -T tlscompat option
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: OpenBSD 6.0 relayd TLS Loadbalancer failing SSL Lab tests (Cipher Support)
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: libressl-2.5.1 patches
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
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
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
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: specify curves via ecdhe statement in httpd.conf
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
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: Fix memory leak in LibreSSL/tls_conninfo_free()
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;
OpenSSL 1.1 API migration path (or the lack thereof...)
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: attach SR drive by force even if not all chunks provide native metadata
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;
Re: httpd: be stricter with TLS configuration
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.
httpd: Add SNI support
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->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);
libtls: Add server side support for SNI
For those who are interested, the following diff adds server side support for SNI to libtls. There are three additional functions: tls_config_add_keypair_file() tls_config_add_keypair_mem() tls_conninfo_servername() The first two allow you to add additional certificates/private keys that will be used if the client sends a TLS servername extension that matches one of the SANs. The third function returns the TLS servername extension that the client specified. Index: tls.c === RCS file: /cvs/src/lib/libtls/tls.c,v retrieving revision 1.45 diff -u -p -r1.45 tls.c --- tls.c 13 Aug 2016 13:05:51 - 1.45 +++ tls.c 13 Aug 2016 17:58:02 - @@ -177,6 +177,24 @@ tls_set_errorx(struct tls *ctx, const ch return (rv); } +struct tls_sni_ctx * +tls_sni_ctx_new(void) +{ + return (calloc(1, sizeof(struct tls_sni_ctx))); +} + +void +tls_sni_ctx_free(struct tls_sni_ctx *sni_ctx) +{ + if (sni_ctx == NULL) + return; + + SSL_CTX_free(sni_ctx->ssl_ctx); + X509_free(sni_ctx->ssl_cert); + + free(sni_ctx); +} + struct tls * tls_new(void) { @@ -207,7 +225,7 @@ tls_configure(struct tls *ctx, struct tl } int -tls_configure_keypair(struct tls *ctx, SSL_CTX *ssl_ctx, +tls_configure_ssl_keypair(struct tls *ctx, SSL_CTX *ssl_ctx, struct tls_keypair *keypair, int required) { EVP_PKEY *pkey = NULL; @@ -274,27 +292,27 @@ tls_configure_keypair(struct tls *ctx, S } int -tls_configure_ssl(struct tls *ctx) +tls_configure_ssl(struct tls *ctx, SSL_CTX *ssl_ctx) { - SSL_CTX_set_mode(ctx->ssl_ctx, SSL_MODE_ENABLE_PARTIAL_WRITE); - SSL_CTX_set_mode(ctx->ssl_ctx, SSL_MODE_ACCEPT_MOVING_WRITE_BUFFER); + SSL_CTX_set_mode(ssl_ctx, SSL_MODE_ENABLE_PARTIAL_WRITE); + SSL_CTX_set_mode(ssl_ctx, SSL_MODE_ACCEPT_MOVING_WRITE_BUFFER); - SSL_CTX_set_options(ctx->ssl_ctx, SSL_OP_NO_SSLv2); - SSL_CTX_set_options(ctx->ssl_ctx, SSL_OP_NO_SSLv3); + SSL_CTX_set_options(ssl_ctx, SSL_OP_NO_SSLv2); + SSL_CTX_set_options(ssl_ctx, SSL_OP_NO_SSLv3); - SSL_CTX_clear_options(ctx->ssl_ctx, SSL_OP_NO_TLSv1); - SSL_CTX_clear_options(ctx->ssl_ctx, SSL_OP_NO_TLSv1_1); - SSL_CTX_clear_options(ctx->ssl_ctx, SSL_OP_NO_TLSv1_2); + SSL_CTX_clear_options(ssl_ctx, SSL_OP_NO_TLSv1); + SSL_CTX_clear_options(ssl_ctx, SSL_OP_NO_TLSv1_1); + SSL_CTX_clear_options(ssl_ctx, SSL_OP_NO_TLSv1_2); if ((ctx->config->protocols & TLS_PROTOCOL_TLSv1_0) == 0) - SSL_CTX_set_options(ctx->ssl_ctx, SSL_OP_NO_TLSv1); + SSL_CTX_set_options(ssl_ctx, SSL_OP_NO_TLSv1); if ((ctx->config->protocols & TLS_PROTOCOL_TLSv1_1) == 0) - SSL_CTX_set_options(ctx->ssl_ctx, SSL_OP_NO_TLSv1_1); + SSL_CTX_set_options(ssl_ctx, SSL_OP_NO_TLSv1_1); if ((ctx->config->protocols & TLS_PROTOCOL_TLSv1_2) == 0) - SSL_CTX_set_options(ctx->ssl_ctx, SSL_OP_NO_TLSv1_2); + SSL_CTX_set_options(ssl_ctx, SSL_OP_NO_TLSv1_2); if (ctx->config->alpn != NULL) { - if (SSL_CTX_set_alpn_protos(ctx->ssl_ctx, ctx->config->alpn, + if (SSL_CTX_set_alpn_protos(ssl_ctx, ctx->config->alpn, ctx->config->alpn_len) != 0) { tls_set_errorx(ctx, "failed to set alpn"); goto err; @@ -302,7 +320,7 @@ tls_configure_ssl(struct tls *ctx) } if (ctx->config->ciphers != NULL) { - if (SSL_CTX_set_cipher_list(ctx->ssl_ctx, + if (SSL_CTX_set_cipher_list(ssl_ctx, ctx->config->ciphers) != 1) { tls_set_errorx(ctx, "failed to set ciphers"); goto err; @@ -310,7 +328,7 @@ tls_configure_ssl(struct tls *ctx) } if (ctx->config->verify_time == 0) { - X509_VERIFY_PARAM_set_flags(ctx->ssl_ctx->param, + X509_VERIFY_PARAM_set_flags(ssl_ctx->param, X509_V_FLAG_NO_CHECK_TIME); } @@ -321,13 +339,13 @@ tls_configure_ssl(struct tls *ctx) } int -tls_configure_ssl_verify(struct tls *ctx, int verify) +tls_configure_ssl_verify(struct tls *ctx, SSL_CTX *ssl_ctx, int verify) { size_t ca_len = ctx->config->ca_len; char *ca_mem = ctx->config->ca_mem; char *ca_free = NULL; - SSL_CTX_set_verify(ctx->ssl_ctx, verify, NULL); + SSL_CTX_set_verify(ssl_ctx, verify, NULL); /* If no CA has been specified, attempt to load the default. */ if (ctx->config->ca_mem == NULL && ctx->config->ca_path == NULL) { @@ -342,19 +360,17 @@ tls_configure_ssl_verify(struct tls *ctx tls_set_errorx(ctx, "ca too long"); goto err; } - if (SSL_CTX_load_verify_mem(ctx->ssl_ctx, ca_mem, - ca_len) != 1) { + if (SSL_CTX_
httpd: be stricter with TLS configuration
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->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->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 *)&s->srv_conf.ss, - (struct sockaddr *)&srv->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 " +
libtls: ALPN support
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(&q, ",")) != 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(&buf[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, &config->alpn, + &config->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,
Re: ntpd && pledge
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: squid 3.5/trunk and SSLv3 removal
On Friday 28 August 2015 13:19:17 Stuart Henderson wrote: > Any suggestions as to what might be needed here, can the SSLv3_method > just be replaced with SSLv23_method? Yes, that should be fine. The code is parsing an SSLv3/TLS client hello message and it is then mapping the ciphersuites to libssl ciphers. I'm guessing the reason for using SSLv3_method() here is to avoid picking up SSLv2 ciphers. Fun fact - due to what is effectively a design flaw, the ciphers returned from SSLv3_method() include TLS1.0 and TLS1.2 ciphers... > http://bazaar.launchpad.net/~squid/squid/3.5/view/head:/src/ssl/bio.cc#L1009 > | ciphers += 2; > | if (ciphersLen) { > | > | const SSL_METHOD *method = SSLv3_method(); > > > > | const int cs = method->put_cipher_by_char(NULL, NULL); > | assert(cs > 0); > | for (size_t i = 0; i < ciphersLen; i += cs) { > | > | const SSL_CIPHER *c = method->get_cipher_by_char((ciphers + > | i)); > | if (c != NULL) { > | > | if (!clientRequestedCiphers.empty()) > | > | clientRequestedCiphers.append(":"); > | > | clientRequestedCiphers.append(c->name); > | > | } else > | > | unknownCiphers = true; > | > | } > | > | } > | debugs(83, 7, "Ciphers requested by client: " << > | clientRequestedCiphers);
Re: httpd: patch to close TLS sockets that,fail before TLS handshake
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.
Re: httpd: patch to close TLS sockets that fail before TLS handshake
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
On Friday 10 July 2015 22:01:43 Karel Gardas wrote: > On Fri, Jul 10, 2015 at 9:34 PM, Chris Cappuccio 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: softraid checksumming discipline.
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
On Tuesday 09 June 2015, Alexey Ivanov wrote: > > On Jun 6, 2015, at 5:31 AM, Joel Sing 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: LibreSSL 2.2 fails to connect to webdav.yandex.com
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: [PATCH] libcrypto: initialize pointer
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: [patch] Turn on Server Cipher Preference
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
Re: crypto softraid and keydisk on same harddrive
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-misc&m=141435482820277&w=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: Do you need/prefer the non-DUID option in the installer?
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-misc&m=138198909623938&w=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
softraid(4) RAID 5 - call for testing
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: libtls manpage diff
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: tls_accept_socket() error message
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: the libressl wikipedia article is awful.
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 : > > > > 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: Too much SUID/SGID files!
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: LibreSSL 2.1.2 linking issues
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: libtls: Secure default cipher list and dtls support
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-portable 2.1.1 s_client supports connecting to SSLv3 servers
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- > > -END SSL SESSION PARAMETERS- > CIPHER 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 > > --- > 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- > > -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: LibreSSL: GOST ciphers implementation
On Thu, 6 Nov 2014, Артур Истомин wrote: > On Tue, Nov 04, 2014 at 08:42:03PM +, Miod Vallat wrote: > > > Two weeks has passed. Is there anything that I can do to > > > push GOST ciphers towards LibreSSL? > > > > Sorry about that. Joel and/or I need to review the diff again and push > > it. I'll try to find time for this next week-end (famous last words). > > > > Miod > > This is suspicious person for me (group of people?). There are lots of > commits since about 2011 in many low-level and/or critical components > from this person: linux kernel, android, gnupg, tcpdump, alsa, tor, > openssl etc, etc.. > > I'm almost certainly wrong, but not too much there competencies for one > person? And if you took the time to actually look at the code you would find that most of it is not actually Dmitry's - it is based on code that was removed from LibreSSL and it has been reworked into a form that is suitable to put back in again... (and I, like many OpenBSD developers, have written/provided/committed diffs to as many - probably even more - projects/areas as you list above; I guess you should not trust any of us either...) -- "Stop assuming that systems are secure unless demonstrated insecure; start assuming that systems are insecure unless designed securely." - Bruce Schneier
Re: libtls future
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: ressl: two way fds extention
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: /c
Re: ressl: two way fds extention
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: libcrypto: use libc string fns
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 > +#include > > #include > #include > @@ -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: Some thoughts about the ressl interface.
On Fri, 24 Oct 2014, Bernhard R. Link wrote: > Hi, > > I hope this is the right list. If it is the wrong list I'd welcome a > hint to the correct list. tech@ is fine. > Some suggestions for the ressl API: > > * Please consider making ressl_config_set_protocols accepting a string > as option instead. Already planned - ressl_config_set_protocols() will continue to require a set of numeric flags, since it provides a way to be explicit. There will be a utility function that takes a protocol string and returns the appropriate flag values (or sets the flag values). This will likely allow both additive/negative names (e.g. "+tls1.2" or "-sslv3") as well as groups (e.g. "compat", "secure", etc). > * Some way to override the name to be looked for in the certificate > would be nice. > > Both to support speaking with broken/incomplete systems (wrong > certificates, certificates not listing alternate interfaces and missing > SNI, ..) and for monitoring (connecting to a specific host behind a > loadblancer or behind DNS-round-robin and checking the certicate as > it will be checked by real clients) a way to specify a different name > to expect in the certificate would be nice. If you call ressl_connect_socket() you already have this functionality via the hostname argument. However, it might be worth adding as a configuration option for the ressl_connect() case. > (If using ressl directly one could always use ressl_connect_socket, > but I guess most programs using it will only offer calling > ressl_config_insecure_noverifyhost (or even only > ressl_config_insecure_noverifycert)). That will not help since ressl_connect_socket() still performs the verification checks, unless they are explicitly disabled. > * An option to only look at subjectAltName and not CN would be nice. > > If only to allow more paranoid monitoring checking if ssl clients > not looking at CN can connect (or not looking at CN if there is > subjectAltName). This one seems a little strange. If you can specify a hostname for validation, it is either in the CN, the SAN or both. I'm not sure that not matching the CN makes sense in that case. Is there a use case that I'm missing? -- "Action without study is fatal. Study without action is futile." -- Mary Ritter Beard
Re: openssl.cnf req defaults -> default_md sha256
On Wed, 1 Oct 2014, Stuart Henderson wrote: > On 2014/10/01 19:05, Joel Sing wrote: > > 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... > > I think I prefer it this way (changing usr.bin/openssl rather than > the library) as there's less risk of impact in unpredictable areas. Agreed, although I think it is probably worth considering both in the longer term. > How about this one? Looks good to me - ok jsing@ > Index: usr.bin/openssl/req.c > === > RCS file: /cvs/src/usr.bin/openssl/req.c,v > retrieving revision 1.2 > diff -u -p -r1.2 req.c > --- usr.bin/openssl/req.c 28 Aug 2014 14:23:52 - 1.2 > +++ usr.bin/openssl/req.c 1 Oct 2014 09:51:37 - > @@ -97,7 +97,7 @@ > #define STRING_MASK "string_mask" > #define UTF8_IN "utf8" > > -#define DEFAULT_KEY_LENGTH 512 > +#define DEFAULT_KEY_LENGTH 2048 > #define MIN_KEY_LENGTH 384 > > > @@ -184,9 +184,8 @@ req_main(int argc, char **argv) > unsigned long chtype = MBSTRING_ASC; > > req_conf = NULL; > -#ifndef OPENSSL_NO_DES > - cipher = EVP_des_ede3_cbc(); > -#endif > + cipher = EVP_aes_256_cbc(); > + digest = EVP_sha256(); > > infile = NULL; > outfile = NULL; > > Index: lib/libcrypto/openssl.cnf > === > RCS file: /cvs/src/lib/libcrypto/openssl.cnf,v > retrieving revision 1.1 > diff -u -p -r1.1 openssl.cnf > --- lib/libcrypto/openssl.cnf 11 Apr 2014 22:51:53 - 1.1 > +++ lib/libcrypto/openssl.cnf 1 Oct 2014 09:51:36 - > @@ -1,41 +1,20 @@ > -# > -# OpenSSL example configuration file. > -# This is mostly being used for generation of certificate requests. > -# > - > -RANDFILE = /dev/arandom > - > - > [ req ] > -default_bits = 1024 > -default_keyfile = privkey.pem > +#default_bits= 2048 > +#default_md = sha256 > +#default_keyfile = privkey.pem > distinguished_name = req_distinguished_name > attributes = req_attributes > > [ req_distinguished_name ] > countryName = Country Name (2 letter code) > -#countryName_default = AU > countryName_min = 2 > countryName_max = 2 > - > stateOrProvinceName = State or Province Name (full name) > -#stateOrProvinceName_default = Some-State > - > localityName = Locality Name (eg, city) > - > 0.organizationName = Organization Name (eg, company) > -#0.organizationName_default = Internet Widgits Pty Ltd > - > -# we can do this but it is not needed normally :-) > -#1.organizationName = Second Organization Name (eg, company) > -#1.organizationName_default = CryptSoft Pty Ltd > - > organizationalUnitName = Organizational Unit Name (eg, section) > -#organizationalUnitName_default = > - > commonName = Common Name (eg, fully qualified host name) > commonName_max = 64 > - > emailAddress = Email Address > emailAddress_max = 64 > > @@ -43,23 +22,3 @@ emailAddress_max = 64 > challengePassword= A challenge password > challengePassword_min= 4 > challengePassword_max= 20 > - > -unstructuredName = An optional company name > - > -[ x509v3_extensions ] > - > -nsCaRevocationUrl= http://www.cryptsoft.com/ca-crl.pem > -nsComment= "This is a comment" > - > -# under ASN.1, the 0 bit would be encoded as 80 > -nsCertType = 0x40 > - > -#nsBaseUrl > -#nsRevocationUrl > -#nsRenewalUrl > -#nsCaPolicyUrl > -#nsSslServerName > -#nsCertSequence > -#nsCertExt > -#nsDataType > - > Index: usr.bin/openssl/openssl.1 > === > RCS file: /cvs/src/usr.bin/openssl/openssl.1,v > retrieving revision 1.3 > diff -u -p -r1.3 openssl.1 > --- usr.bin/openssl/openssl.1 16 Sep 2014 16:05:44 - 1.3 > +++ usr.bin/openssl/openssl.1 1 Oct 2014 09:51:37 - > @@ -5583,7 +5583,7 @@ This gives the > to write the newly created private key to. > If this option is not specified, the filename present in the > configuration file is used. > -.It Fl md4 | md5 | sha1 > +.It
Re: openssl.cnf req defaults -> default_md 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 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_MASK"string_mask" #define UTF8_IN"utf8" -#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: openssl.cnf req defaults -> default_md sha256
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
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: improve ressl config setting
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 > #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 > #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, va
Re: Is there a repo for the latest LibreSSL portable?
On Mon, 11 Aug 2014, Nicholas Wilson wrote: > Hi Ingo, > > On 10 August 2014 15:54, Ingo Schwarze 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: LibreSSL: base64 decoding error
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: LibreSSL: base64 decoding error
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=fce382e3307 >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). -- "Action without study is fatal. Study without action is futile." -- Mary Ritter Beard
Re: "openssl smime -sign" broken
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: "openssl smime -sign" broken
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" > | > | > | > | --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: [PATCH 9] installboot: malloc/memset => calloc
Commited. Thanks. On Sun, 1 Jun 2014, Benjamin Baier wrote: > On Sun, 1 Jun 2014 00:57:43 +1000 > > Joel Sing 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: clean/portable crypto code...
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 6/7] remove parsing of -rand options in openssl apps
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 fi
Re: [PATCH 1/7] If EVP_DecryptInit_ex() returns NULL, j is incremented by a random amount in PEM_do_header()
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 9] installboot: malloc/memset => calloc
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 2/2] include openssl/evp.h for OPENSSL_add_all_algorithms_noconf()
On Mon, 12 May 2014, bust...@gmail.com wrote: > From: Brent Cook > > --- > 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 > #include > > +#include > #include > #include Committed. Thanks. -- "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
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 > > --- > 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 > #include > +#include > > #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: malloc in libssl/src/apps
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 #include #include #include #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: malloc in libssl/src/apps
On Mon, 5 May 2014, Jean-Philippe Ouellet wrote: > On Sun, May 04, 2014 at 12:17:16PM -0600, Theo de Raadt wrote: > > We are going to completely ignore diffs which change multiple idioms > > at once. > > Okay. > > > That is how mistakes get made. > > Yep, more true than I realized. FWIW I already have some of these in my tree - I'll try to pick up the ones that I do not. Comment inline. > Here's a simpler one: > > Index: apps.c > === > RCS file: /cvs/src/lib/libssl/src/apps/apps.c,v > retrieving revision 1.45 > diff -u -p -r1.45 apps.c > --- apps.c3 May 2014 16:03:54 - 1.45 > +++ apps.c4 May 2014 19:35:59 - > @@ -209,13 +209,10 @@ chopup_args(ARGS * arg, char *buf, int * > *argc = 0; > *argv = NULL; > > - 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; > > num = 0; > p = buf; This one is a change in behaviour - if arg->count is > 0 then previously we zeroed arg->data; now we do not. > @@ -232,8 +229,7 @@ chopup_args(ARGS * arg, char *buf, int * > if (num >= arg->count) { > char **tmp_p; > int tlen = arg->count + 20; > - tmp_p = (char **) realloc(arg->data, > - sizeof(char *) * tlen); > + tmp_p = reallocarray(arg->data, tlen, sizeof(char *)); > if (tmp_p == NULL) > return 0; > arg->data = tmp_p; > @@ -1836,9 +1832,9 @@ parse_name(char *subject, long chtype, i >* only become shorter */ > char *buf = malloc(buflen); > size_t max_ne = buflen / 2 + 1; /* maximum number of name elements */ > - char **ne_types = malloc(max_ne * sizeof(char *)); > - char **ne_values = malloc(max_ne * sizeof(char *)); > - int *mval = malloc(max_ne * sizeof(int)); > + char **ne_types = reallocarray(NULL, max_ne, sizeof(char *)); > + char **ne_values = reallocarray(NULL, max_ne, sizeof(char *)); > + int *mval = reallocarray(NULL, max_ne, sizeof(int)); > > char *sp = subject, *bp = buf; > int i, ne_num = 0; > Index: ca.c > === > RCS file: /cvs/src/lib/libssl/src/apps/ca.c,v > retrieving revision 1.48 > diff -u -p -r1.48 ca.c > --- ca.c 2 May 2014 17:06:46 - 1.48 > +++ ca.c 4 May 2014 19:36:00 - > @@ -2002,8 +2002,8 @@ again2: > row[DB_type][0] = 'V'; > row[DB_type][1] = '\0'; > > - if ((irow = (char **)malloc(sizeof(char *) * (DB_NUMBER + 1))) == > - NULL) { > + irow = reallocarray(NULL, DB_NUMBER + 1, sizeof(char *)); > + if (irow == NULL) { > BIO_printf(bio_err, "Memory allocation failure\n"); > goto err; > } > @@ -2267,8 +2267,8 @@ do_revoke(X509 * x509, CA_DB * db, int t > row[DB_type][0] = 'V'; > row[DB_type][1] = '\0'; > > - if ((irow = (char **)malloc(sizeof(char *) * > - (DB_NUMBER + 1))) == NULL) { > + irow = reallocarray(NULL, DB_NUMBER + 1, sizeof(char *)); > + if (irow == NULL) { > BIO_printf(bio_err, "Memory allocation failure\n"); > goto err; > } > Index: ecparam.c > === > RCS file: /cvs/src/lib/libssl/src/apps/ecparam.c,v > retrieving revision 1.10 > diff -u -p -r1.10 ecparam.c > --- ecparam.c 24 Apr 2014 12:22:22 - 1.10 > +++ ecparam.c 4 May 2014 19:36:00 - > @@ -312,7 +312,7 @@ bad: > > crv_len = EC_get_builtin_curves(NULL, 0); > > - curves = malloc((int) (sizeof(EC_builtin_curve) * crv_len)); > + curves = reallocarray(NULL, crv_len, sizeof(EC_builtin_curve)); > > if (curves == NULL) > goto end; > Index: speed.c > === > RCS file: /cvs/src/lib/libssl/src/apps/speed.c,v > retrieving revision 1.38 > diff -u -p -r1.38 speed.c > --- speed.c 2 May 2014 17:06:46 - 1.38 > +++ speed.c 4 May 2014 19:36:00 - > @@ -2178,7 +2178,7 @@ do_multi(int multi) > int *fds; > static char sep[] = ":"; > > - fds = malloc(multi * sizeof *fds); > + fds = reallocarray(NULL, multi, sizeof(int)); > for (n = 0; n < multi; ++n) { > if (pipe(fd) == -1) { > fprintf(stderr, "pipe failure\n"); > Index: srp.c > === > RCS file: /cvs/src/lib/libssl/src/apps/srp.c,v >
Re: 5.5 and dual-boot
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: duid support to dump
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 > +#include > #include > #include > #include > @@ -51,6 +52,7 @@ > #include > #include > #include > +#include > > #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: Kernel page fault in current
On Wed, 23 Oct 2013, Philip Guenther wrote: > On Tue, Oct 22, 2013 at 5:21 AM, Vladimir Támara Patiño > > wrote: > > Hi, I'm having problems with current, I'm using Xorg after some time > > there is a kernel trap (3 times I have seen the same one): > > Current is a moving target, so saying "current" without a timestamp or > at least date isn't precise. Indeed, there have been four commits to > the file holding wsdisplaypoll() since the Oct 18th. > > So, refresh, rebuild, and see whether the most recent commits affect > it. If it still happens, post a new trace, the date of your refresh, > AND THE DMESG THAT YOU LEFT OUT BEFORE. The dmesg was attached to the previous message as "rep-prob-X-oct2013.txt". That said, providing dmesgs inline is always more useful. -- "Action without study is fatal. Study without action is futile." -- Mary Ritter Beard
Re: threaded prof signals
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
usr.sbin/httpd and ECDHE
On Fri, 12 Jul 2013, Joel Sing wrote: > 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 2>&1 is" New, TLSv1/SSLv3, Cipher is ECDHE-RSA-AES256-GCM-SHA384 > And an improved version, after feedback from djm: 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 15 Jul 2013 15:31:19 - @@ -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, or `none' to disable. +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-dist15 Jul 2013 15:31:19 - @@ -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, or `none' to disable. +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 15 Jul 2013 15:31:19 - @@ -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 15 Jul 2013 15:31:19 - @@ -514,6 +514,7 @@ typedef struct { char*szCACertificateFile; char*szLogFile; char*szCipherSuite; +int nECDHCurve; 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_engin
Re: base apache and HonorCipherOrder
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 2>&1 szCertificateChain = NULL; sc->szLogFile = NULL; sc->szCipherSuite = NULL; +sc->szECDHCurve= NULL; sc->nLogLevel = SSL_LOG_NONE; sc->cipher_server_pref = UNSET; sc->nVerifyDepth = UNSET; @@ -253,6 +254,7 @@ void *ssl_config_server_merge(pool *p, v cfgMergeString(szCertificateChain); cfgMergeString(szLogFile); cfgMergeString(szCipherSuite); +cfgMergeString(szECDHCurve); cfgMergeBool(cipher_server_pref); cfgMerge(nLogLevel, SSL_LOG_NONE); cfgMergeInt(nVerifyDepth); @@ -549,6 +551,15 @@ const char *ssl_cmd_SSLCipherSuite( sc->szCipherSuite = arg; else dc->szCipherSuite = arg; +return NULL; +} + +const char *ssl_cmd_SSLECDHCurve( +cmd_parms *cmd, char *struct_ptr, char *arg) +{ +SSLSrvConfigRec *sc = mySrvConfig(cmd->server); + +sc->szECDHCurve = arg; return NULL; } Index: src/modules/ssl/ssl_engine_init.c === RCS file: /cvs/src/usr.sbin/httpd/src/modules/ssl/ssl_engine_init.c,v retrieving revision 1.29 diff -u -p -u -p -r1.29 ssl_engine_init.c --- src/modules/ssl/ssl_engine_init.c 11 Jul 2013 12:41:52 - 1.29 +++ src/modules/ssl/ssl_engine_init.c 11 Jul 2013 15:28:22 - @@ -530,6 +530,7 @@ void ssl_init_ConfigureServer(server_rec char *cpVHostID; EVP_PKEY *pKey; SSL_CTX *ctx; +EC_KEY *ecdhKey; STACK_OF(X509_NAME) *skCAList; ssl_asn1_t *asn1; unsigned char *ucp; @@ -537,7 +538,7 @@ void ssl_init_ConfigureServer(server_rec BOOL ok; BOOL bSkipFirst; int isca, pathlen; -int i, n; +int i, n, nid; /* * Create the server host:port string because we need it a lot @@ -639,6 +640,32 @@ void ssl_init_ConfigureServer(server_rec cpVHostID); ssl_die(); } +} + +/* + * Configure ECDH Curve + */ +if (sc->szECDHCurve != NULL) { +ssl_log(s, SSL_LOG_TRACE, +"Init: (%s) Configuring ECDH named curve [%s]", +cpVHostID, sc->szECDHCurve); +nid = OBJ_sn2nid((const char *)sc->szECDHCurve); +if (nid == 0) { +ssl_log(s, SSL_LOG_ERROR|SSL_ADD_SSLERR, +"Init: (%s) Unable to configure ECDH with named curve [%s]", +cpVHostID, sc->szECDHCurve); +ssl_die(); +} +ecdhKey = EC_KEY_new_by_curve_name(nid); +if (ecdhKey == NULL) { +ssl_log(s, SSL_LOG_ERROR|SSL_ADD_SSLERR, +"Init: (%s) Failed to create new EC key using named curve", +cpVHostID); +ssl_die(); +} +SSL_CTX_set_tmp_ecdh(ctx, ecdhKey); +SSL_CTX_set_options(ctx, SSL_OP_SINGLE_ECDH_USE); +EC_KEY_free(ecdhKey); } /* -- "Action without study is fatal. Study without action is futile." -- Mary Ritter Beard
Re: softraid(4) about boot support
On Tue, 12 Mar 2013, Alexander Polakov wrote: > It seems like this is not true any more. Right? Thanks, however I think we need to keep it but make it more explicit (only some disciplines are supported and only on some architectures). I'll work with jmc@ to get it fixed this week. > Index: softraid.4 > === > RCS file: /cvs/src/share/man/man4/softraid.4,v > retrieving revision 1.30 > diff -u -p -u -r1.30 softraid.4 > --- softraid.414 Aug 2012 01:08:19 - 1.30 > +++ softraid.411 Mar 2013 21:25:39 - > @@ -178,8 +178,6 @@ This is due to the scrub functionality n > .Pp > Currently there is no automated mechanism to recover from failed disks. > .Pp > -There is no boot support at this time for any disciplines. > -.Pp > Sparc hardware needs to use fstype > .Dq 4.2BSD > instead of -- "Action without study is fatal. Study without action is futile." -- Mary Ritter Beard
Re: bioctl patch testing
On Sun, 10 Feb 2013, Scott McEachern wrote: > Moving this to tech. > > I tested the patch found at > http://marc.info/?l=openbsd-tech&m=133513662106783&w=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: [PATCH] remove goto label in msdosfs_vfsops.c
On Wed, 21 Nov 2012, Michael W. Bombardieri wrote: > Hi Tech, > > I have a small msdosfs patch... > > Error cases are handled inconsistently in msdosfs_mount(). > Some cases use the construct "if (error) goto error;", > others just return error immediately. > The following patch removes the goto label and makes the > function return error consistently. > > Does this look OK, or is this change not worth the bother? IMO it is preferable to have all of the exit conditions as a goto - it is then completely obvious as to how much state needs to be cleaned up on exit (if any). > Index: msdosfs_vfsops.c > === > RCS file: /cvs/src/sys/msdosfs/msdosfs_vfsops.c,v > retrieving revision 1.62 > diff -u -r1.62 msdosfs_vfsops.c > --- msdosfs_vfsops.c 10 Sep 2012 11:10:59 - 1.62 > +++ msdosfs_vfsops.c 11 Oct 2012 09:18:01 - > @@ -168,12 +168,12 @@ >*/ > error = copyinstr(args.fspec, fspec, sizeof(fspec), NULL); > if (error) > - goto error; > + return (error); > disk_map(fspec, fspec, MNAMELEN, DM_OPENBLCK); > > NDINIT(ndp, LOOKUP, FOLLOW, UIO_SYSSPACE, fspec, p); > if ((error = namei(ndp)) != 0) > - goto error; > + return (error); > > devvp = ndp->ni_vp; > > @@ -232,7 +232,7 @@ > else { > if ((error = msdosfs_root(mp, &rvp)) != 0) { > msdosfs_unmount(mp, MNT_FORCE, p); > - goto error; > + return (error); > } > pmp->pm_flags |= findwin95(VTODE(rvp)) >? MSDOSFSMNT_LONGNAME > @@ -256,8 +256,6 @@ > > error_devvp: > vrele(devvp); > - > -error: > return (error); > } -- "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
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
softraid: CONCAT discipline
The following diff provides a softraid CONCAT discipline, which serially concatenates data over the given chunks. This relies on the previous metadata initialisation cleanup diff that I just sent out. All testing appreciated. ok? Index: sbin/bioctl/bioctl.c === RCS file: /cvs/src/sbin/bioctl/bioctl.c,v retrieving revision 1.103 diff -u -p -u -p -r1.103 bioctl.c --- sbin/bioctl/bioctl.c1 Aug 2011 08:23:52 - 1.103 +++ sbin/bioctl/bioctl.c26 Dec 2011 15:55:23 - @@ -393,6 +393,11 @@ bio_inq(char *name) switch (bv.bv_level) { case 'C': printf("%11s %-10s %14s %-7s CRYPTO%s%s\n", + volname, status, size, bv.bv_dev, + percent, seconds); + break; + case 'c': + printf("%11s %-10s %14s %-7s CONCAT%s%s\n", volname, status, size, bv.bv_dev, percent, seconds); break; Index: sbin/bioctl/bioctl.8 === RCS file: /cvs/src/sbin/bioctl/bioctl.8,v retrieving revision 1.84 diff -u -p -u -p -r1.84 bioctl.8 --- sbin/bioctl/bioctl.822 Dec 2010 16:25:32 - 1.84 +++ sbin/bioctl/bioctl.826 Dec 2011 15:55:24 - @@ -203,9 +203,13 @@ A striping discipline with floating pari .It C CRYPTO: An encrypting discipline. +.It c +CONCAT: +A concatenating discipline. .El .Pp -The RAID 0 and RAID 1 disciplines requires a minimum of two devices passed to +The RAID 0, RAID 1 and CONCAT disciplines require a minimum of two devices +passed to .Fl l , RAID 4 and RAID 5 require at least three devices, and the CRYPTO discipline requires exactly one. Index: share/man/man4/softraid.4 === RCS file: /cvs/src/share/man/man4/softraid.4,v retrieving revision 1.27 diff -u -p -u -p -r1.27 softraid.4 --- share/man/man4/softraid.4 8 Dec 2009 14:12:05 - 1.27 +++ share/man/man4/softraid.4 26 Dec 2011 15:55:24 - @@ -101,6 +101,12 @@ An discipline. It encrypts data on a single chunk to provide for data confidentiality. CRYPTO does not provide redundancy. +.It CONCAT +A +.Em concatenating +discipline. +It writes data to each chunk in serial, providing increased capacity. +CONCAT does not provide any form of redundancy. .El .Sh EXAMPLES An example to create a 3 chunk RAID 1 from scratch is as follows: Index: sys/conf/files === RCS file: /cvs/src/sys/conf/files,v retrieving revision 1.532 diff -u -p -u -p -r1.532 files --- sys/conf/files 24 Dec 2011 04:34:20 - 1.532 +++ sys/conf/files 26 Dec 2011 15:55:24 - @@ -491,6 +491,7 @@ filedev/softraid_raidp.csoftraid file dev/softraid_crypto.c softraid & crypto file dev/softraid_aoe.c softraid & ether & aoe file dev/softraid_raid6.csoftraid +file dev/softraid_concat.c softraid # SPD Memory EEPROM device spdmem Index: sys/dev/softraid.c === RCS file: /cvs/src/sys/dev/softraid.c,v retrieving revision 1.260 diff -u -p -u -p -r1.260 softraid.c --- sys/dev/softraid.c 26 Dec 2011 14:54:52 - 1.260 +++ sys/dev/softraid.c 26 Dec 2011 15:55:25 - @@ -3687,6 +3662,9 @@ sr_discipline_init(struct sr_discipline sr_crypto_discipline_init(sd); break; #endif + case 'c': + sr_concat_discipline_init(sd); + break; default: goto bad; } Index: sys/dev/softraid_concat.c === RCS file: sys/dev/softraid_concat.c diff -N sys/dev/softraid_concat.c --- /dev/null 1 Jan 1970 00:00:00 - +++ sys/dev/softraid_concat.c 26 Dec 2011 15:55:25 - @@ -0,0 +1,350 @@ +/* $OpenBSD: softraid_concat.c,v 1.22 2010/07/02 09:20:26 jsing Exp $ */ +/* + * Copyright (c) 2008 Marco Peereboom + * Copyright (c) 2011 Joel Sing + * + * 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, DA
softraid: cleanup metadata initialisation
The following diff cleans up the metadata initialisation process. The sr_meta_init() function initialises both the volume and chunk metadata and is called before the discipline specific sd_create() function is called. The sr_meta_init_complete() function is then called to complete the initialisation based on values provided by sd_create(). - The UUID is generated for the volume and then copied to the chunks. Previously this was generated for the chunks and then copied from chunk zero back into the volume. - ssd_data_offset is now initialised before we call sd_create(). The only functional change should be that the volid for chunks is no longer updated - it is not required since we use the uuids to do the chunk to volume matching and only need the volid in the volume in order to maintain attachment ordering. Testing would be appreciated, especially with new volume creation. ok? Index: softraid.c === RCS file: /cvs/src/sys/dev/softraid.c,v retrieving revision 1.260 diff -u -p -u -p -r1.260 softraid.c --- softraid.c 26 Dec 2011 14:54:52 - 1.260 +++ softraid.c 26 Dec 2011 15:30:00 - @@ -148,10 +148,8 @@ intsr_meta_attach(struct sr_disciplin intsr_meta_rw(struct sr_discipline *, dev_t, void *, size_t, daddr64_t, long); intsr_meta_clear(struct sr_discipline *); -void sr_meta_chunks_create(struct sr_softc *, - struct sr_chunk_head *); -void sr_meta_init(struct sr_discipline *, - struct sr_chunk_head *); +void sr_meta_init(struct sr_discipline *, int, int); +void sr_meta_init_complete(struct sr_discipline *); void sr_meta_opt_handler(struct sr_discipline *, struct sr_meta_opt_hdr *); @@ -525,85 +523,78 @@ done: } void -sr_meta_chunks_create(struct sr_softc *sc, struct sr_chunk_head *cl) +sr_meta_init(struct sr_discipline *sd, int level, int no_chunk) { - struct sr_chunk *ch_entry; - struct sr_uuid uuid; + struct sr_softc *sc = sd->sd_sc; + struct sr_metadata *sm = sd->sd_meta; + struct sr_chunk_head*cl = &sd->sd_vol.sv_chunk_list; + struct sr_meta_chunk*scm; + struct sr_chunk *chunk; int cid = 0; - char*name; - u_int64_t max_chunk_sz = 0, min_chunk_sz; + u_int64_t max_chunk_sz = 0, min_chunk_sz = 0; - DNPRINTF(SR_D_META, "%s: sr_meta_chunks_create\n", DEVNAME(sc)); + DNPRINTF(SR_D_META, "%s: sr_meta_init\n", DEVNAME(sc)); - sr_uuid_get(&uuid); + if (!sm) + return; - /* fill out stuff and get largest chunk size while looping */ - SLIST_FOREACH(ch_entry, cl, src_link) { - name = ch_entry->src_devname; - ch_entry->src_meta.scmi.scm_size = ch_entry->src_size; - ch_entry->src_meta.scmi.scm_chunk_id = cid++; - ch_entry->src_meta.scm_status = BIOC_SDONLINE; - strlcpy(ch_entry->src_meta.scmi.scm_devname, name, - sizeof(ch_entry->src_meta.scmi.scm_devname)); - bcopy(&uuid, &ch_entry->src_meta.scmi.scm_uuid, - sizeof(ch_entry->src_meta.scmi.scm_uuid)); + /* Initialise volume metadata. */ + sm->ssdi.ssd_magic = SR_MAGIC; + sm->ssdi.ssd_version = SR_META_VERSION; + sm->ssdi.ssd_vol_flags = sd->sd_meta_flags; + sm->ssdi.ssd_volid = 0; + sm->ssdi.ssd_chunk_no = no_chunk; + sm->ssdi.ssd_level = level; - if (ch_entry->src_meta.scmi.scm_size > max_chunk_sz) - max_chunk_sz = ch_entry->src_meta.scmi.scm_size; - } + sm->ssd_data_offset = SR_DATA_OFFSET; + sm->ssd_ondisk = 0; - /* get smallest chunk size */ - min_chunk_sz = max_chunk_sz; - SLIST_FOREACH(ch_entry, cl, src_link) - if (ch_entry->src_meta.scmi.scm_size < min_chunk_sz) - min_chunk_sz = ch_entry->src_meta.scmi.scm_size; + sr_uuid_get(&sm->ssdi.ssd_uuid); - /* equalize all sizes */ - SLIST_FOREACH(ch_entry, cl, src_link) - ch_entry->src_meta.scmi.scm_coerced_size = min_chunk_sz; + /* Initialise chunk metadata and get min/max chunk sizes. */ + SLIST_FOREACH(chunk, cl, src_link) { + scm = &chunk->src_meta; + scm->scmi.scm_size = chunk->src_size; + scm->scmi.scm_chunk_id = cid++; + scm->scm_status = BIOC_SDONLINE; + scm->scmi.scm_volid = 0; + strlcpy(scm->scmi.scm_devname, chunk->src_devname, + sizeof(scm->scmi.scm_devname)); + bcopy(&sm->ssdi.ssd_uuid, &scm->
sysctl__string fails to return old length
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
dkcsum: do not sleep whilst walking alldevs
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 #include #include +#include #include #include +#include #include #include #include @@ -44,16 +46,23 @@ #include -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) hit->bsd_dev = MAKEBOOTDEV(major(bp->b_dev), 0, 0, DISKUNIT(bp->b_dev)
Re: Future of ccd(4) and raid(4)?
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 sd 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
Re: Future of ccd(4) and raid(4)?
On Saturday 25 June 2011, Christian Weisgerber wrote: > 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. > > 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)?
On Friday 24 June 2011, Matthew Dempsky wrote: > On Thu, Jun 23, 2011 at 7:29 PM, Kenneth R Westerback > > 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: OpenBSD 4.8 RAID 0+1 or 1+0 or 5
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: 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: /bsd: splassert: assertwaitok: want -1 have 1
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: /bsd: splassert: assertwaitok: want -1 have 1
On Thursday 20 January 2011, Gregory Edigarov wrote: > On Wed, 19 Jan 2011 20:14:01 +1100 > > Joel Sing 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
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. -- "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
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",
Re: bioctl should retry passphrase
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 To: merlyn CC: Marco Peereboom , tech@openbsd.org Date: 2010-09-15 05:21 On Tue, Sep 14, 2010 at 3:46 PM, merlyn 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
Change vnd disk name based on mode
When a vnd(4) device is configured the device name is always configured as vndX, even when it is created as a "safe" vnd (or svndX). This device name is also used as the name for the disk: $ vnconfig -c svnd0 /tmp/test $ sysctl hw.disknames hw.disknames=sd0:19291b8cb83eb8b8,cd0:,vnd0: When DUIDs are used we end up mapping back to a device node using the name of the disk. This means that we always end up using /dev/vnd* even for svnd disks. The following diff adds a separate disk name that is populated based on the mode that the vnd(4) is created with. This means that if you created a vnd disk then the disk name will be vndX and diskmap(4) will open the /dev/vnd* devices. Whereas if you created a svnd disk then the disk name will be svndX and diskmap(4) will open /dev/svnd* devices. ok? Index: vnd.c === RCS file: /cvs/src/sys/dev/vnd.c,v retrieving revision 1.103 diff -u -p -r1.103 vnd.c --- vnd.c 22 Sep 2010 01:18:57 - 1.103 +++ vnd.c 16 Nov 2010 14:37:41 - @@ -125,6 +125,7 @@ struct pool vndbufpl; struct vnd_softc { struct devicesc_dev; struct disk sc_dk; + char sc_dk_name[16]; char sc_file[VNDNLEN]; /* file we're covering */ int sc_flags; /* flags */ @@ -780,6 +781,7 @@ vndioctl(dev_t dev, u_long cmd, caddr_t return (error); } + /* Set device name. */ bzero(vnd->sc_dev.dv_xname, sizeof(vnd->sc_dev.dv_xname)); if (snprintf(vnd->sc_dev.dv_xname, sizeof(vnd->sc_dev.dv_xname), "vnd%d", unit) >= sizeof(vnd->sc_dev.dv_xname)) { @@ -788,6 +790,16 @@ vndioctl(dev_t dev, u_long cmd, caddr_t return(ENXIO); } + /* Set disk name depending on how we were created. */ + bzero(vnd->sc_dk_name, sizeof(vnd->sc_dk_name)); + if (snprintf(vnd->sc_dk_name, sizeof(vnd->sc_dk_name), + "%svnd%d", ((vnd->sc_flags & VNF_SIMPLE) ? "s" : ""), + unit) >= sizeof(vnd->sc_dk_name)) { + printf("VNDIOCSET: disk name too long\n"); + vndunlock(vnd); + return(ENXIO); + } + /* Set geometry for device. */ vnd->sc_secsize = vio->vnd_secsize; vnd->sc_ntracks = vio->vnd_ntracks; @@ -865,7 +877,7 @@ vndioctl(dev_t dev, u_long cmd, caddr_t vnd->sc_vp, (unsigned long long)vnd->sc_size); /* Attach the disk. */ - vnd->sc_dk.dk_name = vnd->sc_dev.dv_xname; + vnd->sc_dk.dk_name = vnd->sc_dk_name; disk_attach(&vnd->sc_dev, &vnd->sc_dk); vndunlock(vnd); -- "Stop assuming that systems are secure unless demonstrated insecure; start assuming that systems are insecure unless designed securely." - Bruce Schneier
Teach msdos_mount() about DUIDs
This allows MS-DOS file systems to be mounted by DUID. The primary change is the inclusion of the disk_map() call, however we need to modify the error handling to suit. Some long lines wrapped for free. ok? Index: msdosfs_vfsops.c === RCS file: /cvs/src/sys/msdosfs/msdosfs_vfsops.c,v retrieving revision 1.58 diff -u -p -r1.58 msdosfs_vfsops.c --- msdosfs_vfsops.c23 Sep 2010 18:40:00 - 1.58 +++ msdosfs_vfsops.c13 Nov 2010 16:51:53 - @@ -62,6 +62,7 @@ #include #include #include +#include #include #include @@ -103,10 +104,12 @@ msdosfs_mount(struct mount *mp, const ch size_t size; int error, flags; mode_t accessmode; + char *fspec = NULL; error = copyin(data, &args, sizeof(struct msdosfs_args)); if (error) return (error); + /* * If updating, check whether changing from read-only to * read/write; if there is no device name, that's all we do. @@ -114,7 +117,8 @@ msdosfs_mount(struct mount *mp, const ch if (mp->mnt_flag & MNT_UPDATE) { pmp = VFSTOMSDOSFS(mp); error = 0; - if (!(pmp->pm_flags & MSDOSFSMNT_RONLY) && (mp->mnt_flag & MNT_RDONLY)) { + if (!(pmp->pm_flags & MSDOSFSMNT_RONLY) && + (mp->mnt_flag & MNT_RDONLY)) { flags = WRITECLOSE; if (mp->mnt_flag & MNT_FORCE) flags |= FORCECLOSE; @@ -125,7 +129,8 @@ msdosfs_mount(struct mount *mp, const ch error = EOPNOTSUPP; if (error) return (error); - if ((pmp->pm_flags & MSDOSFSMNT_RONLY) && (mp->mnt_flag & MNT_WANTRDWR)) { + if ((pmp->pm_flags & MSDOSFSMNT_RONLY) && + (mp->mnt_flag & MNT_WANTRDWR)) { /* * If upgrade to read-write by non-root, then verify * that user has necessary permissions on the device. @@ -157,23 +162,31 @@ msdosfs_mount(struct mount *mp, const ch &args.export_info)); } } + /* * Not an update, or updating the name: look up the name * and verify that it refers to a sensible block device. */ - NDINIT(ndp, LOOKUP, FOLLOW, UIO_USERSPACE, args.fspec, p); + fspec = malloc(MNAMELEN, M_MOUNT, M_WAITOK); + if ((error = copyinstr(args.fspec, fspec, MNAMELEN - 1, &size)) != 0) + goto error; + disk_map(fspec, fspec, MNAMELEN, DM_OPENBLCK); + + NDINIT(ndp, LOOKUP, FOLLOW, UIO_SYSSPACE, fspec, p); if ((error = namei(ndp)) != 0) - return (error); + goto error; + devvp = ndp->ni_vp; if (devvp->v_type != VBLK) { - vrele(devvp); - return (ENOTBLK); + error = ENOTBLK; + goto error_devvp; } if (major(devvp->v_rdev) >= nblkdev) { - vrele(devvp); - return (ENXIO); + error = ENXIO; + goto error_devvp; } + /* * If mount by non-root, then verify that user has necessary * permissions on the device. @@ -184,12 +197,11 @@ msdosfs_mount(struct mount *mp, const ch accessmode |= VWRITE; vn_lock(devvp, LK_EXCLUSIVE | LK_RETRY, p); error = VOP_ACCESS(devvp, accessmode, p->p_ucred, p); - if (error) { - vput(devvp); - return (error); - } VOP_UNLOCK(devvp, 0, p); + if (error) + goto error_devvp; } + if ((mp->mnt_flag & MNT_UPDATE) == 0) error = msdosfs_mountfs(devvp, mp, p, &args); else { @@ -198,10 +210,9 @@ msdosfs_mount(struct mount *mp, const ch else vrele(devvp); } - if (error) { - vrele(devvp); - return (error); - } + if (error) + goto error_devvp; + pmp = VFSTOMSDOSFS(mp); pmp->pm_gid = args.gid; pmp->pm_uid = args.uid; @@ -210,7 +221,8 @@ msdosfs_mount(struct mount *mp, const ch if (pmp->pm_flags & MSDOSFSMNT_NOWIN95) pmp->pm_flags |= MSDOSFSMNT_SHORTNAME; - else if (!(pmp->pm_flags & (MSDOSFSMNT_SHORTNAME | MSDOSFSMNT_LONGNAME))) { + else if (!(pmp->pm_flags & + (MSDOSFSMNT_SHORTNAME | MSDOSFSMNT_LONGNAME))) { struct vnode *rvp; /* @@ -221,7 +233,7 @@ msdosfs_mount(struct mount *mp, const ch else { if ((error = msdosfs_root(mp, &rvp)) != 0) { msdosfs_unmount(mp, MNT_FORCE, p); -
Make fsck_msdos work with DUIDs
The following diff switches fsck_msdos to opendev() so that it works with disklabel UIDs (DUIDs). The output is also changed to follow fsck_ffs(8) (i.e. print the realname then the DUID in brackets). ok? Index: Makefile === RCS file: /cvs/src/sbin/fsck_msdos/Makefile,v retrieving revision 1.4 diff -u -p -r1.4 Makefile --- Makefile21 Sep 1997 11:36:40 - 1.4 +++ Makefile13 Nov 2010 16:30:36 - @@ -3,7 +3,9 @@ PROG= fsck_msdos MAN= fsck_msdos.8 SRCS= main.c check.c boot.c fat.c dir.c fsutil.c -CFLAGS+= -I${.CURDIR}/../fsck .PATH: ${.CURDIR}/../fsck +CFLAGS+= -I${.CURDIR}/../fsck +DPADD+=${LIBUTIL} +LDADD+=-lutil .include Index: check.c === RCS file: /cvs/src/sbin/fsck_msdos/check.c,v retrieving revision 1.12 diff -u -p -r1.12 check.c --- check.c 27 Oct 2009 23:59:33 - 1.12 +++ check.c 13 Nov 2010 16:30:36 - @@ -33,6 +33,7 @@ * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. */ +#include #include #include #include @@ -48,29 +49,31 @@ checkfilesys(const char *fname) int dosfs; struct bootblock boot; struct fatEntry *fat = NULL; + char *realdev; int i; int mod = 0; rdonly = alwaysno; - if (!preen) - printf("** %s", fname); - dosfs = open(fname, rdonly ? O_RDONLY : O_RDWR, 0); + dosfs = opendev(fname, rdonly ? O_RDONLY : O_RDWR, 0, &realdev); if (dosfs < 0 && !rdonly) { - dosfs = open(fname, O_RDONLY, 0); - if (dosfs >= 0) - pwarn(" (NO WRITE)\n"); - else if (!preen) - printf("\n"); + dosfs = opendev(fname, O_RDONLY, 0, &realdev); rdonly = 1; - } else if (!preen) - printf("\n"); - + } if (dosfs < 0) { xperror("Can't open"); return (8); } + if (!preen) { + printf("** %s", realdev); + if (strncmp(fname, realdev, PATH_MAX) != 0) + printf(" (%s)", fname); + if (rdonly) + printf(" (NO WRITE)"); + printf("\n"); + } + if (readboot(dosfs, &boot) != FSOK) { (void)close(dosfs); return (8); -- "Stop assuming that systems are secure unless demonstrated insecure; start assuming that systems are insecure unless designed securely." - Bruce Schneier