<div dir="ltr"><div><br></div><div>Hi Jesse!</div><div><br></div><div>Thank you very much for sending me the test data. I now know why it used to work and is no longer working. Indeed it has to do with re-encoding a decode certificate however the place that is failing is not in the ssl application code but in the public_key applications code, although it is a change in the ssl application that causes the situation. Even if the root cause is that the certificate is wrongly encoded in the first place.</div><div><br></div><div>So the first patch I made should not be needed as in this case we are verifying that a possible sent ROOT cert matches the ROOT cert that we trust. And if it is the same they will have the same possible incorrect DER encoding protected by the signature and they will match. That is we have not performed an decode and then encoded it back again.</div><div><br></div><div>My PR, we will want to have to be able to tolerate such certificates (in a different scenario than yours) but we will need to add some additional commit to handle your case. So the change that made it fail is optimization effort to avoid decoding certs multiple times. Although, I realized when looking into this issue, actually instead creates a need for encoding the certs again. So before the public_key application got the certs in DER and now they get them decoded. I think the "sane" thing will be to allow public_key to also accept a list with both encoded (as received from peer) and decoded. This will avoid all extra encode and decode needs, and allow for tolerance of some decoding errors in certificates that can be "worked around". </div><div><br></div><div>Always these interop tradeoffs, well this will be best for performance also. I will add it to my PR. </div><div><br></div><div>Regards Ingela Erlang/OTP team - Ericsson AB</div><div><br></div><div><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">Den ons 22 sep. 2021 kl 00:21 skrev Jesse Bickel - NOAA Affiliate <<a href="mailto:jesse.bickel@noaa.gov">jesse.bickel@noaa.gov</a>>:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">I apologize for the delayed response. Thank you all for working on this issue even though it may be more of a gnutls issue.<br>
<br>
Kenneth Lundin wrote:<br>
> In this case it is the encoding of KeyUsage which is the problem, it is questionable if it is correctly encoded by GnuTLS.<br>
> <br>
> It is declared like this:<br>
> KeyUsage ::= BIT STRING {<br>
> digitalSignature (0),<br>
> nonRepudiation (1),<br>
> keyEncipherment (2),<br>
> dataEncipherment (3),<br>
> keyAgreement (4),<br>
> keyCertSign (5),<br>
> cRLSign (6),<br>
> encipherOnly (7),<br>
> decipherOnly (8) }<br>
> <br>
> And encoded like this by GnuTLS (as I understand it from Jesses initial description)<br>
> <<3,3,7,16#A0,0>> with the interpretation of bytes and bits like this:<br>
> Byte 0: Value 3: The UNIVERSAL tag for BIT STRING<br>
> Byte 1: Value 3: Length = 3<br>
> Byte 2: Value 7: Number of unused bits in last octet = 7<br>
> Byte 3: Value 16#A0: (2#10100000) The first 8 bits of the bitstring with bit (0) digitalSignature and bit (2) keyEncipherment set<br>
> Byte 4: Value 0: The first bit is zero and the 7 remaining bits are unused<br>
> <br>
> If we read the standard docs below I interpret this as 1) we have a BIT STRING with a NamedBitList (22.7 in X.680 applies) and then we apply 11.2.1 and<br>
> especially 11.2.2 from X.690 which says that trailing zero bits should be removed before encoding which I don't think has been done in the above case.<br>
> <br>
> So I think GnuTLS might be wrong here in that it does not encode according to DER.<br>
<br>
Yes, this is consistent with what I see. Gnutls keeps the trailing zero bits while openssl strips the trailing zero bits. But both tolerate each others' differing representations (for better or worse).<br>
<br>
> The encoding that Erlang/OTP is using which I claim is according to DER is like this:<br>
> <<3,2,5,16#A0>> with the interpretation of bytes and bits like this:<br>
> Byte 0: Value 3: The UNIVERSAL tag for BIT STRING<br>
> Byte 1: Value 2: Length = 2<br>
> Byte 2: Value 5: Number of unused bits in last octet = 5<br>
> Byte 3: Value 16#A0: (2#10100000) The first 3 bits of the bitstring with bit (0) digitalSignature and bit (2) keyEncipherment set<br>
> , the remaining 5 bits are unused. <br>
> This encoding is removing all trailing zero bits which make the encoding use as few bytes as possible<br>
> <br>
> From the standard in X.690 and X.680 we have :<br>
> <br>
> X.690<br>
> 11.2 Unused bits <br>
> 11.2.1 Each unused bit in the final octet of the encoding of a bit string value shall be set to zero. <br>
> 11.2.2 Where Rec. ITU-T X.680 | ISO/IEC 8824-1, 22.7, applies, the bitstring shall have all trailing 0 bits removed before <br>
> it is encoded. <br>
> NOTE 1 – In the case where a size constraint has been applied, the abstract value delivered by a decoder to the application will be on e <br>
> of those satisfying the size constraint and differing from the transmitted value only in the number of trailing 0 bits. <br>
> NOTE 2 – If a bitstring value has no 1 bits, then an encoder shall encode the value with a length of 1 and an initial octet set to 0 <br>
> <br>
> X.680<br>
> 22.7 When a "NamedBitList" is used in defining a bitstring type ASN.1 encoding rules are free to add (or remove) <br>
> arbitrarily any trailing 0 bits to (or from) values that are being encoded or decoded. Application designers should ISO/IEC 8824-1:2021 (E) <br>
> Rec. ITU-T X.680 (02/2021) 43 <br>
> therefore ensure that different semantics are not associated with such values which differ only in the number of trailing <br>
> 0 bits.<br>
> <br>
> This is just one case where the intention with DER does not hold since many certificates created also with OpenSSL does not follow the standard (The ASN.1<br>
> specs) for example when it comes to the encoding of CountryName (there are a few other data fields more which are problematic). The CountryName is<br>
> sometimes encoded as Printable STRING and sometimes as UTF8 STRING and the length constraint of 2<br>
> is also violated in some occasions. As the UNIVERSAL Tag for these types differ the encoded bytes will be different.<br>
> The Erlang/OTP SSL implementation have been forced to accept those "non confirming to standard" encoding because otherwise the interoperability with other<br>
> servers and clients would have been to limited.<br>
<br>
I am not as familiar with interpreting the x.680/x.690 documents but I think you make a strong case. It would make sense to have exactly one correct encoding of a certificate such that one always gets exactly the same signature, for example.<br>
<br>
> The solution is to never trust that a certificate which is decoded up to its internal Erlang representation and then encoded back again will result in<br>
> exactly the same sequence of bytes as in the original.<br>
> <br>
> /Kenneth<br>
><br>
> Bram Verburg <<a href="mailto:bram.verburg@voltone.net" target="_blank">bram.verburg@voltone.net</a>> wrote:<br>
> <br>
> Hi Ingela,<br>
> <br>
> I was looking into this too, and I noticed that this particular leaf certificate, once decoded as an OTPCertificate record, does not encode back to<br>
> the same DER value (which is likely due to the unusual representation of the KeyUsage extension in the original file):<br>
> <br>
> 1> {ok, PEM} = file:read_file("cert.pem"). <br>
> {ok,<<"-----BEGIN CERTIFICATE-----\nMIIEfDCCAuSgAwIBAgIUdcWg+bxobl7OWSOjbNxyaXTnqx8wDQYJKoZIhvcNAQEL\nBQAwMTEXMBUGA1U"...>>}<br>
> 2> DER = element(2, hd(public_key:pem_decode(PEM))).<br>
> <<48,130,4,124,48,130,2,228,160,3,2,1,2,2,20,117,197,160,<br>
> 249,188,104,110,94,206,89,35,163,108,220,...>><br>
> 3> DER = public_key:pkix_encode('OTPCertificate', public_key:pkix_decode_cert(DER, otp), otp).<br>
> ** exception error: no match of right hand side value <<48,130,4,123,48,130,2,227,160,3,2,1,2,2,20,117,197,160,<br>
> 249,188,104,110,94,206,89,35,163,108,220,...>><br>
> <br>
> (That would presumably mean the original DER representation is not really proper DER, as I believe ASN.1's DER encoding is supposed to always produce<br>
> the same (canonical) byte sequence, in order to allow for reliable signature verification...)<br>
> <br>
> I haven't quite pinned it down, but this commit <a href="https://github.com/erlang/otp/pull/4941/commits/80f9b71323c9c5a3bc111fcbc5c6c5ee497e27a6" rel="noreferrer" target="_blank">https://github.com/erlang/otp/pull/4941/commits/80f9b71323c9c5a3bc111fcbc5c6c5ee497e27a6</a> that was<br>
> included in 24.0.4 seems to sometimes reconstruct a DER representation from an OTPCertificate record, and if that DER value is passed to<br>
> public_key:pkix_verify/2 instead of the original DER representation, the signature won't match and ssl will throw the reported error.<br>
> <br>
> Hope that might help,<br>
> <br>
> Bram<br>
> <br>
><br>
> Ingela Andin <<a href="mailto:ingela.andin@gmail.com" target="_blank">ingela.andin@gmail.com</a>> wrote:<br>
> <br>
> Hello again! <br>
> <br>
> I thought of a possible cause of your strange problem. If the ROOT-cert is sent as part of the chain (optional in the protocol). We must check<br>
> that it is a certificate that we trust, part of our "trust store". This was done by a match of the DER-version of the cert and we should match<br>
> the decode structure instead. <br>
> <br>
> In such cases the following patch ought to solve your problem. Can you please test it? It is based on the latest maint!<br>
<br>
I tried this to no avail but we see there is a later patch at <a href="https://github.com/erlang/otp/pull/5222" rel="noreferrer" target="_blank">https://github.com/erlang/otp/pull/5222</a> which I assume is related. When I tried this patch a few minutes ago, I still get this:<br>
** exception error: no match of right hand side value {error,{tls_alert,{bad_certificate,"TLS client: In state certify at ssl_handshake.erl:1989 generated CLIENT ALERT: Fatal - Bad Certificate\n"}}}<br>
<br>
Ingela, I directly emailed you the dummy/throwaway/test key corresponding to the leaf certificate that I posted earlier and also the commands my teammate found to help reproduce the issue. I hope this helps, thank you for your help too.<br>
<br>
Best,<br>
<br>
Jesse Bickel<br>
<br>
-- <br>
Contractor, ERT, Inc.<br>
Federal Affiliation: NWC/OWP/NOAA/DOC</blockquote></div></div>