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

libsodium regression affecting dnscrypt-wrapper #191

Closed
pysiak opened this issue Sep 21, 2014 · 3 comments
Closed

libsodium regression affecting dnscrypt-wrapper #191

pysiak opened this issue Sep 21, 2014 · 3 comments

Comments

@pysiak
Copy link

pysiak commented Sep 21, 2014

Summary: Renegerating certificates with dnscrypt-wrapper works fine when linked to libsodium.so.4 (libsodium.so.4.5.0 in my case), but when linked to later versions that I tried (libsodium.so.10, 13) produces an invalid certificate. (that's at least libsodium 6, 6.1, 7, 7.1)

What I was able to find:
Whatever the change is, it is visible in dnscrypt-proxy code in src/proxy/cert.c in cert_open_bincert() where crypto_sign_ed25519_open() is called where it fails and reports "Suspicious certificate received".

I've added some debug code to dnscrypt-proxy to dump that invalid certificate in hexdump and readable ascii and spotted something indeed suspicious.

Certificates generated with dnscrypt-wrapper linked to non-working libsodium version generate repeating sequences every time (bolded parts) below:

2.dnscrypt-cert 86400 IN TXT "DNSC\000\001\000\000_
\222\023\152\180\130\147\139\030P\221\249\163\216\031\020w\155\129g\199\034r96\219\201m;E>\237\2030N\191\207\162 \190\227\192\207|\218\207\127\254\235\231,\092
G\187\148\253\014,b~\013\150\182\010_
\222\023\152\180\130\147\139\030P\221\249\163\216\031\020w\155\129g\199\034r96\219\201m;E>\237\2030N\191\207\162 \190\227\192\207|\218\207\127\254\235\231,\092
"

That is suspicious, that doesn't work, it's invalid.

When linked to libsodium 4.5 the resulting certs do not repeat, e.g.:

2.dnscrypt-cert 86400 IN TXT "DNSC\000\001\000\000\012_\183;c\204j'\150S\004\230\245i\222\026\207yi\234\132\014\015\006]\030?\028\156D\030\254\229\139\147j{\214\246X\185\149\238 B\192\137\017\219,\197Z\203\141\211U\184\190@\006H\231 \003p\198/8\171YV\014\168t\251L_\150IO+p\207\241\133\011e\240\211\196\1386\254\008oa7PYqwfzt0001T\030\214\015V\000\009\143"

Sorry, I don't have the time to investigate further, but at least this could be raised as a known issue until a fix is found:

When generating certificates for dnscrypt-wrapper, use libsodium 4.5.

  • If you haven't compiled it before, compile and install it.
  • edit dnscrypt-wrapper/Makefile and modify line 11 from:
    • BASIC_LDFLAGS = -lm -lsodium
    • to:
    • BASIC_LDFLAGS = -lm -l:libsodium.so.4
  • Compile and use the new copy to --gen-cert-file
  • Afterwards, it is OK to run dnscrypt-wrapper linked to any recent version of libsodium

This probably is quite a surprise as @jedisct1 told me on this issue shouldn't be related to libsodium versions.

@jedisct1
Copy link
Owner

Hi,
And thanks a lot for your excellent bug report.

I won't be able to look at it today. Just took a very quick glance at dnscrypt-wrapper and saw this:

  if (crypto_sign_ed25519
        (signed_cert->server_publickey,
         (unsigned long long *)&crypted_signed_data_len,
         signed_cert->server_publickey, signed_data_len,
         provider_secretkey) != 0) {
        return -1;
    }

That doesn't look quite right.
signed_cert->server_publickey is too small to store the output of this function.

@jedisct1
Copy link
Owner

But the actual bug is how overlapping regions are handled. The message shouldn't be copied after the signature is written but before. Fix is pretty straightforward. I'll commit it tonight.

@jedisct1
Copy link
Owner

Should be fixed with 51dfcfc and ad5a165

Thanks a lot for spotting this!

Repository owner locked and limited conversation to collaborators Aug 28, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants