Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Client example error - Expected and actual key signature do not match #32

Closed
diagonalfish opened this issue Jan 25, 2019 · 10 comments
Closed
Assignees

Comments

@diagonalfish
Copy link

Hello.

Following the instructions in the readme to create a certificate and key and testing out the client example, I get the following error with the latest version of the library (as of today):

$ go run examples/dial/main.go 
panic: dtls: Expected and actual key signature do not match

goroutine 1 [running]:
github.com/pions/dtls/examples/util.Check(...)
	/home/vagrant/dtls/examples/util/util.go:41
main.main()
	/home/vagrant/dtls/examples/dial/main.go:28 +0x27d
exit status 2

I'm on CentOS 7. My OpenSSL version is 1.0.2k-fips, and I'm on Go 1.11.2.

@diagonalfish
Copy link
Author

I download and compiled the latest OpenSSL (1.1.0j) and it appears to not be affected by this problem. I am not sure if this is a bug in OpenSSL or some other subtle incompatibility that was resolved in 1.1.0+.

@Sean-Der
Copy link
Member

@diagonalfish thanks for the report!

I will run 1.0.2k and check it out. Does CentOS 7 ship that by default? (just looking for the easiest way to test) vs building it myself.

I can also help debug quicker if you can spew.Dump at client_handlers.go:91 and get a pcap of all udp traffic. If you don't have time totally understand though!

clientRandom
serverRandom
expectedHash
h
c

@diagonalfish
Copy link
Author

Yes, it is the default in the current version of CentOS 7 (7.6.1810 at time of writing). The full package version is openssl-1.0.2k-16.el7.

@diagonalfish
Copy link
Author

Here is a spew of all the variables you wanted to see: https://gist.github.com/diagonalfish/ed5c393f73b5b76444ba66f5285b37f1

pcap's a bit harder because I'm communicating on localhost but here's the OpenSSL debug output which includes packet hex dumps: https://gist.github.com/diagonalfish/836cac3938221c10a4f966995b6ca927

@Sean-Der
Copy link
Member

Perfect, thank you @diagonalfish

I was able to reproduce this, and will be working on this. I will update you when I know the exact issue and then when I push a fix.

Thanks for using Pion DTLS!

@Sean-Der
Copy link
Member

Sean-Der commented Feb 1, 2019

@diagonalfish

I was able to get OpenSSL as a client and Pion DTLS working as a server with this patch

[root@b71a7b964168 dtls]# git diff
diff --git a/server_handlers.go b/server_handlers.go
index 034471e..9344608 100644
--- a/server_handlers.go
+++ b/server_handlers.go
@@ -167,9 +167,6 @@ func serverFlightHandler(c *Conn) (bool, error) {
                                                &extensionSupportedEllipticCurves{
                                                        ellipticCurves: []namedCurve{namedCurveX25519, namedCurveP256},
                                                },
-                                               &extensionUseSRTP{
-                                                       protectionProfiles: []srtpProtectionProfile{SRTP_AES128_CM_HMAC_SHA1_80},
-                                               },
                                                &extensionSupportedPointFormats{
                                                        pointFormats: []ellipticCurvePointFormat{ellipticCurvePointFormatUncompressed},
                                                },

I will get this patch into master. We should conditionally support getting/setting extensions like crypto/tls but I just defaulted them because I only was concerned about pion-WebRTC at the time.

Still working on pion DTLS as a client. This is probably the most frustrating part, it is just in hash verification, if you do this everything works. You can do this if you want to be unblocked, but definitely don't ship with this. I am sure this is just an off-by-one somewhere, but really tough to debug this part with OpenSSL.

diff --git a/client_handlers.go b/client_handlers.go
index 7cb5225..fa0fae6 100644
--- a/client_handlers.go
+++ b/client_handlers.go
@@ -86,10 +86,10 @@ func clientHandshakeHandler(c *Conn) error {
                                        return err
                                }

-                               expectedHash := valueKeySignature(clientRandom, serverRandom, h.publicKey, c.namedCurve, h.hashAlgorithm)
-                               if err := verifyKeySignature(expectedHash, h.signature, c.remoteCertificate); err != nil {
-                                       return err
-                               }
+                               // expectedHash := valueKeySignature(clientRandom, serverRandom, h.publicKey, c.namedCurve, h.hashAlgorithm)
+                               // if err := verifyKeySignature(expectedHash, h.signature, c.remoteCertificate); err != nil {
+                               //      return err
+                               // }
                        }

                case *handshakeMessageCertificateRequest:

@diagonalfish
Copy link
Author

Sorry - was out last week and didn't see this. I appreciate the work you've put into this so far. If there's anything else I can do to help let me know.

Sean-Der added a commit that referenced this issue Feb 13, 2019
Library when initially written was only used for pion-WebRTC,
where we always wanted SRTP. This puts it behind configuration
so we can start up a Client/Server without SRTP

Relates to: #32
Sean-Der added a commit that referenced this issue Feb 13, 2019
Sean-Der added a commit that referenced this issue Feb 13, 2019
Library when initially written was only used for pion-WebRTC,
where we always wanted SRTP. This puts it behind configuration
so we can start up a Client/Server without SRTP

Relates to: #32
Sean-Der added a commit that referenced this issue Feb 13, 2019
@Sean-Der
Copy link
Member

Sean-Der commented Feb 13, 2019

Hey good news @diagonalfish this is all fixed! Can you try out my branch when you get a chance? #37

I will merge into master if it all works, thanks!

@Sean-Der Sean-Der self-assigned this Feb 14, 2019
@diagonalfish
Copy link
Author

Can confirm that the issue-32 branch solves the problem. Excellent :)

Sean-Der added a commit that referenced this issue Feb 14, 2019
Library when initially written was only used for pion-WebRTC,
where we always wanted SRTP. This puts it behind configuration
so we can start up a Client/Server without SRTP

Relates to: #32
Sean-Der added a commit that referenced this issue Feb 14, 2019
Sean-Der added a commit that referenced this issue Feb 14, 2019
Library when initially written was only used for pion-WebRTC,
where we always wanted SRTP. This puts it behind configuration
so we can start up a Client/Server without SRTP

Relates to: #32
Sean-Der added a commit that referenced this issue Feb 14, 2019
@Sean-Der
Copy link
Member

Fantastic, merged!

Thanks again for the bug report. If there is every anything else I am all ears, hopefully the package is easy to use :)

Also feel free to join us in Slack! Most people are talking about pions/webrtc but always happy to help debug/discuss anything.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants