Modify

#714 closed defect (fixed)

build with NSS as SSL library

Reported by: Matěj Cepl <mcepl@…> Owned by:
Priority: normal Milestone:
Component: BitlBee Version: 3.0
Keywords: Cc:
IRC client+version: Client-independent Operating System: Public server
OS version/distro:

Description

Attached patch makes buildable with --ssl=nss and nss-3.12.8.

ssl_starttls and its friends are just identical with ones from ssl_openssl (and they should be probably refactored to some more general module), so I am not worried about them. However, ssl_des3_encrypt is my own creation based on http://www.mozilla.org/projects/security/pki/nss/sample-code/sample2.html and I wouldn't trust it two eurocents of my own. It needs through review and testing (I don't have MSN account, and as it is apparently the only protocol which uses it).

Also I have strong doubts about lib/des.c module. Not sure what it is good for now.

Attachments (6)

nss.diff (5.5 KB) - added by Matěj Cepl <mcepl@…> at 2010-11-06T18:11:38Z.
making NSS build work
bitlbee-des3-implement.patch (3.8 KB) - added by mcepl@… at 2010-12-28T14:50:26Z.
NSS-based implementation of ssl_des3_encrypt
bitlbee-fix-NSS.patch (719 bytes) - added by mcepl@… at 2010-12-28T14:54:44Z.
two more tiny fixes of support for NSS-based implementation of SSL
bitlbee-des3-implement.2.patch (3.8 KB) - added by mcepl@… at 2010-12-28T15:17:54Z.
NSS-based implementation of ssl_des3_encrypt
bitlbee-des3-implement.3.patch (11.5 KB) - added by mcepl@… at 2012-06-22T13:30:34Z.
The last version of the patch
0001-NSS-based-implementation-of-SSL-related-operations.patch (12.6 KB) - added by Matěj Cepl <mcepl@…> at 2012-07-24T15:12:22Z.
The last version of the patch

Download all attachments as: .zip

Change History (18)

Changed at 2010-11-06T18:11:38Z by Matěj Cepl <mcepl@…>

Attachment: nss.diff added

making NSS build work

comment:1 Changed at 2010-11-20T15:15:09Z by wilmer

Resolution: fixed
Status: newclosed

Submitted without the 3DES code since it didn't work and generated lots of compiler warnings. :-/ I'll just keep that code around for now, but would be happy if someone could fix the nss version. Thanks! This was broken for at least 2-3 years already.

comment:2 Changed at 2010-11-22T01:12:36Z by mcepl@…

Resolution: fixed
Status: closedreopened

I am still missing this diff in configure:

jakoubek:git (nss) $ git diff master configure
diff --git a/configure b/configure
index cd492b9..7298ba3 100755
--- a/configure
+++ b/configure
@@ -427,7 +427,7 @@ if [ "$ret" = "0" ]; then
        exit 1
 fi;
 
-if [ "$msn" = "1" -a "$ssl" != "openssl" -a "$ssl" != "gnutls" ]; then
+if [ "$msn" = "1" -a "$ssl" != "openssl" -a "$ssl" != "nss" -a "$ssl" != "gnutls" ]; then
        # Needed for MSN only. OpenSSL exports nice cipher functions already,
        # in case of GnuTLS we should be able to use gcrypt. Otherwise, use
        # built-in stuff. (Since right now those are the only two supported
jakoubek:git (nss) $ 

comment:3 Changed at 2010-11-22T10:39:22Z by wilmer

Resolution: fixed
Status: reopenedclosed

Yes, as I told in the previous comment I left out the 3des part so des.c is still necessary when compiling with NSS.

comment:4 Changed at 2010-12-28T14:39:58Z by mcepl@…

Resolution: fixed
Status: closedreopened

We have two more patches to add (originally by Ricky Zhou while helping me to fix https://bugzilla.redhat.com/show_bug.cgi?id=665553 and https://bugzilla.redhat.com/show_bug.cgi?id=666022).

Changed at 2010-12-28T14:50:26Z by mcepl@…

NSS-based implementation of ssl_des3_encrypt

Changed at 2010-12-28T14:54:44Z by mcepl@…

Attachment: bitlbee-fix-NSS.patch added

two more tiny fixes of support for NSS-based implementation of SSL

Changed at 2010-12-28T15:17:54Z by mcepl@…

NSS-based implementation of ssl_des3_encrypt

comment:5 Changed at 2010-12-28T15:19:06Z by anonymous

Oh well, I cannot edit this ticket ... from the three patches attached today, the third one is a combination of the first two, which are thus obsolete. Only the last one should be applied.

Changed at 2012-06-22T13:30:34Z by mcepl@…

The last version of the patch

comment:6 Changed at 2012-07-10T19:57:05Z by Matěj Cepl <mcepl@…>

hold on with merging, please ... I am waiting on review of those patches by our NSS people and apparently they will need a thorough rewrite (using SSL without checking certificates is probably not a best idea).

comment:7 Changed at 2012-07-24T15:12:01Z by Matěj Cepl <mcepl@…>

OK, you can merge it ... it doesn't actually check for identity of the server and doesn't validate certificates. If we have (which suspect we do) mostly self-signed certificates, we probably would need to implement something like what ssh does (saving known certificates and checking their consistency), but that would require changes in UI, so I would first apply this patch as it is, even in the incomplete state.

What do you think?

Changed at 2012-07-24T15:12:22Z by Matěj Cepl <mcepl@…>

The last version of the patch

comment:8 Changed at 2012-08-19T15:33:52Z by wilmer

Resolution: fixed
Status: reopenedclosed
* Compiling ssl_nss.c
ssl_nss.c: In function ‘ssl_write’:
ssl_nss.c:267:5: warning: ‘PR_err’ may be used uninitialized in this function [-Wuninitialized]

I fixed it by inserting a PR_err = PR_GetError() like in the read function. Please do pay attention to compiler warnings like this.

I haven't tested this much, hereby NSS support is your responsibility. :-)

comment:9 Changed at 2012-08-19T15:34:59Z by wilmer

BTW I've closed the bug, feel free to open a new one for new changes (like cert verification) if that happens at some point.

comment:10 Changed at 2012-08-28T15:11:17Z by mcepl@…

Resolution: fixed
Status: closedreopened

http://code.bitlbee.org/lh/bitlbee/revision/909 is not correct ... it doesn't contain any changes to lib/ssl_nss.c which is the most important (or only important) part of the patch.

comment:11 Changed at 2012-08-28T17:49:13Z by wilmer

W-T-F ...

I know that I applied the whole patch, IIRC I even made some additional corrections to the file. Yes, I did, I was commenting about those up there. Thanks for reopening.

comment:12 Changed at 2012-09-15T14:50:02Z by wilmer

Resolution: fixed
Status: reopenedclosed

Hilarious. So I did indeed commit this patch, I just forgot to push it to the repository.

changeset:devel,918.

Modify Ticket

Action
as closed The ticket will remain with no owner.
The resolution will be deleted. Next status will be 'reopened'.

Add Comment


E-mail address and name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.