Re: [TLS] Newcomer’s Implementation Experience of TLS 1.3 Draft 16

2016-10-14 Thread Ilari Liusvaara
On Fri, Oct 14, 2016 at 05:10:01PM +0200, Hubert Kario wrote:
> On Thursday, 13 October 2016 23:33:19 CEST Ilari Liusvaara wrote:
> > Ok, dumped the handshake using wireshark. Wireshark seems to think
> > the SNI with two lengths is perfectly sane.
> 
> that's because wireshark doesn't perform length checks on many fields of TLS
> 
> There are both valid messages rejected by wireshark (fragmented over multiple 
> records) and invalid messages accepted by wireshark.

Actually, AFAICT, the message was encoded like it should (one length
from the outer list length and one from the name length, as specified
by the TLS standard).


-Ilari

___
TLS mailing list
TLS@ietf.org
https://www.ietf.org/mailman/listinfo/tls


Re: [TLS] Newcomer’s Implementation Experience of TLS 1.3 Draft 16

2016-10-14 Thread Hubert Kario
On Thursday, 13 October 2016 23:33:19 CEST Ilari Liusvaara wrote:
> Ok, dumped the handshake using wireshark. Wireshark seems to think
> the SNI with two lengths is perfectly sane.

that's because wireshark doesn't perform length checks on many fields of TLS

There are both valid messages rejected by wireshark (fragmented over multiple 
records) and invalid messages accepted by wireshark.

-- 
Regards,
Hubert Kario
Senior Quality Engineer, QE BaseOS Security team
Web: www.cz.redhat.com
Red Hat Czech s.r.o., Purkyňova 99/71, 612 45, Brno, Czech Republic

signature.asc
Description: This is a digitally signed message part.
___
TLS mailing list
TLS@ietf.org
https://www.ietf.org/mailman/listinfo/tls


Re: [TLS] Newcomer’s Implementation Experience of TLS 1.3 Draft 16

2016-10-13 Thread Kazuho Oku
Hi Ilari,

Thank you for trying picotls, and thank you very much for notifying me
of the issues you found.

I have fixed three issues you reported (i.e. SNI decode error, EC
group check error, PKCS not included in Signature Algorithms).

Regarding the crash, is your implementation available to public so
that I could use it for debugging?

Thank you in advance.


2016-10-14 5:33 GMT+09:00 Ilari Liusvaara :
> On Thu, Oct 13, 2016 at 12:18:03PM +0300, Ilari Liusvaara wrote:
>> On Thu, Oct 13, 2016 at 03:17:32PM +0900, Kazuho Oku wrote:
>> > TLDR: the spec. was clear and easy to implement, but some test vectors
>> > and clarification on what constitutes a Handshake Context would have
>> > helped.
>> >
>> > FWIW, please let me share my experience of implementing TLS 1.3.
>> >
>> > This month, I have written a TLS 1.3 implementation (named picotls,
>> > available at https://github.com/h2o/picotls) based on draft 16 from
>> > scratch.
>>
>> Tried interop versus my own implementation (with my implementation
>> as client).
>>
>> Didn't work... I traced the blowup to client_hello_decode_server_name():
>>
>> The sent contents of the SNI extension is:
>>
>> 00 0B 00 00 08 "h2o.test"
>>
>> Which AFAICT is a server name list of 11 bytes, containing entry of
>> type 0 (host_name), with length 8 and name "h2o.test".
>
> Further testing:
>
> My implementation vs. picotls:
> --
>
> Ok, dumped the handshake using wireshark. Wireshark seems to think
> the SNI with two lengths is perfectly sane.
>
> I modified the picotls code to read the extra length.
>
> Then I hit another issue: eckey_is_on_group seems to be busted: It
> presumably should use == instead of !=, since it is presumably
> supposed to return 1 if groups match, since it is used to check
> if the key is P-256.
>
>
> Couple of bugs in my code later, I could get successful handshake
> with my code as client and picotls as server. Yay.
>
>
> picotls vs my implementation:
> -
>
> Trying picotls as client and my code as server first hit the issue that
> picotls doesn't send RSA-PKCS-V1.5 as supported, causing my
> implementation to fail with no suitable certificate (it requires
> algorithms for the chain too, not just the end key).
>
> After defeating that check, the handshake got further, but something
> in server's messages (ServerHello, EncryptedExtensions, Certificate,
> CertificateVerify, Finished) made picotls crash (heap corruption as
> evidenced by the malloc() error). Haven't debugged that error yet.
>
>
>
> -Ilari



