I may not have time to fully digest the change before the release date, but
I'm not sure this snippet quite works:
if (ctx->read_start == ctx->read_end) { /* time to read more data */
ctx->read_end = ctx->read_start = &(ctx->buf[BUF_OFFSET]);
ctx->read_end += BIO_read(next, ctx->r
On Sun, Jul 31, 2016 at 6:18 PM Michel via RT wrote:
> > I was able to trigger a crash simply by chaining an encrypt BIO with a
> memory BIO containing a large plaintext and then stream 100 bytes out of it
> at a time. BIO_read would consistently return 128 and, by the time the
> function returne
Hey folks,
I do not believe this fix works and introduces buffer overflows.
Looking at this change:
https://git.openssl.org/gitweb/?p=openssl.git;a=commitdiff;h=abdb460d8abe68fedcf00b52d2ba4bf4b7c1725c
It makes EVP_CipherUpdate write to out directly. While not unreasonable
(this BIO probably sho
On Fri, Jul 29, 2016 at 9:21 AM Matt Caswell via RT wrote:
> On Tue Jun 14 20:30:09 2016, david...@google.com wrote:
> > I recently made some changes around BoringSSL's SSL_set_bio, etc.
> > which you
> > all might be interested in. The BIO management has two weird behaviors
> > right now:
> >
>
On Fri, Jul 22, 2016 at 7:30 PM Hubert Kario via RT wrote:
> On Friday, 22 July 2016 17:14:43 CEST Stephen Henson via RT wrote:
> > On Fri Jul 22 14:56:11 2016, hka...@redhat.com wrote:
> > > the issue is present in master 0ed26acce328ec16a3aa and looks to have
> > > been
> >
> > > introduced in
On Fri, Jun 17, 2016 at 8:48 AM Matt Caswell via RT wrote:
>
>
> On 14/06/16 21:30, David Benjamin via RT wrote:
> > For OpenSSL master, I believe it'd also work to add an s->rbio != s->wbio
> > check to SSL_set_rbio, but I think those are worse semantics for
&
Hey folks,
It seems the X509_LU_* values (now an X509_LOOKUP_TYPE enum in master) are
slightly confused. See this commit message and diff for details:
https://boringssl.googlesource.com/boringssl/+/da7f0c65efb72556f8fc92e460e6c90cd1b1add7%5E%21/
The relevant point is that X509_LU_RETRY doesn't wo
I don't think that will work. The SSL code uses in-place buffers
extensively, so in == out definitely needs to be defined. The question is
only whether out < in is also acceptable.
Either way, for BoringSSL, I've gone ahead and tightened our aliasing
constraints to forbid out < in and require equa
I recently made some changes around BoringSSL's SSL_set_bio, etc. which you
all might be interested in. The BIO management has two weird behaviors
right now:
1. The existence of bbio is leaked in the public API when it should be an
implementation detail. (Otherwise you're stuck with it for DTLS wh
On Mon, Jun 13, 2016 at 4:04 AM Matt Caswell via RT wrote:
> On Thu Jun 02 23:24:44 2016, paul.d...@oracle.com wrote:
> > The DTLS packet reassembly code has a performance problem that could
> > result in a DoS attack being possible.
> >
> >
> >
> > The DTLS packet reassembly uses the data struct
On Tue, Mar 29, 2016 at 12:17 PM Emilia Käsper wrote:
> While we're at this, shouldn't we then also check the length in oct2priv?
> (And
> either reject or reduce mod n.) Afaics it accepts arbitrary BNs currently,
> which means some keys can be parsed but cannot be re-encoded?
>
Probably. Boring
On Tue, Mar 29, 2016 at 9:47 AM Andy Polyakov via RT wrote:
> > In the non-SIMD paths, I believe this is fine because $r0's and $r1's
> > cleared high bits mean we should have plenty of slack to leave that
> > unreduced. (And indeed its normally not reduced on input from the
> > addition.) Then p
On Fri, Mar 25, 2016 at 3:26 PM David Benjamin wrote:
> Right. What I meant is that a fully reduced h has $h2 < 4. Is it possible
> that $h2, after that adc, ends up at 4, exceeding that bound? If it were,
> that would require one more reduction.
>
> In the non-SIMD paths, I believe this is fine
On Fri, Mar 25, 2016 at 3:07 PM Andy Polyakov via RT wrote:
> > For x86-64, this seems to be the bug:
> >
> > $ git diff
> > diff --git a/crypto/poly1305/asm/poly1305-x86_64.pl
> b/crypto/poly1305/asm/
> > poly1305-x86_64.pl
> > index 3c810c5..bc14ed1 100755
> > --- a/crypto/poly1305/asm/poly1305
For x86-64, this seems to be the bug:
$ git diff
diff --git a/crypto/poly1305/asm/poly1305-x86_64.pl b/crypto/poly1305/asm/
poly1305-x86_64.pl
index 3c810c5..bc14ed1 100755
--- a/crypto/poly1305/asm/poly1305-x86_64.pl
+++ b/crypto/poly1305/asm/poly1305-x86_64.pl
@@ -97,6 +97,7 @@ $code.=<<___;
On Sun, Mar 20, 2016 at 10:47 PM David Benjamin wrote:
> On Sun, Mar 20, 2016 at 5:05 PM Andy Polyakov via RT
> wrote:
>
>> No, it doesn't depend on call pattern. Please confirm that attached
>> patch solves the problem. Thanks.
>>
>
> (Right, sorry, I meant that the test vectors I have seem to
On Sun, Mar 20, 2016 at 5:05 PM Andy Polyakov via RT wrote:
> No, it doesn't depend on call pattern. Please confirm that attached
> patch solves the problem. Thanks.
>
(Right, sorry, I meant that the test vectors I have seem to only with their
corresponding call patterns.)
The patch works on my
Patch attached. This is a mechanical change. BIO_new takes a non-const
BIO_METHOD and the various BIO_METHODs defined in the library are also
non-const, so they don't get placed in .rodata.
The change to BIO_new and the BIO struct should be source-compatible.
Fixing the in-library BIO_METHODs is n
On Thu, Mar 17, 2016 at 5:22 PM David Benjamin via RT
wrote:
> I'm probably going to write something to generate random inputs and stress
> all your other poly1305 codepaths against a reference implementation. I
> recommend doing the same in your own test harness, to make sure
Hi folks,
You know the drill. See the attached poly1305_test2.c.
$ OPENSSL_ia32cap=0 ./poly1305_test2
PASS
$ ./poly1305_test2
Poly1305 test failed.
got: 2637408fe03086ea73f971e3425e2820
expected: 2637408fe13086ea73f971e3425e2820
I believe this affects both the SSE2 and AVX2 code. It does se
The current state is that, as far as I can tell, overlapping requirements
are undocumented (or is it somewhere and I missed it?) and, for ChaCha,
architecture-specific. I think something certainly needs to be done. Either
changing chacha-x86.pl and allowing any out <= in overlap, or declaring
that
By the way, returning the original subject, I don't believe there is a leak
here.
If EC_GROUP_copy fails, dest still exists and is owned by the caller. It's
the caller's obligation to call EC_GROUP_free and that will release the
partially-copied EC_GROUP. (Which will, with this patch, cause a
doub
On Mon, Mar 7, 2016 at 5:08 PM David Benjamin via RT wrote:
> No patch for this one since I'm not that familiar with your state machine.
> If the peer sends handshake messages fragmented across records such that
> the handshake message header is split over two records AND the two
ssl3_get_record silently discards empty records without much context, which
means OpenSSL will happily accept, e.g., empty app data records
mid-handshake or empty records of bogus type. They get silently discarded
and never returned to the caller, so this is harmless, just a little odd.
This is wh
No patch for this one since I'm not that familiar with your state machine.
If the peer sends handshake messages fragmented across records such that
the handshake message header is split over two records AND the two records
are received in different steps asynchronously, OpenSSL fails to reassemble
The private key is a scalar and should be sized by the order, not the
degree.
(Unlike my other recent emails, this has nothing to do with BoringSSL
tests. :-) )
David
--
Ticket here: http://rt.openssl.org/Ticket/Display.html?id=4393
Please log in as guest with password guest if prompted
0007
Session resumption involves a version check, so version negotiation must
happen first. Currently, the DTLS implementation cannot do session
resumption in DTLS 1.0 because the ssl_version check always checks against
1.2.
Switching the order also removes the need to fixup ssl_version in DTLS
version
ChangeCipherSpec messages have a defined value. They also may not occur in
the middle of a handshake message. The current logic will accept a
ChangeCipherSpec with value 2. It also would accept up to three bytes of
handshake data before the ChangeCipherSpec which it would discard (because
s->init_n
Per RFC 5246,
Note: this extension is not meaningful for TLS versions prior to 1.2.
Clients MUST NOT offer it if they are offering prior versions.
However, even if clients do offer it, the rules specified in [TLSEXT]
require servers to ignore extensions they do not understand.
Alt
Per RFC 4507, section 3.3:
This message [NewSessionTicket] MUST be sent if the
server included a SessionTicket extension in the ServerHello. This
message MUST NOT be sent if the server did not include a
SessionTicket extension in the ServerHello.
The presence of the NewSessionTicket
If the server consumer configures NPN and not ALPN, OpenSSL should still
resolve NPN against clients which advertises it. (NB: Chrome will be
removing NPN soon, so hopefully there won't be any such consumers.) Losing
the alpn_select_cb check makes OpenSSL depend on whether ALPN or NPN comes
first i
The V2ClientHello code creates an empty compression list, but the
compression list must explicitly contain the null compression (and later
code enforces this). As a result, all V2ClientHellos currently get rejected
on master.
The SendV2ClientHello-Sync test in BoringSSL's test suite can be used to
See attached. OpenSSL can't actually represent large universal tags because
it collides with the V_ASN1_NEG flag, yet it happily parses them in high
tag number form. d2i_ASN1_TYPE interprets 1f82020100 as a negative zero,
rather than an element with tag [UNIVERSAL 258].
I've intentionally made the
I'm unclear on what EVP_CIPHER's interface guarantees are, but our EVP_AEAD
APIs are documented to allow in/out buffers to alias as long as out is <=
in. This matches what callers might expect from a naive implementation.
Our AES-GCM EVP_AEADs, which share code with OpenSSL, have tended to match
t
On Fri, Feb 26, 2016 at 4:48 PM Andy Polyakov via RT wrote:
> > There seems to be a bug in the AVX2 codepath in poly1305-x86.pl. I have
> not
> > attempted to debug this, but I have attached a test file which produces
> > different output in normal and AVX2 codepaths. Our existing poly1305
> > im
On Thu, Feb 25, 2016 at 6:16 PM David Benjamin wrote:
> From looking at valgrind, this pattern seems to give good coverage. I
> used valgrind --tool=callgrind --dump-instr=yes --collect-jumps=yes and
> then kcachegrind to inspect the output. (kcachegrind is a bit heavy for
> this. I'm hoping I ca
There seems to be a bug in the AVX2 codepath in poly1305-x86.pl. I have not
attempted to debug this, but I have attached a test file which produces
different output in normal and AVX2 codepaths. Our existing poly1305
implementation agrees with the former.
$ OPENSSL_ia32cap=0 ./poly1305_test
PASS
$
Patch attached. This is just a little cleanup change to fix not everything
using the OPENSSL_armcap constants. (Existing ones already are using them,
so I'm assuming this is okay.)
David
--
Ticket here: http://rt.openssl.org/Ticket/Display.html?id=4341
Please log in as guest with password guest
On Sun, Feb 21, 2016 at 3:27 PM Andy Polyakov via RT wrote:
> Hi,
>
> > The partial-block tail code in chacha-armv4.pl also seems to have
> problems.
> > My colleague Steven and I made an attempt to debug it, but we're not
> > familiar enough with ARM to fix it.
> >
> > From playing with it in a
Hi Andy,
The partial-block tail code in chacha-armv4.pl also seems to have problems.
My colleague Steven and I made an attempt to debug it, but we're not
familiar enough with ARM to fix it.
>From playing with it in a debugger, it doesn't look like @t[3] contains the
length. We suspect something i
Hi folks,
I've started playing with the ChaCha20 assembly that was recently checked
in and found a few problems. Most of these do not affect OpenSSL as you
only ever call ChaCha20_ctr32 on a whole number of blocks. But this isn't
documented as a constraint in internal/chacha.h and the assembly has
The recently-added DH_CHECK_PUBKEY_INVALID was set to 0x3, but
DH_CHECK_PUBKEY_* values are flags, so it should be 0x4 to avoid colliding
with DH_CHECK_PUBKEY_TOO_SMALL (0x01) and DH_CHECK_PUBKEY_TOO_LARGE (0x02).
See DH_check_pub_key's *ret |= logic.
https://git.openssl.org/gitweb/?p=openssl.git;
DSAPublicKey lost the dsa_cb in
https://git.openssl.org/gitweb/?p=openssl.git;a=commitdiff;h=ea6b07b54c1f8fc2275a121cdda071e2df7bd6c1
This results in d2i_DSAPublicKey using crypto/asn1's default allocation
logic rather than calling into DSA_new. I believe it should
use ASN1_SEQUENCE_cb.
I've atta
On Thu, Dec 17, 2015 at 2:43 PM Kurt Roeckx via RT wrote:
> On Wed, Dec 16, 2015 at 11:34:56PM +0000, David Benjamin via RT wrote:
> > EVP_MD_CTX_copy_ex is implemented with memcpy, followed by manually
> fixing
> > up |out->pctx| and |out->md_data|.
> >
> >
EVP_MD_CTX_copy_ex is implemented with memcpy, followed by manually fixing
up |out->pctx| and |out->md_data|.
https://git.openssl.org/gitweb/?p=openssl.git;a=blob;f=crypto/evp/digest.c;h=5da0e01039a6da039942db9f1bf8b70753f509f2;hb=HEAD#l292
If allocating |out->md_data| fails, then both |out->pctx
dsa_do_sign retries the operation if |r| or |s| end up zero. This results
in leaking the first iteration's value of |ret| since you end up clobbering
the previous allocation.
https://git.openssl.org/gitweb/?p=openssl.git;a=blob;f=crypto/dsa/dsa_ossl.c;h=34b4a4ea4a267b62b21916a85ab79350cd276065;hb=H
On Wed, Nov 4, 2015 at 7:04 AM Matt Caswell via RT wrote:
>
>
> On 03/11/15 17:43, David Benjamin via RT wrote:
>
> > I'm not sure that fix quite works though. If BIO_flush completes
> > asynchronously
>
> Ahhh, yes good point. Updated patch attached.
>
&g
On Tue, Nov 3, 2015 at 12:42 PM David Benjamin wrote:
> I'm not sure that fix quite works though. If BIO_flush completes
> asynchronously (hrm, it's missing an rwstate update), then I believe you'll
> be in a state where you *do* want to repeat the init_off / init_num adjust.
> You might be able
On Tue, Nov 3, 2015 at 11:16 AM Matt Caswell via RT wrote:
> Whilst investigating this I noticed another bug which is actually
> probably more significant. My eyeball only look at the BoringSSL source
> suggests that it is there too, so I'm not sure why you haven't seen it
> in the test that you
It seems unlikely I'll be getting around to doing another newsletter, but
while I'm reporting bugs, here's another that came to mind:
RFC 6066 is somewhat obnoxious and allows the server to decline to send
CertificateStatus even after negotiating the extension.
https://tools.ietf.org/html/rfc6066
Hey folks,
We found a small DTLS bug while writing some tests. I think it affects
1.0.1 and 1.0.2 too, so I thought I'd send you a note. (Note sure about
master. I'm unfamiliar with the new state machine mechanism.)
In DTLS, each ClientHello is supposed to reset the handshake hash (in case
of Hel
51 matches
Mail list logo