David Benjamin [Sun, 6 Mar 2016 03:50:44 +0000 (22:50 -0500)]
Tighten up logic around ChangeCipherSpec.
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_num gets reset).
Instead, require that s->init_num is 0 when a ChangeCipherSpec comes in.
RT#4391
Reviewed-by: Andy Polyakov <appro@openssl.org> Reviewed-by: Matt Caswell <matt@openssl.org>
Matt Caswell [Tue, 17 May 2016 11:28:14 +0000 (12:28 +0100)]
Simplify SSL BIO buffering logic
The write BIO for handshake messages is bufferred so that we only write
out to the network when we have a complete flight. There was some
complexity in the buffering logic so that we switched buffering on and
off at various points through out the handshake. The only real reason to
do this was historically it complicated the state machine when you wanted
to flush because you had to traverse through the "flush" state (in order
to cope with NBIO). Where we knew up front that there was only going to
be one message in the flight we switched off buffering to avoid that.
In the new state machine there is no longer a need for a flush state so
it is simpler just to have buffering on for the whole handshake. This
also gives us the added benefit that we can simply call flush after every
flight even if it only has one message in it. This means that BIO authors
can implement their own buffering strategies and not have to be aware of
the state of the SSL object (previously they would have to switch off
their own buffering during the handshake because they could not rely on
a flush being received when they really needed to write data out). This
last point addresses GitHub Issue #322.
Andy Polyakov [Mon, 16 May 2016 14:44:33 +0000 (16:44 +0200)]
rand/randfile.c: remove _XOPEN_SOURCE definition.
Defintions of macros similar to _XOPEN_SOURCE belong in command line
or in worst case prior first #include directive in source. As for
macros is was allegedly controlling. One can argue that we are
probably better off demanding S_IS* macros but there are systems
that just don't comply, hence this compromise solution...
Viktor Dukhovni [Fri, 13 May 2016 04:36:56 +0000 (00:36 -0400)]
When strict SCT fails record verification failure
Since with SSL_VERIFY_NONE, the connection may continue and the
session may even be cached, we should save some evidence that the
chain was not sufficiently verified and would have been rejected
with SSL_VERIFY_PEER. To that end when a CT callback returs failure
we set the verify result to X509_V_ERR_NO_VALID_SCTS.
Note: We only run the CT callback in the first place if the verify
result is still X509_V_OK prior to start of the callback.
Viktor Dukhovni [Tue, 17 May 2016 17:40:57 +0000 (13:40 -0400)]
Ensure verify error is set when X509_verify_cert() fails
Set ctx->error = X509_V_ERR_OUT_OF_MEM when verificaiton cannot
continue due to malloc failure. Also, when X509_verify_cert()
returns <= 0 make sure that the verification status does not remain
X509_V_OK, as a last resort set it it to X509_V_ERR_UNSPECIFIED,
just in case some code path returns an error without setting an
appropriate value of ctx->error.
Reviewed-by: Richard Levitte <levitte@openssl.org>
Kazuki Yamaguchi [Tue, 10 May 2016 10:46:08 +0000 (19:46 +0900)]
Fix a NULL dereference in chacha20_poly1305_init_key()
chacha20_poly1305_init_key() dereferences NULL when called with inkey !=
NULL && iv == NULL. This function is called by EVP_EncryptInit_ex()
family, whose documentation allows setting key and iv in separate calls.
Reviewed-by: Matt Caswell <matt@openssl.org> Reviewed-by: Richard Levitte <levitte@openssl.org>
Matt Caswell [Tue, 17 May 2016 14:27:09 +0000 (15:27 +0100)]
Add a comment to explain the use of |num_recs|
In the SSLV2ClientHello processing code in ssl3_get_record, the value of
|num_recs| will always be 0. This isn't obvious from the code so a comment
is added to explain it.
Matt Caswell [Tue, 26 Apr 2016 15:07:17 +0000 (16:07 +0100)]
Use the current record offset in ssl3_get_record
The function ssl3_get_record() can obtain multiple records in one go
as long as we are set up for pipelining and all the records are app
data records. The logic in the while loop which reads in each record is
supposed to only continue looping if the last record we read was app data
and we have an app data record waiting in the buffer to be processed. It
was actually checking that the first record had app data and we have an
app data record waiting. This actually amounts to the same thing so wasn't
wrong - but it looks a bit odd because it uses the |rr| array without an
offset.
Matt Caswell [Tue, 26 Apr 2016 15:00:09 +0000 (16:00 +0100)]
There is only one read buffer
Pipelining introduced the concept of multiple records being read in one
go. Therefore we work with an array of SSL3_RECORD objects. The pipelining
change erroneously made a change in ssl3_get_record() to apply the current
record offset to the SSL3_BUFFER we are using for reading. This is wrong -
there is only ever one read buffer. This reverts that change. In practice
this should make little difference because the code block in question is
only ever used when we are processing a single record.
Matt Caswell [Mon, 16 May 2016 15:54:28 +0000 (16:54 +0100)]
Workaround an IO::Socket::IP bug
Workaround an apparent IO:Socket::IP bug where a seemingly valid
server socket is being returned even though a valid connection does not
exist. This causes the tests to intermittently hang. We additionally check
that the peerport looks ok to verify that the returned socket looks usable.
Reviewed-by: Richard Levitte <levitte@openssl.org>
Kazuki Yamaguchi [Sat, 12 Dec 2015 15:51:06 +0000 (00:51 +0900)]
Fix NPN protocol name list validation
Since 50932c4 "PACKETise ServerHello processing",
ssl_next_proto_validate() incorrectly allows empty protocol name.
draft-agl-tls-nextprotoneg-04[1] says "Implementations MUST ensure that
the empty string is not included and that no byte strings are
truncated."
This patch restores the old correct behavior.
Reviewed-by: Kurt Roeckx <kurt@openssl.org> Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/1057)
FdaSilvaYY [Mon, 9 May 2016 16:48:13 +0000 (18:48 +0200)]
Fix various methods declaration in pod file
Reviewed-by: Kurt Roeckx <kurt@openssl.org> Reviewed-by: Matt Caswell <matt@openssl.org> Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/1042)
The current limit of 2^14 bytes is too low (e.g. RFC 5246 specifies the
maximum size of just the extensions field to be 2^16-1), and may cause
bogus failures.
RT#4063
Reviewed-by: Kurt Roeckx <kurt@openssl.org> Reviewed-by: Rich Salz <rsalz@openssl.org> Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/413)
Richard Levitte [Fri, 13 May 2016 09:21:06 +0000 (11:21 +0200)]
VMS perl: Fix glob output
In some cases, perl's glob() thinks it needs to return file names with
generation numbers, such as when a file name pattern includes two
periods. Constructing other file names by simple appending to file
names with generation numbers isn't a good idea, so for the VMS case,
just peal the generation numbers if they are there.
Fortunately, this is easy, as the returned generation number delimiter
will always be a semi-colon.
David Benjamin [Sun, 6 Mar 2016 00:35:52 +0000 (19:35 -0500)]
The NewSessionTicket message is not optional.
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 message should be determined
entirely from the ServerHello without probing.
RT#4389
Reviewed-by: Emilia Käsper <emilia@openssl.org> Reviewed-by: Matt Caswell <matt@openssl.org>
Richard Levitte [Tue, 10 May 2016 12:41:19 +0000 (14:41 +0200)]
DJGPP adjustments
* Configure: Replaced -DTERMIO by -DTERMIOS in CFLAGS.
* crypto/bio/bss_dgram.c [WATT32]: Remove obsolete redefinition of
function names: sock_write, sock_read and sock_puts.
* crypto/bio/bss_sock.c [WATT32]: For Watt-32 2.2.11 sock_write,
sock_read and sock_puts are redefined to their private names so
their names must be undefined first before they can be redefined
again.
* crypto/bio/bss_file.c (file_fopen) [__DJGPP__]: Make a copy of the
passed file name and replace the leading dots in the dirname part
and the basname part of the file name, unless LFN is supported.
* e_os.h [__DJGPP__]: Undefine macro DEVRANDOM_EGD. Neither MS-DOS nor
FreeDOS provide 'egd' sockets.
New macro HAS_LFN_SUPPORT checks if underlying file system supports
long file names or not.
Include sys/un.h.
Define WATT32_NO_OLDIES.
* INSTALL.DJGPP: Update URL of WATT-32 library.
Submitted by Juan Manuel Guerrero <juan.guerrero@gmx.de>