Re: 8245686: Ed25519 and Ed448 present in handshake messages
Thanks for the comment.. I moved the code up toward the top. Tony On 6/9/20 4:04 PM, Xuelei Fan wrote: A simple fix like this looks good to me. I may check this first, before the EC available and signature checking. Xuelei On 6/9/2020 3:12 PM, Anthony Scarpino wrote: Hi, I need a code review of this very simple change for a situation that I'm not sure is a problem in the real world. The original TLS 1.3 putback added EdDSA to the TLS signature extensions enumeration before there was an EdDSA JCE implementation or JSSE support. Without an implementation, a signature checks would not include EdDSA for TLS extensions, signature_algorithms and signature_algorithm_cert. Now with JCE EdDSA support, the signature check adds EdDSA to the extension, despite JSSE not having support yet (JDK-8166596). This causes a signature scheme authentication failure, and JSSE moves onto the next certificate provided. The only time this is a problem is if EdDSA is the only cert provided. I'm not sure how realistic it is for one certificate to be provided. If someone knows multiple certificates are always available, I'm happy to not make this change. The fix is a simple check in the constructor to set the curves unavailable after the signature check. This code can be deleted when JDK-8166596 is fixed in jdk16. I had thought about commenting out the enums, but then the logging code would not know what the id's were when other clients and servers passed them to JSSE. https://cr.openjdk.java.net/~ascarpino/8245686/webrev/ Tony
Re: 8245686: Ed25519 and Ed448 present in handshake messages
Thanks for catching that. Tony On 6/9/20 5:23 PM, Bradford Wetmore wrote: Update the year, but otherwise looks good. Brad On 6/9/2020 4:04 PM, Xuelei Fan wrote: A simple fix like this looks good to me. I may check this first, before the EC available and signature checking. Xuelei On 6/9/2020 3:12 PM, Anthony Scarpino wrote: Hi, I need a code review of this very simple change for a situation that I'm not sure is a problem in the real world. The original TLS 1.3 putback added EdDSA to the TLS signature extensions enumeration before there was an EdDSA JCE implementation or JSSE support. Without an implementation, a signature checks would not include EdDSA for TLS extensions, signature_algorithms and signature_algorithm_cert. Now with JCE EdDSA support, the signature check adds EdDSA to the extension, despite JSSE not having support yet (JDK-8166596). This causes a signature scheme authentication failure, and JSSE moves onto the next certificate provided. The only time this is a problem is if EdDSA is the only cert provided. I'm not sure how realistic it is for one certificate to be provided. If someone knows multiple certificates are always available, I'm happy to not make this change. The fix is a simple check in the constructor to set the curves unavailable after the signature check. This code can be deleted when JDK-8166596 is fixed in jdk16. I had thought about commenting out the enums, but then the logging code would not know what the id's were when other clients and servers passed them to JSSE. https://cr.openjdk.java.net/~ascarpino/8245686/webrev/ Tony
Re: 8245686: Ed25519 and Ed448 present in handshake messages
Update the year, but otherwise looks good. Brad On 6/9/2020 4:04 PM, Xuelei Fan wrote: A simple fix like this looks good to me. I may check this first, before the EC available and signature checking. Xuelei On 6/9/2020 3:12 PM, Anthony Scarpino wrote: Hi, I need a code review of this very simple change for a situation that I'm not sure is a problem in the real world. The original TLS 1.3 putback added EdDSA to the TLS signature extensions enumeration before there was an EdDSA JCE implementation or JSSE support. Without an implementation, a signature checks would not include EdDSA for TLS extensions, signature_algorithms and signature_algorithm_cert. Now with JCE EdDSA support, the signature check adds EdDSA to the extension, despite JSSE not having support yet (JDK-8166596). This causes a signature scheme authentication failure, and JSSE moves onto the next certificate provided. The only time this is a problem is if EdDSA is the only cert provided. I'm not sure how realistic it is for one certificate to be provided. If someone knows multiple certificates are always available, I'm happy to not make this change. The fix is a simple check in the constructor to set the curves unavailable after the signature check. This code can be deleted when JDK-8166596 is fixed in jdk16. I had thought about commenting out the enums, but then the logging code would not know what the id's were when other clients and servers passed them to JSSE. https://cr.openjdk.java.net/~ascarpino/8245686/webrev/ Tony
Re: 8245686: Ed25519 and Ed448 present in handshake messages
A simple fix like this looks good to me. I may check this first, before the EC available and signature checking. Xuelei On 6/9/2020 3:12 PM, Anthony Scarpino wrote: Hi, I need a code review of this very simple change for a situation that I'm not sure is a problem in the real world. The original TLS 1.3 putback added EdDSA to the TLS signature extensions enumeration before there was an EdDSA JCE implementation or JSSE support. Without an implementation, a signature checks would not include EdDSA for TLS extensions, signature_algorithms and signature_algorithm_cert. Now with JCE EdDSA support, the signature check adds EdDSA to the extension, despite JSSE not having support yet (JDK-8166596). This causes a signature scheme authentication failure, and JSSE moves onto the next certificate provided. The only time this is a problem is if EdDSA is the only cert provided. I'm not sure how realistic it is for one certificate to be provided. If someone knows multiple certificates are always available, I'm happy to not make this change. The fix is a simple check in the constructor to set the curves unavailable after the signature check. This code can be deleted when JDK-8166596 is fixed in jdk16. I had thought about commenting out the enums, but then the logging code would not know what the id's were when other clients and servers passed them to JSSE. https://cr.openjdk.java.net/~ascarpino/8245686/webrev/ Tony
Re: 8245686: Ed25519 and Ed448 present in handshake messages
Looks fine to me. --Jamil On 6/9/2020 3:12 PM, Anthony Scarpino wrote: Hi, I need a code review of this very simple change for a situation that I'm not sure is a problem in the real world. The original TLS 1.3 putback added EdDSA to the TLS signature extensions enumeration before there was an EdDSA JCE implementation or JSSE support. Without an implementation, a signature checks would not include EdDSA for TLS extensions, signature_algorithms and signature_algorithm_cert. Now with JCE EdDSA support, the signature check adds EdDSA to the extension, despite JSSE not having support yet (JDK-8166596). This causes a signature scheme authentication failure, and JSSE moves onto the next certificate provided. The only time this is a problem is if EdDSA is the only cert provided. I'm not sure how realistic it is for one certificate to be provided. If someone knows multiple certificates are always available, I'm happy to not make this change. The fix is a simple check in the constructor to set the curves unavailable after the signature check. This code can be deleted when JDK-8166596 is fixed in jdk16. I had thought about commenting out the enums, but then the logging code would not know what the id's were when other clients and servers passed them to JSSE. https://cr.openjdk.java.net/~ascarpino/8245686/webrev/ Tony
8245686: Ed25519 and Ed448 present in handshake messages
Hi, I need a code review of this very simple change for a situation that I'm not sure is a problem in the real world. The original TLS 1.3 putback added EdDSA to the TLS signature extensions enumeration before there was an EdDSA JCE implementation or JSSE support. Without an implementation, a signature checks would not include EdDSA for TLS extensions, signature_algorithms and signature_algorithm_cert. Now with JCE EdDSA support, the signature check adds EdDSA to the extension, despite JSSE not having support yet (JDK-8166596). This causes a signature scheme authentication failure, and JSSE moves onto the next certificate provided. The only time this is a problem is if EdDSA is the only cert provided. I'm not sure how realistic it is for one certificate to be provided. If someone knows multiple certificates are always available, I'm happy to not make this change. The fix is a simple check in the constructor to set the curves unavailable after the signature check. This code can be deleted when JDK-8166596 is fixed in jdk16. I had thought about commenting out the enums, but then the logging code would not know what the id's were when other clients and servers passed them to JSSE. https://cr.openjdk.java.net/~ascarpino/8245686/webrev/ Tony
