Opened 9 years ago

Closed 9 years ago

Last modified 9 years ago

#951 closed defect (fixed)

SHA224 unsupported algorithm unit test failure

Reported by: jreed Owned by: jelte
Priority: medium Milestone: Sprint-20110531
Component: Unclassified Version:
Keywords: Cc:
CVSS Scoring: Parent Tickets:
Sensitive: no Defect Severity: N/A
Sub-Project: DNS Feature Depending on Ticket:
Estimated Difficulty: 2.0 Add Hours to Ticket: 0
Total Hours: 0 Internal?: no

Description

[ RUN      ] CryptoLinkTest.HMAC_SHA256_RFC4231_SIGN
[       OK ] CryptoLinkTest.HMAC_SHA256_RFC4231_SIGN
[ RUN      ] CryptoLinkTest.HMAC_SHA224_RFC4231_SIGN
terminate called after throwing an instance of 'isc::cryptolink::UnsupportedAlgorithm'
  what():  own hash algorithm: 
/bin/sh: line 5: 29375 Aborted                 ${dir}$tst
FAIL: run_unittests

This system has: Botan 1.8.7

Subtickets

Change History (12)

comment:1 follow-up: Changed 9 years ago by jinmei

I confirmed SHA224 was the only test that failed and temporarily
disabled it. I'm giving it to Feng, who implemented this test first.
The failed test can be reproduced on bind10.isc.org, so please check
and fix it.

In general we shouldn't keep DISABLED tests too long, so if you don't
think it can be fixed soon, please consider refer the offending change.

comment:2 Changed 9 years ago by jinmei

  • Owner set to hanfeng
  • Status changed from new to assigned

comment:3 in reply to: ↑ 1 Changed 9 years ago by hanfeng

Replying to jinmei:

I confirmed SHA224 was the only test that failed and temporarily
disabled it. I'm giving it to Feng, who implemented this test first.
The failed test can be reproduced on bind10.isc.org, so please check
and fix it.

In general we shouldn't keep DISABLED tests too long, so if you don't
think it can be fixed soon, please consider refer the offending change.

I found that in Bind10.isc.org, there are two versions of botan, one is located in /usr/lib/libbotan-1.7.8.so, and another is located in /opt/pkg/lib/libbotan-1.8.2.so, I checked that libbotan-1.7.8.so won't export sha224 symbol but libbotan-1.8.2.so does. Since we require the version of lib botan to be higher than 1.8.0. So the error can be fixed with link to right library.

comment:4 Changed 9 years ago by hanfeng

  • Owner changed from hanfeng to jreed

comment:5 Changed 9 years ago by jelte

  • Owner changed from jreed to jelte

actually, having the lib on the system should not be a problem, as long as it doesn't actually get linked in. I think this is just a matter of the correct magic in the Makefile. Stealing this ticket :) (it's not the only failure, though i do not know yet whether the other ones are related)

comment:6 Changed 9 years ago by jelte

  • Owner changed from jelte to UnAssigned
  • Status changed from assigned to reviewing

I made two commits, the first one is a workaround for the compilation problem.

I'm not sure that is an acceptable solution though; the problem is that GTEST_LDFLAGS is added, which in itself should not be bad, but on this particular system the path in there also contains the wrong botan. libtool or ld does match this and then links to it, instead of the right one (which is also in the existing path settings, but later). So the workaround adds BOTAN_LDFLAGS to the relevant makefiles. I also added a test here to trigger this problem in libdns++ tests.

The second patch is only slightly related; there were more disabled tests (three truncated sigs tests), which were disabled because they failed. Turns out they failed because of an hardcoded return false in verify() (which did have nice documentation as to why, btw). Removed that temporary code, and enabled those tests.

If we decide not to merge the first commit, I request we cherry-pick the second one (which is quite simple)

comment:7 Changed 9 years ago by shane

  • Milestone changed from New Tasks to Sprint-20110531

I'm putting this on the current sprint since that is when the work was done.

Can we get a decision about this and move forward?

comment:8 Changed 9 years ago by vorner

  • Owner changed from UnAssigned to vorner

comment:9 follow-up: Changed 9 years ago by vorner

  • Owner changed from vorner to jelte

Hello

Regarding the second commit:

-                // Once we support truncation correctly, this if-clause should
-                // (and the capitalized comment above) be removed.
-                return (false);

Maybe remove even the capitalised comment, then? ;-).

As for the second, well, the problem is either caused by bug in the system installation or bug in libtool/ld or something like that. So, as a workaround for it, I think it is OK. I'd not like to see inclusion of the flags for every binary that uses TSIG, on the other hand. Tests are fine IMO.

So, please merge, so we don't spend too much time by stuff like this.

comment:10 in reply to: ↑ 9 Changed 9 years ago by jinmei

Replying to vorner:

Regarding the second commit:

-                // Once we support truncation correctly, this if-clause should
-                // (and the capitalized comment above) be removed.
-                return (false);

Maybe remove even the capitalised comment, then? ;-).

Wait...we cannot remove this part yet. The "truncation" here is not
the one we are dealing with in this ticket; it's about the TSIG
implementation. In fact with removing this part TSIGTest.tooShortMAC
will fail.

I'll re-add the check myself.

comment:11 Changed 9 years ago by jelte

  • Resolution set to fixed
  • Status changed from reviewing to closed

ok. merged, closing ticket.

comment:12 Changed 9 years ago by jelte

  • Estimated Difficulty changed from 0.0 to 2
Note: See TracTickets for help on using tickets.