-- 
Kazuho Oku

___
TLS mailing list
TLS@ietf.org
https://www.ietf.org/mailman/listinfo/tls


Re: [TLS] Newcomer’s Implementation Experience of TLS 1.3 Draft 16

2016-10-13 Thread Kazuho Oku
Hi Martin,

2016-10-13 16:07 GMT+09:00 Martin Thomson :
> Thanks Kazuho!
>
> Experiences like your own are critical at this stage. It is encouraging to
> see that there were so few problems.
>
> As for the key schedule, EKR and I have discussed taking a dump from one of
> our many test cases and putting that in a draft, including private keys and
> all the intermediate values. It might get a bit long, so we were thinking of
> doing a separate companion document. Would that help you?

Yes. That would have helped.

Especially it would be great if there was an example of generating
AEAD key and IV from the handshake messages.

The logic (e.g. HKDF-Expand-Label and Derive-Secret defined in section
7.1) and the parameters that the logic takes as input are defined in
different sections of the draft. For example, you have to look at
section 2 and 7.3 to find out the correct parameters.

So I had to concentrate on getting all them together to form an
interoperable code. I won't say it was hard, but having a test vector
that goes though the process step by step would have helped a lot.

> We haven't done anything yet because the draft has been changing quite a
> bit, but I believe that the changes are now mostly done.
>
> I am happy to take ideas on what configurations to trace. The only thing NSS
> doesn't support right now is KeyUpdate (coming soon after -17 I hope).
>
> Did you want to add your implementation to the wiki?
> https://github.com/tlswg/tls13-spec/wiki/Implementations  and you would be
> most welcome to n join the next hackathon in Seoul.

Thank you for the offer. I added my implementation to the wiki.

>
>
> On 13 Oct 2016 5:17 PM, "Kazuho Oku"  wrote:
>>
>> TLDR: the spec. was clear and easy to implement, but some test vectors
>> and clarification on what constitutes a Handshake Context would have
>> helped.
>>
>> FWIW, please let me share my experience of implementing TLS 1.3.
>>
>> This month, I have written a TLS 1.3 implementation (named picotls,
>> available at https://github.com/h2o/picotls) based on draft 16 from
>> scratch.
>>
>> This has been my first time to implement TLS.
>>
>> I wrote my implementation by going through the draft. While writing my
>> code, I did not refer to other implementations except for looking into
>> OpenSSL to see if there was an optimized path for implementing AES-GCM
>> for TLS 1.3 (which turned out to not exist in 1.0.2; it has been
>> introduced in OpenSSL 1.1.0).
>>
>> After my own implementation of server and client started talking to
>> each other, I started to test interoperability by using Firefox
>> Nightly.
>>
>> I had to fix five issues before picotls started talking with Firefox,
>> which took about half a day of work (some errors are not strictly
>> related to TLS).
>>
>> Commit 479f25f, ddd50b7 fixed errors in AEAD construction.
>> Commit 5cb99c5 fixed an error in RSA signing.
>> Commit 2d20c86 fixed a mis-optimization in my implementation of
>> Derive-Secret.
>> Commit 5780bfc fixed a silly mistake in generating a CertificateVerify.
>>
>> Details of each commit can be found at
>> https://github.com/h2o/picotls/commits/master
>>
>> It was possible to fix the errors by observing the fatal alert sent by
>> Firefox and going back to the Internet Draft. But it would have been
>> even more easier if the draft included test vectors especially for the
>> cryptographic operations.
>>
>> Aside from the bugs I fixed, it seemed to me that the draft was vague
>> on whether if msg_type and length of Handshake should be considered as
>> part of the Handshake Context (please forgive me if I missed somewhere
>> that mentions it).
>>
>> In section 4.4, the draft states that, quote: a Handshake Context
>> based on the hash of the handshake messages. This text seems to imply
>> that msg_type and length should be considered part of the Context, but
>> I could not find a formal definition of what a “handshake message” is.
>>
>> OTOH, other parts of the Draft seem to refer to Handshake Context as a
>> list of Handshake bodies. For example, the table in section 4.4 states
>> that the Handshake Context for 0-RTT mode is ClientHello. I think this
>> could be interpreted that the Context is not expected to include
>> msg_type and length, since ClientHello is a structure that is used as
>> a message body.
>>
>> So even though my interpretation (the former of the two) was correct
>> in sense that it matched that of Firefox, I needed to check if the
>> browser was interpreting the draft in the latter way, while I tried to
>> fix the AEAD error.
>>
>> The other two issues I had are my confusion on why a Handshake Context
>> may contain Certificate and CertificateVerify after ServerFinished
>> (answered by Illari at
>> https://www.ietf.org/mail-archive/web/tls/current/msg21476.html), and
>> a mistake in encoding draft 16 as 0x16
>> (https://github.com/tlswg/tls13-spec/issues/682).
>>
>> To summarize, the draft was clear enough for a newcomer to implement
>> the specification, but

Re: [TLS] Newcomer’s Implementation Experience of TLS 1.3 Draft 16

2016-10-13 Thread Ilari Liusvaara
On Thu, Oct 13, 2016 at 12:18:03PM +0300, Ilari Liusvaara wrote:
> On Thu, Oct 13, 2016 at 03:17:32PM +0900, Kazuho Oku wrote:
> > TLDR: the spec. was clear and easy to implement, but some test vectors
> > and clarification on what constitutes a Handshake Context would have
> > helped.
> > 
> > FWIW, please let me share my experience of implementing TLS 1.3.
> > 
> > This month, I have written a TLS 1.3 implementation (named picotls,
> > available at https://github.com/h2o/picotls) based on draft 16 from
> > scratch.
> 
> Tried interop versus my own implementation (with my implementation
> as client).
> 
> Didn't work... I traced the blowup to client_hello_decode_server_name():
> 
> The sent contents of the SNI extension is:
> 
> 00 0B 00 00 08 "h2o.test"
> 
> Which AFAICT is a server name list of 11 bytes, containing entry of
> type 0 (host_name), with length 8 and name "h2o.test".

Further testing:

My implementation vs. picotls:
--

Ok, dumped the handshake using wireshark. Wireshark seems to think
the SNI with two lengths is perfectly sane.

I modified the picotls code to read the extra length.

Then I hit another issue: eckey_is_on_group seems to be busted: It
presumably should use == instead of !=, since it is presumably
supposed to return 1 if groups match, since it is used to check
if the key is P-256.


Couple of bugs in my code later, I could get successful handshake
with my code as client and picotls as server. Yay.


picotls vs my implementation:
-

Trying picotls as client and my code as server first hit the issue that
picotls doesn't send RSA-PKCS-V1.5 as supported, causing my
implementation to fail with no suitable certificate (it requires
algorithms for the chain too, not just the end key).

After defeating that check, the handshake got further, but something
in server's messages (ServerHello, EncryptedExtensions, Certificate,
CertificateVerify, Finished) made picotls crash (heap corruption as
evidenced by the malloc() error). Haven't debugged that error yet.



-Ilari

___
TLS mailing list
TLS@ietf.org
https://www.ietf.org/mailman/listinfo/tls


Re: [TLS] Newcomer’s Implementation Experience of TLS 1.3 Draft 16

2016-10-13 Thread Eric Rescorla
Kazuho,

Thanks for the feedback. This is very helpful.

On Wed, Oct 12, 2016 at 11:17 PM, Kazuho Oku  wrote:
>
> I wrote my implementation by going through the draft. While writing my
> code, I did not refer to other implementations except for looking into
> OpenSSL to see if there was an optimized path for implementing AES-GCM
> for TLS 1.3 (which turned out to not exist in 1.0.2; it has been
> introduced in OpenSSL 1.1.0).
>
> After my own implementation of server and client started talking to
> each other, I started to test interoperability by using Firefox
> Nightly.
>
> I had to fix five issues before picotls started talking with Firefox,
> which took about half a day of work (some errors are not strictly
> related to TLS).
>
> Commit 479f25f, ddd50b7 fixed errors in AEAD construction.
> Commit 5cb99c5 fixed an error in RSA signing.
> Commit 2d20c86 fixed a mis-optimization in my implementation of
> Derive-Secret.
> Commit 5780bfc fixed a silly mistake in generating a CertificateVerify.
>
> Details of each commit can be found at
> https://github.com/h2o/picotls/commits/master
>
> It was possible to fix the errors by observing the fatal alert sent by
> Firefox and going back to the Internet Draft. But it would have been
> even more easier if the draft included test vectors especially for the
> cryptographic operations.
>

We have heard this a number of times. We'll see what we can do about
producing some
vectors from a working implementation.


Aside from the bugs I fixed, it seemed to me that the draft was vague
> on whether if msg_type and length of Handshake should be considered as
> part of the Handshake Context (please forgive me if I missed somewhere
> that mentions it).
>
> In section 4.4, the draft states that, quote: a Handshake Context
> based on the hash of the handshake messages. This text seems to imply
> that msg_type and length should be considered part of the Context, but
> I could not find a formal definition of what a “handshake message” is.
>

Ouch. Yes, I see what you mean here. There used to be some text that made
this clear, but I think it got lost in an edit. I have filed an issue to
fix this
(https://github.com/tlswg/tls13-spec/issues/688) and will try to get it in
by -17.


The other two issues I had are my confusion on why a Handshake Context
> may contain Certificate and CertificateVerify after ServerFinished
> (answered by Illari at
> https://www.ietf.org/mail-archive/web/tls/current/msg21476.html), and
>

It sounds like test vectors would help here.



> a mistake in encoding draft 16 as 0x16
> (https://github.com/tlswg/tls13-spec/issues/682).
>

I have clarified this in in:
https://github.com/tlswg/tls13-spec/commit/0353994e038cfbf15becc68c412644789d2e3009

Thanks for the bug report!


Thank you very much for the great draft, and providing answers to my
> issues. I am looking forward to seeing it formalized.


Thank you very much for your input. It's great to see people doing
implementations from
the specification and having success!

Best,
-Ekr


>
> --
> Kazuho Oku
>
> ___
> TLS mailing list
> TLS@ietf.org
> https://www.ietf.org/mailman/listinfo/tls
>
___
TLS mailing list
TLS@ietf.org
https://www.ietf.org/mailman/listinfo/tls


Re: [TLS] Newcomer’s Implementation Experience of TLS 1.3 Draft 16

2016-10-13 Thread Ilari Liusvaara
On Thu, Oct 13, 2016 at 03:17:32PM +0900, Kazuho Oku wrote:
> TLDR: the spec. was clear and easy to implement, but some test vectors
> and clarification on what constitutes a Handshake Context would have
> helped.
> 
> FWIW, please let me share my experience of implementing TLS 1.3.
> 
> This month, I have written a TLS 1.3 implementation (named picotls,
> available at https://github.com/h2o/picotls) based on draft 16 from
> scratch.

Tried interop versus my own implementation (with my implementation
as client).

Didn't work... I traced the blowup to client_hello_decode_server_name():

The sent contents of the SNI extension is:

00 0B 00 00 08 "h2o.test"

Which AFAICT is a server name list of 11 bytes, containing entry of
type 0 (host_name), with length 8 and name "h2o.test".

picotls seems to interpret the first byte as the type (happens to
be 0, which is host_name), and then interprets 0B 00 as the
length of the hostname. Which of course blows up as the length
is way too big (over 2kB).


Also, my implementation does not show any alert being received
from the peer (as result of this failure), just connection being
closed (internally, picotls returns -50, which I think is
decode_error)?


-Ilari

___
TLS mailing list
TLS@ietf.org
https://www.ietf.org/mailman/listinfo/tls


Re: [TLS] Newcomer’s Implementation Experience of TLS 1.3 Draft 16

2016-10-13 Thread Martin Thomson
Thanks Kazuho!

Experiences like your own are critical at this stage. It is encouraging to
see that there were so few problems.

As for the key schedule, EKR and I have discussed taking a dump from one of
our many test cases and putting that in a draft, including private keys and
all the intermediate values. It might get a bit long, so we were thinking
of doing a separate companion document. Would that help you?

We haven't done anything yet because the draft has been changing quite a
bit, but I believe that the changes are now mostly done.

I am happy to take ideas on what configurations to trace. The only thing
NSS doesn't support right now is KeyUpdate (coming soon after -17 I hope).

Did you want to add your implementation to the wiki?
https://github.com/tlswg/tls13-spec/wiki/Implementations  and you would be
most welcome to n join the next hackathon in Seoul.

On 13 Oct 2016 5:17 PM, "Kazuho Oku"  wrote:

> TLDR: the spec. was clear and easy to implement, but some test vectors
> and clarification on what constitutes a Handshake Context would have
> helped.
>
> FWIW, please let me share my experience of implementing TLS 1.3.
>
> This month, I have written a TLS 1.3 implementation (named picotls,
> available at https://github.com/h2o/picotls) based on draft 16 from
> scratch.
>
> This has been my first time to implement TLS.
>
> I wrote my implementation by going through the draft. While writing my
> code, I did not refer to other implementations except for looking into
> OpenSSL to see if there was an optimized path for implementing AES-GCM
> for TLS 1.3 (which turned out to not exist in 1.0.2; it has been
> introduced in OpenSSL 1.1.0).
>
> After my own implementation of server and client started talking to
> each other, I started to test interoperability by using Firefox
> Nightly.
>
> I had to fix five issues before picotls started talking with Firefox,
> which took about half a day of work (some errors are not strictly
> related to TLS).
>
> Commit 479f25f, ddd50b7 fixed errors in AEAD construction.
> Commit 5cb99c5 fixed an error in RSA signing.
> Commit 2d20c86 fixed a mis-optimization in my implementation of
> Derive-Secret.
> Commit 5780bfc fixed a silly mistake in generating a CertificateVerify.
>
> Details of each commit can be found at
> https://github.com/h2o/picotls/commits/master
>
> It was possible to fix the errors by observing the fatal alert sent by
> Firefox and going back to the Internet Draft. But it would have been
> even more easier if the draft included test vectors especially for the
> cryptographic operations.
>
> Aside from the bugs I fixed, it seemed to me that the draft was vague
> on whether if msg_type and length of Handshake should be considered as
> part of the Handshake Context (please forgive me if I missed somewhere
> that mentions it).
>
> In section 4.4, the draft states that, quote: a Handshake Context
> based on the hash of the handshake messages. This text seems to imply
> that msg_type and length should be considered part of the Context, but
> I could not find a formal definition of what a “handshake message” is.
>
> OTOH, other parts of the Draft seem to refer to Handshake Context as a
> list of Handshake bodies. For example, the table in section 4.4 states
> that the Handshake Context for 0-RTT mode is ClientHello. I think this
> could be interpreted that the Context is not expected to include
> msg_type and length, since ClientHello is a structure that is used as
> a message body.
>
> So even though my interpretation (the former of the two) was correct
> in sense that it matched that of Firefox, I needed to check if the
> browser was interpreting the draft in the latter way, while I tried to
> fix the AEAD error.
>
> The other two issues I had are my confusion on why a Handshake Context
> may contain Certificate and CertificateVerify after ServerFinished
> (answered by Illari at
> https://www.ietf.org/mail-archive/web/tls/current/msg21476.html), and
> a mistake in encoding draft 16 as 0x16
> (https://github.com/tlswg/tls13-spec/issues/682).
>
> To summarize, the draft was clear enough for a newcomer to implement
> the specification, but I think some test vectors and clarification on
> what consists a Handshake Context might help others trying to
> implement the protocol.
>
> Thank you very much for the great draft, and providing answers to my
> issues. I am looking forward to seeing it formalized.
>
> --
> Kazuho Oku
>
> ___
> TLS mailing list
> TLS@ietf.org
> https://www.ietf.org/mailman/listinfo/tls
>
___
TLS mailing list
TLS@ietf.org
https://www.ietf.org/mailman/listinfo/tls