Re: [openssl-dev] License change agreement

2017-03-24 Thread Otto Moerbeek
On Fri, Mar 24, 2017 at 01:29:53PM +0100, Richard Levitte wrote:

> In message <20170324121435.gq70...@colo.drijf.net> on Fri, 24 Mar 2017 
> 13:14:35 +0100, Otto Moerbeek  said:
> 
> otto> On Fri, Mar 24, 2017 at 11:53:10AM +, Blumenthal, Uri - 0553 - 
> MITLL wrote:
> otto> 
> otto> > I personally think this issue is being blown way out of proportion 
> and beyond the boundary of reason. 
> otto> > 
> otto> > Regards,
> otto> > Uri
> otto> 
> otto> Is it reasonable to step on the rights of authors with the backing of
> otto> large corporations? Individual authors that might have chosen to
> otto> change email address or are unable to be contacted for other reasons?
> otto> 
> otto> It is sad to see the corporate giants dictate the policies of yet
> otto> another open source project, without regard for the spirit of
> otto> copyright law which is to protect the individual author.
> 
> If I'm reading you correctly, *any* license change faces the exact
> same problem.  My interpretation of what you say is that unless we can
> successfully reach all contributors, no exception, we're stuck with
> the current license, probably for life.
> 
> Am I reading you correctly?

Yes, the default is "no, you're not allowed to change the license", not
"yes, you are allowed".

If you do not have explicit permission, the contribution(s) of an
auther must remain under the existing license or be removed. If you do
no want that, you should rewrite that piece so you can attach your
preferred license as author.

-Otto
-- 
openssl-dev mailing list
To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev


Re: [openssl-dev] License change agreement

2017-03-24 Thread Otto Moerbeek
On Fri, Mar 24, 2017 at 11:53:10AM +, Blumenthal, Uri - 0553 - MITLL wrote:

> I personally think this issue is being blown way out of proportion and beyond 
> the boundary of reason. 
> 
> Regards,
> Uri

Is it reasonable to step on the rights of authors with the backing of
large corporations? Individual authors that might have chosen to
change email address or are unable to be contacted for other reasons?

It is sad to see the corporate giants dictate the policies of yet
another open source project, without regard for the spirit of
copyright law which is to protect the individual author.

-Otto
-- 
openssl-dev mailing list
To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev


Re: [openssl-dev] License change agreement

2017-03-24 Thread Otto Moerbeek
On Fri, Mar 24, 2017 at 09:40:16AM +0100, Kurt Roeckx wrote:

> On Fri, Mar 24, 2017 at 08:36:02AM +0100, Otto Moerbeek wrote:
> > On Fri, Mar 24, 2017 at 08:21:49AM +0100, Marcus Meissner wrote:
> > 
> > > On Fri, Mar 24, 2017 at 07:48:58AM +0100, Otto Moerbeek wrote:
> > > > On Fri, Mar 24, 2017 at 04:11:48AM +, Blumenthal, Uri - 0553 - 
> > > > MITLL wrote:
> > > > 
> > > > > Apache license is fine for me, while GPL could be problematic. 
> > > > > Incompatibility with GPLv2 is not a problem for us. 
> > > > > 
> > > > > If it is a problem for somebody - feel free to explain the details. 
> > > > > Though I think the decision has been made, and the majority is OK 
> > > > > with it. 
> > > > 
> > > > I like to mention that any license change cannot be made based on a
> > > > majority vote or any other method other than getting each author (or
> > > > its legal representative) to *explicitly* allow the change. The method
> > > > of "nothing heard equals consent" is not valid in any jurisdiction I
> > > > know of.
> > > > 
> > > > While I'm not a contributor (I think I only sent in a small diff years
> > > > ago), I would like to stress that the planned relicensing procedure is
> > > > not legal and can be challenged in court.
> > > 
> > > Well, emails were sent yesterday out to _all_ contributors for ack/deny 
> > > the change.
> > 
> > Read the last line of the mail, it says the no reactions equals
> > consent. That is the illegal part.
> 
> The legal advice we got said that we should do our best to contact
> people. If we contacted them, they had the possibility to say no.
> We will give them time and go over all people that didn't reply to
> try to reach them.
> 
> But if they don't reply, as said, we will assume they have no
> problem with the license change. If at some later point in time
> they do come forward and say no, we will deal with that at that
> time.
> 
> 
> Kurt

Probably illegal and definitely immoral, in my opinion. Copyright law
exists to protect authors from these kind of practises.

-Otto
-- 
openssl-dev mailing list
To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev


Re: [openssl-dev] License change agreement

2017-03-24 Thread Otto Moerbeek
On Fri, Mar 24, 2017 at 08:21:49AM +0100, Marcus Meissner wrote:

> On Fri, Mar 24, 2017 at 07:48:58AM +0100, Otto Moerbeek wrote:
> > On Fri, Mar 24, 2017 at 04:11:48AM +, Blumenthal, Uri - 0553 - MITLL 
> > wrote:
> > 
> > > Apache license is fine for me, while GPL could be problematic. 
> > > Incompatibility with GPLv2 is not a problem for us. 
> > > 
> > > If it is a problem for somebody - feel free to explain the details. 
> > > Though I think the decision has been made, and the majority is OK with 
> > > it. 
> > 
> > I like to mention that any license change cannot be made based on a
> > majority vote or any other method other than getting each author (or
> > its legal representative) to *explicitly* allow the change. The method
> > of "nothing heard equals consent" is not valid in any jurisdiction I
> > know of.
> > 
> > While I'm not a contributor (I think I only sent in a small diff years
> > ago), I would like to stress that the planned relicensing procedure is
> > not legal and can be challenged in court.
> 
> Well, emails were sent yesterday out to _all_ contributors for ack/deny the 
> change.
> 
> Ciao, Marcus
> -- 
> openssl-dev mailing list
> To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev

Read the last line of the mail, it says the no reactions equals
consent. That is the illegal part.

-Otto



-- 
openssl-dev mailing list
To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev


Re: [openssl-dev] License change agreement

2017-03-23 Thread Otto Moerbeek
On Fri, Mar 24, 2017 at 04:11:48AM +, Blumenthal, Uri - 0553 - MITLL wrote:

> Apache license is fine for me, while GPL could be problematic. 
> Incompatibility with GPLv2 is not a problem for us. 
> 
> If it is a problem for somebody - feel free to explain the details. Though I 
> think the decision has been made, and the majority is OK with it. 

I like to mention that any license change cannot be made based on a
majority vote or any other method other than getting each author (or
its legal representative) to *explicitly* allow the change. The method
of "nothing heard equals consent" is not valid in any jurisdiction I
know of.

While I'm not a contributor (I think I only sent in a small diff years
ago), I would like to stress that the planned relicensing procedure is
not legal and can be challenged in court.

-Otto
-- 
openssl-dev mailing list
To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev


Re: [openssl.org #3448] Overflow

2014-07-10 Thread Otto Moerbeek
On Thu, Jul 10, 2014 at 11:26:46AM +0200, Chaney, Benjamin via RT wrote:

> Hello,
>   I have been looking at the OpenSSL source code, and this jumped out as a
> possible error. 'n?? is an unsigned before it is passed into ssl3_read_n,
> which causes the worry of an overflow. To prevent this, I added check that
> just makes sure that n is not less than zero, which wouldn't make sense
> anyway. This is my first time posting a change for OpenSSL, so please
> forgive any formatting errors.

n is signed:

 139 int ssl3_read_n(SSL *s, int n, int max, int extend)

and the checks
 153 if (n <= 0) return n;
and
 200 if (left > 0 && n > left)
 201 n = left;

make sure n > 0 already. 

-Otto


> Thanks,
>   Ben Chaney
> 
> 
> diff --git a/ssl/s3_pkt.c b/ssl/s3_pkt.c
> index 8fc3bb4..1d0bc6a 100644
> --- a/ssl/s3_pkt.c
> +++ b/ssl/s3_pkt.c
> @@ -224,7 +224,7 @@ int ssl3_read_n(SSL *s, int n, int max, int extend)
> rb->offset = len + align;
> }
>  
> -   if (n > (int)(rb->len - rb->offset)) /* does not happen */
> +   if ( (n > (int)(rb->len - rb->offset)) || (n < 0) ) /* does not
> happen */
> {
> SSLerr(SSL_F_SSL3_READ_N,ERR_R_INTERNAL_ERROR);
> return -1;
> 
> __
> OpenSSL Project http://www.openssl.org
> Development Mailing List   openssl-dev@openssl.org
> Automated List Manager   majord...@openssl.org
__
OpenSSL Project http://www.openssl.org
Development Mailing List   openssl-dev@openssl.org
Automated List Manager   majord...@openssl.org


Re: [openssl.org #3428] bug report : crypto/des/ofb64enc.c: Uninitialized variable: d

2014-07-04 Thread Otto Moerbeek
On Fri, Jul 04, 2014 at 08:38:23AM +0200, Kurt Roeckx wrote:

> On Fri, Jul 04, 2014 at 08:21:15AM +0200, Otto Moerbeek wrote:
> > On Thu, Jul 03, 2014 at 11:35:15PM +0200, Kurt Roeckx wrote:
> > 
> > > On Thu, Jul 03, 2014 at 09:28:47PM +0100, Ben Laurie wrote:
> > > > On 3 July 2014 20:06, Kurt Roeckx via RT  wrote:
> > > > > On Thu, Jul 03, 2014 at 07:51:28PM +0200, Toralf F?rster via RT wrote:
> > > > >> I think cppcheck is right here in void DES_ofb64_encrypt(), line 84, 
> > > > >> 85
> > > > >> and 96, or ?:
> > > > >>
> > > > > The line before that:
> > > > >
> > > > > dp=d;
> > > > >> l2c(v0,dp);<--- Uninitialized variable: d
> > > > >> l2c(v1,dp);<--- Uninitialized variable: d
> > > > >> while (l--)
> > > > >> {
> > > > >> if (n == 0)
> > > > >> {
> > > > >> DES_encrypt1(ti,schedule,DES_ENCRYPT);
> > > > >> dp=d;
> > > > >> t=ti[0]; l2c(t,dp);
> > > > >> t=ti[1]; l2c(t,dp);
> > > > >> save++;
> > > > >> }
> > > > >> *(out++)= *(in++)^d[n];<--- Uninitialized variable: d
> > > > >> n=(n+1)&0x07;
> > > > >> }
> > > > >
> > > > > d is uninitialized, but it's being written to, not read from,
> > > > > so I don't see a problem with this.
> > > > 
> > > > What?
> > > 
> > > So l2c is:
> > > #define l2c(l,c)(*((c)++)=(unsigned char)(((l))&0xff), \
> > >  *((c)++)=(unsigned char)(((l)>> 8L)&0xff), \
> > >  *((c)++)=(unsigned char)(((l)>>16L)&0xff), \
> > >  *((c)++)=(unsigned char)(((l)>>24L)&0xff))
> > > 
> > > It reads v0 and v1 and writes to d (dp).  d being uninitialized
> > > shouldn't be an issue.  Or am I missing something?
> > 
> > Yes, c (which is d) is both incremented and dereferenced. 
> 
> So we have:
> DES_cblock d;
> which as far as I know really is:
> unsigned char d[8];
> 
> and:
> register unsigned char *dp=d;
> *((dp)++) = foo;
> 
> d is a valid pointer, but the content it points to is
> uninitialized.  We end up writing to d[0], d[1], ..., d[7].  I
> don't see us reading it, nor do I see a problem with it.
> 
> 
> Kurt

OK, but then d *is* initialized. It would cause less confusion if
you'd make a difference between d and *d in your comments.

-Otto
__
OpenSSL Project http://www.openssl.org
Development Mailing List   openssl-dev@openssl.org
Automated List Manager   majord...@openssl.org


Re: [openssl.org #3428] bug report : crypto/des/ofb64enc.c: Uninitialized variable: d

2014-07-03 Thread Otto Moerbeek
On Thu, Jul 03, 2014 at 11:35:15PM +0200, Kurt Roeckx wrote:

> On Thu, Jul 03, 2014 at 09:28:47PM +0100, Ben Laurie wrote:
> > On 3 July 2014 20:06, Kurt Roeckx via RT  wrote:
> > > On Thu, Jul 03, 2014 at 07:51:28PM +0200, Toralf F?rster via RT wrote:
> > >> I think cppcheck is right here in void DES_ofb64_encrypt(), line 84, 85
> > >> and 96, or ?:
> > >>
> > > The line before that:
> > >
> > > dp=d;
> > >> l2c(v0,dp);<--- Uninitialized variable: d
> > >> l2c(v1,dp);<--- Uninitialized variable: d
> > >> while (l--)
> > >> {
> > >> if (n == 0)
> > >> {
> > >> DES_encrypt1(ti,schedule,DES_ENCRYPT);
> > >> dp=d;
> > >> t=ti[0]; l2c(t,dp);
> > >> t=ti[1]; l2c(t,dp);
> > >> save++;
> > >> }
> > >> *(out++)= *(in++)^d[n];<--- Uninitialized variable: d
> > >> n=(n+1)&0x07;
> > >> }
> > >
> > > d is uninitialized, but it's being written to, not read from,
> > > so I don't see a problem with this.
> > 
> > What?
> 
> So l2c is:
> #define l2c(l,c)(*((c)++)=(unsigned char)(((l))&0xff), \
>  *((c)++)=(unsigned char)(((l)>> 8L)&0xff), \
>  *((c)++)=(unsigned char)(((l)>>16L)&0xff), \
>  *((c)++)=(unsigned char)(((l)>>24L)&0xff))
> 
> It reads v0 and v1 and writes to d (dp).  d being uninitialized
> shouldn't be an issue.  Or am I missing something?

Yes, c (which is d) is both incremented and dereferenced. 

-Otto

__
OpenSSL Project http://www.openssl.org
Development Mailing List   openssl-dev@openssl.org
Automated List Manager   majord...@openssl.org


Re: [openssl.org #3375] Patch: Off-by-one errors in ssl_cipher_get_evp()

2014-06-21 Thread Otto Moerbeek
On Sat, Jun 21, 2014 at 09:58:33PM +0100, Matt Caswell wrote:

> On 21 June 2014 19:51, Otto Moerbeek  wrote:
> > You care confusing the matter. Kurt already expained he got the fix
> > from OpenBSD. After that explanation, the OpenSSL repo was fixed to
> > contain the attribution.
> >
> 
> I think we are all getting confused in this thread! :-)
> 
> Otto - I think you are confusing the other case of missing attribution
> with this one. The other case was fixed, this one has not been.
> 
> I was hoping that Kurt Cancemi would explain whether he got the patch
> from OpenBSD or independently discovered it. I haven't seen a response
> - are you saying that you have Otto?
> 
> I am happy to fix the repo to correctly attribute the fix, if that is
> the right course of action. To be honest in the absence of a response
> from Kurt I am unsure what the right thing to do is!?
> 
> Given that its been a week since this occurred, a compromise could be
> to fix the repo by keeping the commit as it is with its attribution,
> but adding an additional comment saying that we note that OpenBSD also
> discovered this issue on 24th May. Is that acceptable?
> 
> (NB: by fixing the repo I mean, adding a revert commit, and reapplying
> the change...I can't actually rewrite history)
> 
> Matt
> __
> OpenSSL Project http://www.openssl.org
> Development Mailing List   openssl-dev@openssl.org
> Automated List Manager   majord...@openssl.org

Oh yes, I see. Sorry for adding to the confusion...

Kurt, any comment?

-Otto
__
OpenSSL Project http://www.openssl.org
Development Mailing List   openssl-dev@openssl.org
Automated List Manager   majord...@openssl.org


Re: [openssl.org #3375] Patch: Off-by-one errors in ssl_cipher_get_evp()

2014-06-21 Thread Otto Moerbeek
On Sat, Jun 21, 2014 at 06:15:28PM +0100, Ben Laurie wrote:

> On 12 June 2014 23:15, Matt Caswell  wrote:
> >
> >
> > On 12/06/14 22:43, Otto Moerbeek wrote:
> >> On Thu, Jun 12, 2014 at 10:26:56PM +0200, Matt Caswell via RT wrote:
> >>
> >>> Patch applied:
> >>> https://git.openssl.org/gitweb/?p=openssl.git;a=commit;h=abfb989fe0b749ad61f1aa4cdb0ea4f952fc13e0
> >>>
> >>> Many thanks for your contribution.
> >>>
> >>> Matt
> >>
> >> http://www.openbsd.org/cgi-bin/cvsweb/src/lib/libssl/src/ssl/ssl_ciph.c.diff?r1=1.38;r2=1.39
> >>
> >> Again no attribution in problem report and commit. Claiming
> >> independent discovery is not going to be credible.
> >
> > The commit *is* attributed. The author is listed as Kurt Cancemi - this
> > is as it is attributed in the patch supplied in the problem report.
> 
> I presume he meant in the OpenBSD repo...
> 
> Kurt does not appear to be attributed there:
> http://www.openbsd.org/cgi-bin/cvsweb/src/lib/libssl/src/ssl/ssl_ciph.c?rev=1.39.
> __
> OpenSSL Project http://www.openssl.org
> Development Mailing List   openssl-dev@openssl.org
> Automated List Manager   majord...@openssl.org

You care confusing the matter. Kurt already expained he got the fix
from OpenBSD. After that explanation, the OpenSSL repo was fixed to
contain the attribution. 

-Otto

__
OpenSSL Project http://www.openssl.org
Development Mailing List   openssl-dev@openssl.org
Automated List Manager   majord...@openssl.org


Re: [openssl.org #3375] Patch: Off-by-one errors in ssl_cipher_get_evp()

2014-06-12 Thread Otto Moerbeek
On Thu, Jun 12, 2014 at 11:15:18PM +0100, Matt Caswell wrote:

> 
> 
> On 12/06/14 22:43, Otto Moerbeek wrote:
> > On Thu, Jun 12, 2014 at 10:26:56PM +0200, Matt Caswell via RT wrote:
> > 
> >> Patch applied:
> >> https://git.openssl.org/gitweb/?p=openssl.git;a=commit;h=abfb989fe0b749ad61f1aa4cdb0ea4f952fc13e0
> >>
> >> Many thanks for your contribution.
> >>
> >> Matt
> > 
> > http://www.openbsd.org/cgi-bin/cvsweb/src/lib/libssl/src/ssl/ssl_ciph.c.diff?r1=1.38;r2=1.39
> > 
> > Again no attribution in problem report and commit. Claiming
> > independent discovery is not going to be credible. 
> 
> The commit *is* attributed. The author is listed as Kurt Cancemi - this
> is as it is attributed in the patch supplied in the problem report.
> 
> I cannot say how Kurt found this defect - that is for him to answer.
> 
> All I can go on is the information supplied to me in the problem report
> and patch. I had no idea that openbsd had also discovered and fixed this
> defect until you sent the above link.

OK, let's hope Kurt shares his story and the attribution can be
retrofitted if needed.

-Otto
__
OpenSSL Project http://www.openssl.org
Development Mailing List   openssl-dev@openssl.org
Automated List Manager   majord...@openssl.org


Re: [openssl.org #3375] Patch: Off-by-one errors in ssl_cipher_get_evp()

2014-06-12 Thread Otto Moerbeek
On Thu, Jun 12, 2014 at 10:26:56PM +0200, Matt Caswell via RT wrote:

> Patch applied:
> https://git.openssl.org/gitweb/?p=openssl.git;a=commit;h=abfb989fe0b749ad61f1aa4cdb0ea4f952fc13e0
> 
> Many thanks for your contribution.
> 
> Matt

http://www.openbsd.org/cgi-bin/cvsweb/src/lib/libssl/src/ssl/ssl_ciph.c.diff?r1=1.38;r2=1.39

Again no attribution in problem report and commit. Claiming
independent discovery is not going to be credible. 

-Otto







__
OpenSSL Project http://www.openssl.org
Development Mailing List   openssl-dev@openssl.org
Automated List Manager   majord...@openssl.org


Re: Another security bug, this time in MAC verification...

2014-06-10 Thread Otto Moerbeek
On Tue, Jun 10, 2014 at 11:35:06PM +0100, Matt Caswell wrote:

> On 10 June 2014 21:52, Kurt Roeckx  wrote:
> >> As far as I can see this is SSLv3 only, and only about the Finish
> >> message.
> >>
> >> So it seems that function return the length of the digest, and in
> >> some error cases 0.  We'll end up with a wrong value in
> >> (peer_)finish_md_len.
> >>
> >> It should then result in this error:
> >> if (i != n)
> >> {
> >> al=SSL_AD_DECODE_ERROR;
> >> SSLerr(SSL_F_SSL3_GET_FINISHED,SSL_R_BAD_DIGEST_LENGTH);
> >> goto f_err;
> >> }
> >>
> >> So at first look there doesn't seem to be anything wrong with the
> >> current code.  But their patch doesn't do anything wrong either.
> >
> > So to clarify this a little more.  ssl3_final_finish_mac() returns
> > 0 on an internal error, or the length of the digest.  In case of SSLv3
> > it's both an MD5 and SHA1.  In ssl3_final_finish_mac() they only
> > get calculated and the length is returned.  The check that they
> > are correct happens just after the if I quoted above.
> 
> I can't see a way that this could be exploited. It is a bug though.
> 
> I've just pushed a fix:
> https://git.openssl.org/gitweb/?p=openssl.git;a=commit;h=2f1dffa88e1b120add4f0b3a794fbca65aa7768d
> 
> Matt
> __
> OpenSSL Project http://www.openssl.org
> Development Mailing List   openssl-dev@openssl.org
> Automated List Manager   majord...@openssl.org

It's common courtecy to attribute fixes to the original author or at
least the project.

-Otto

__
OpenSSL Project http://www.openssl.org
Development Mailing List   openssl-dev@openssl.org
Automated List Manager   majord...@openssl.org


Re: Prime generation

2014-05-26 Thread Otto Moerbeek
On Tue, May 27, 2014 at 08:23:29AM +0200, Otto Moerbeek wrote:

> On Tue, May 27, 2014 at 05:23:45AM +, mancha wrote:
> 
> > On Mon, May 26, 2014 at 09:01:53PM +, mancha wrote:
> > > On Mon, May 26, 2014 at 08:49:03PM +, Viktor Dukhovni wrote:
> > > > On Mon, May 26, 2014 at 08:20:43PM +, mancha wrote:
> > > > 
> > > > > For our purposes, the operative question is whether the
> > > > > distribution bias created can be leveraged in any way to attack
> > > > > factoring (RSA) or dlog (DH).
> > > > 
> > > > The maximum gap between primes of size $n$ is conjectured to be
> > > > around $log(n)^2$.  If $n$ is $2^k$, the gap is at most $k^2$, with
> > > > an average value of $k$.  Thus the most probable primes are most $k$
> > > > times more probable than is typical, and we lose at most $log(k)$
> > > > bits of entropy.  This is not a problem.
> > > 
> > > One consequence of the k-tuple conjecture (generally believed to be
> > > true) is that the size of gaps between primes is distributed poisson.
> > > 
> > > You're right when you say the entropy loss between a uniform
> > > distribution to OpenSSL's biased one is small. In that sense there is
> > > not much to be gained entropy-wise from using a process that gives
> > > uniformly distributed primes over what OpenSSL does.
> > > 
> > > However, if a way exists to exploit the OpenSSL distribution bias, it
> > > can be modified to be used against uniformly distributed primes with
> > > only minimal algorithmic complexity increases. In other words, the
> > > gold standard here isn't a uniform distribution.
> > > 
> > > --mancha
> > 
> > This is probably more wonkish than Ben intended with his question but
> > for those interested, the Poisson result I alluded to is due to
> > Gallagher [1].
> > 
> > [1] Gallagher, On the distribution of primes in short intervals,
> > Mathematika, 1976
> 
> Would this work: if you are worried the algorithm will never pick the
> highest of a prime pair, just make it search backward half of the time?

(unless it hits it right on of course)

> 
> But I understand it has no real security implications.
> 
>   -Otto
> 
> __
> OpenSSL Project http://www.openssl.org
> Development Mailing List   openssl-dev@openssl.org
> Automated List Manager   majord...@openssl.org
__
OpenSSL Project http://www.openssl.org
Development Mailing List   openssl-dev@openssl.org
Automated List Manager   majord...@openssl.org


Re: Prime generation

2014-05-26 Thread Otto Moerbeek
On Tue, May 27, 2014 at 05:23:45AM +, mancha wrote:

> On Mon, May 26, 2014 at 09:01:53PM +, mancha wrote:
> > On Mon, May 26, 2014 at 08:49:03PM +, Viktor Dukhovni wrote:
> > > On Mon, May 26, 2014 at 08:20:43PM +, mancha wrote:
> > > 
> > > > For our purposes, the operative question is whether the
> > > > distribution bias created can be leveraged in any way to attack
> > > > factoring (RSA) or dlog (DH).
> > > 
> > > The maximum gap between primes of size $n$ is conjectured to be
> > > around $log(n)^2$.  If $n$ is $2^k$, the gap is at most $k^2$, with
> > > an average value of $k$.  Thus the most probable primes are most $k$
> > > times more probable than is typical, and we lose at most $log(k)$
> > > bits of entropy.  This is not a problem.
> > 
> > One consequence of the k-tuple conjecture (generally believed to be
> > true) is that the size of gaps between primes is distributed poisson.
> > 
> > You're right when you say the entropy loss between a uniform
> > distribution to OpenSSL's biased one is small. In that sense there is
> > not much to be gained entropy-wise from using a process that gives
> > uniformly distributed primes over what OpenSSL does.
> > 
> > However, if a way exists to exploit the OpenSSL distribution bias, it
> > can be modified to be used against uniformly distributed primes with
> > only minimal algorithmic complexity increases. In other words, the
> > gold standard here isn't a uniform distribution.
> > 
> > --mancha
> 
> This is probably more wonkish than Ben intended with his question but
> for those interested, the Poisson result I alluded to is due to
> Gallagher [1].
> 
> [1] Gallagher, On the distribution of primes in short intervals,
> Mathematika, 1976

Would this work: if you are worried the algorithm will never pick the
highest of a prime pair, just make it search backward half of the time?

But I understand it has no real security implications.

-Otto

__
OpenSSL Project http://www.openssl.org
Development Mailing List   openssl-dev@openssl.org
Automated List Manager   majord...@openssl.org


Re: [openssl.org #3317] Patch: Avoid out-of-bounds write in SSL_get_shared_ciphers

2014-05-12 Thread Otto Moerbeek
On Mon, May 12, 2014 at 11:20:19AM +0200, Otto Moerbeek wrote:

> On Mon, May 12, 2014 at 01:09:15AM +0200, Matt Caswell via RT wrote:
> 
> > Patch applied in commit 308505b838e4e3ce8485bb30f5b26e2766dc7f8b. Similar
> > commits in the 1.0.2, 1.0.1, 1.0.0 and 0.9.8 branches.
> > 
> > Many thanks for your contribution.
> > 
> > Matt
> > 
> > __
> > OpenSSL Project http://www.openssl.org
> > Development Mailing List   openssl-dev@openssl.org
> > Automated List Manager   majord...@openssl.org
> 
> This diff contains a use before init (spotted by Miod Vallat).


To be more precise, the proposed diff does not contain a use before
init, but the committed code does. 

-Otto

__
OpenSSL Project http://www.openssl.org
Development Mailing List   openssl-dev@openssl.org
Automated List Manager   majord...@openssl.org


Re: [openssl.org #3317] Patch: Avoid out-of-bounds write in SSL_get_shared_ciphers

2014-05-12 Thread Otto Moerbeek
On Mon, May 12, 2014 at 01:09:15AM +0200, Matt Caswell via RT wrote:

> Patch applied in commit 308505b838e4e3ce8485bb30f5b26e2766dc7f8b. Similar
> commits in the 1.0.2, 1.0.1, 1.0.0 and 0.9.8 branches.
> 
> Many thanks for your contribution.
> 
> Matt
> 
> __
> OpenSSL Project http://www.openssl.org
> Development Mailing List   openssl-dev@openssl.org
> Automated List Manager   majord...@openssl.org

This diff contains a use before init (spotted by Miod Vallat).

-0tto
__
OpenSSL Project http://www.openssl.org
Development Mailing List   openssl-dev@openssl.org
Automated List Manager   majord...@openssl.org


Re: OpenSSL has exploit mitigation countermeasures to make sure its exploitable

2014-04-12 Thread Otto Moerbeek
On Fri, Apr 11, 2014 at 05:51:17PM -0500, Reini Urban wrote:

> On 04/11/2014 04:13 PM, Carlos Alberto Lopez Perez wrote:
> >Probably this blog post provides more information about what Akamai has
> >been doing related to this issue:
> >
> >https://blogs.akamai.com/2014/04/heartbleed-update.html
> >
> >It would be appreciated if you cared to contribute back your own custom
> >secure_malloc allocator.
> 
> just tcmalloc. which uses mmap, not sbrk.
> you can build your libssl also against tcmalloc. google prefers it also I
> heard.

While tcmalloc is very fast, it will not detect various forms of
misuse. It does not returns memory to the OS, for example. 

See http://goog-perftools.sourceforge.net/doc/tcmalloc.html

-Otto
__
OpenSSL Project http://www.openssl.org
Development Mailing List   openssl-dev@openssl.org
Automated List Manager   majord...@openssl.org


Re: OpenSSL has exploit mitigation countermeasures to make sure its exploitable

2014-04-10 Thread Otto Moerbeek
On Thu, Apr 10, 2014 at 12:46:23PM -0400, Salz, Rich wrote:

> We've been compiling -DOPENSSL_NO_BUF_FREELISTS forever.  Our only complaint 
> is that the BUF is misspelled :)
> 
> Theo can be obnoxious.  This should not be news to most folks.

Read what Ted wrote. There's is a use after free if you
-DOPENSSL_NO_BUF_FREELISTS It would have been spotted by OpenBSD's malloc. 

http://www.tedunangst.com/flak/post/analysis-of-openssl-freelist-reuse

-Otto
__
OpenSSL Project http://www.openssl.org
Development Mailing List   openssl-dev@openssl.org
Automated List Manager   majord...@openssl.org


[openssl.org #1396] patch: use S_ISDIR macro instead of S_IFDIR flags

2006-09-26 Thread Otto Moerbeek via RT


It is better to use the S_IS* macros instead of only masking with the
S_IF* flags. For example, sb.st_mode & S_ISDIR is true also when the
file is actually a unix domain socket. 

-Otto


Index: ca.c
===
RCS file: /cvs/src/lib/libssl/src/apps/ca.c,v
retrieving revision 1.19
diff -u -p -r1.19 ca.c
--- ca.c27 Jun 2006 05:06:54 -  1.19
+++ ca.c25 Sep 2006 18:31:22 -
@@ -832,8 +832,8 @@ bad:
perror(outdir);
goto err;
}
-#ifdef S_IFDIR
-   if (!(sb.st_mode & S_IFDIR))
+#ifdef S_ISDIR
+   if (!S_ISDIR(sb.st_mode))
{
BIO_printf(bio_err,"%s need to be a 
directory\n",outdir);
perror(outdir);



__
OpenSSL Project http://www.openssl.org
Development Mailing List   openssl-dev@openssl.org
Automated List Manager   [EMAIL PROTECTED]


Re: "call to OpenBSD arms"

2004-08-31 Thread Otto Moerbeek


On Mon, 30 Aug 2004, Andy Polyakov wrote:

> Are there any OpenBSD people on the list? Yesterday I was cross-testing
> assembler modules in HEAD and noticed that OpenBSD shared build is totally
> inoperational. After adjusting rules and fixing up assembler modules to be
> compiled with -fPIC, I've ran into problem with linker. Normally we link with
> -so-name=lib[crypto|ssl].so.$VERSION -Bsymbolic --whole-archive
> lib[crypto|ssl].a --no-whole-archive. None of these seem to be applicable in
> OpenBSD case. -so-name is ignored. -Bsymbolic seems to be applicable to libs
> which do not have unresolved references. --whole-archive applies till the end
> of line and therefore tries to pull in the whole libgcc.a... So I've
> introduced separate link_*.bsd rule-set in Makefile.shared which links as
> --whole-archive lib[crypto|ssl].a -nostdlib. For the moment this applies to
> OpenBSD-i386 only. Remaining questions are: a) what's the heck with .so
> versioning? what substitutes for -so-name? b) can somebody test it on other
> OpenBSD hardware platforms? A.

OpenBSD maintains its own OpenSSL tree in its CVS repository under 
lib/libssl. That tree is derived from the OpenSSL distribution and once in 
a while the the OpenBSD tree is synced with the OpenSSL tree.
 
Needless to say, the version of OpenSSL that ships with OpenBSD builds 
fine on all supported architectures.

-Otto

__
OpenSSL Project http://www.openssl.org
Development Mailing List   [EMAIL PROTECTED]
Automated List Manager   [EMAIL PROTECTED]


Re: bignum audit

2003-10-28 Thread Otto Moerbeek


On Mon, 27 Oct 2003, Geoff Thorpe wrote:

> Hi y'all,
> 
> This is a "ping -b" to anyone who has an interest in the integer math code 
> in openssl. Otto and Nils had reported a few discrepencies a while back, 
> and there is already a RT ticket from Otto about one aspect of this. 
> However, as I wade into crypto/bn/, I finding that it's a bit of a 
> pandorah's box (I would gone with the "rabbit hole" thing, but that's a 
> little too fashionable lately).

I had exactly the same feeling when I looked at the crypto/bn code 
while investigating the BN_add_word problem.

> My interest is a little more fundamental than the particular error cases 
> that have been discussed so far - I wold like to make the API more robust 
> against operations that leave BIGNUM variables in an inconsistent state. 

Yeah, ever since I looked into the bn functions I have the feeling more 
bugs are hidden there, and some of these bugs could have been avoided by 
being more strict wrt to representation of BNs. (I also hate the use of 
the letter l as a variable name, but that sure is OT).

> BTW: my definition of a consistent state for a bignum x is for it to be 
> invariant under bn_fix_top(). It's a home-brewed definition for sure, but 
> I think it's logical enough because of what "fix" means. IMHO 
> bn_fix_top() should only be needed to recalibrate a bignum structure 
> after directly manipulating bignum's data representation, *especially* 
> from the point of view of API users though this seems rational for 
> internal code too. Of course, there are obviously other ways for a BIGNUM 
> structure to be inconsistent too, but I'm only interested in the 
> relationship between the x->top index and the x->d[0..(top-1)] array-data 
> because that is there we currently legitimise something I consider to be 
> a bug. I've started some auditing/hacking that involves hijacking some 
> existing macros (eg. bn_[fix|check]_top) and adding some others (eg. 
> bn_verify_top), and in doing this, I've stumbled onto a question that I 
> realise Nils had also posted some time ago, so I'll quote his version of 
> it;
>
> > Another question would be if both bignum representations for '0'
> > should be considered legal, i.e. is {{0, 0}, 0, 2, 0, 0} the same as
> > {{0, 0}, 1, 2, 0, 0} (BN_is_zero returns 1 for both representations
> > of '0') ?
> 
> This observation/question turns out to be quite a nutcracker. To explain, 
> for some perverse reason there is a convention in openssl to accept that 
> a legitimate "zero-valued" BIGNUM could either have x->top set to zero, 
> or it can have x->top set to one but with the corresponding word of the 
> bignum data (x->d[0]) set to zero. Anyone who has done more than one 
> high-school induction proof or implemented any kind of loop algorithm 
> should intuitively feel itchy about this kind of thing, and there are 
> some solid reasonings against this that go beyond intuition too. The best 
> generalised rule I could come up with was this; x->top should be the 
> minimal number of words for its data representation, which coincidently 
> happens to be a "rule" that is 1-1 with the output from bn_fix_top() 
> anyway. In particular one of these two "zero" forms is the output you get 
> from bn_fix_top() no matter which of the two you feed in. So in the case 
> of '0', this reasoning would suggest that x->top be zero. Does anyone out 
> there have some historical knowledge (SSLeay?) or some convincing 
> explanations for why the the exceptional 1-word form for '0' should be 
> considered acceptable?

I can imagine that intermediate results of some functions can have leading
zeroes. But any number that is exposed should be normalized (have leading 
zeroes removed). That includes zero, IMO.

> 
> Going on from that, this "tolerance" is the cause of quite a bit of 
> complexity in crypto/bn/, so as part of this "audit" I'm wondering to 
> what extent we can clean this up. The obvious issue is backwards 
> compatibility with applications or libraries that have some reliance on 
> this perversity. Speaking for myself, whilst I could understand some 

Perversity indeed :-) BNs are supposed to be opaque. Any app relying on
the represenation is already walking on thin ice. From the bn(3) man page:

   The basic object in this library is a BIGNUM. It is used
   to hold a single large integer. This type should be
   considered opaque and fields should not be modified or
   accessed directly.


> caution from people sensitive to that, my own caution is sensitive to 
> complexity and bugs in openssl itself and I think both stem rather easily 
> from working around this mathematically-illogical exceptional case. 
> Currently, all the bignum code needs to ensure it is robust against this 
> "rule", including the exposed API macros that are used in tight-loop 
> algorithm code for word-based adds, compares, etc. Read this another way: 
> any elegant algorithmic code that doesn't have an ugly "if()" befo

Re: BN_add_word bug

2003-09-25 Thread Otto Moerbeek


On Thu, 25 Sep 2003, Geoff Thorpe wrote:

> On September 25, 2003 03:33 am, Nils Larsch wrote:
> > Otto Moerbeek wrote:
> > 
> >
> > > OK, that would amount to the fixes below:
> > >
> > > - in BN_cmp, call bn_fix_top just before comparing the two tops.
> >
> > Not really necessary as the normal BN_* functions which change the
> > value of the bignum should always ensure that the top value is
> > correct (i.e. as small as possible), but of course adding bn_fix_top
> > does not really harm (besides wasting some cpu time :-) .
> 
> I would go one step further and suggest that BN_cmp() and operations like 
> it that should treat their inputs as "const" should not modify the 
> BIGNUMs at all. Fixing BIGNUMs wherever we spot problems helps propogate 
> the current mess, the correct fix (as Nils implied) is to trace the 
> problem back to where it is caused and fix it there. No BIGNUM structure 
> should ever be in an invalid state outside the scope of a BN_*** API 
> function (or macro). So each function should, on exit, have correctly 
> adjusted any data as necessary.

Well mmust say I agree, but until a more or less complete audit of the
BN code has been done, I like to bet on the safe side.

The moment one is sure that all BN ops result in valid BN's, the extra
bn_fix_top() calls can and should be removed. But since my work does
concentrate on using the NM lib, and _not_ on maintaining it, i went for
the safe, albeit ugly solution.

> Perhaps (any volunteers?) the best way to do this would be to define a 
> validation function/macro and call it from every exit path for every 
> BIGNUM API function and macro. This could sanity-check the in/out 
> variables and abort() if there's something wrong so that the debugger can 
> catch these problems where they occur rather than where we first notice 
> them (later on in things like BN_cmp). IMHO this would be infinitely more 
> useful than continually adding calls to bn_fix_top() everywhere it seems 
> it could seal off another "case".

This is extra tricky. The particular problem I found was not dependent on
the exit path taken, but on the particular values supplied to
BN_add_word(). But having a test set that has a full return path coverage
_would_ be a good start.

> > Perhaps you should send a bug report to [EMAIL PROTECTED] containing the
> > patch and a link to this thread.
> 
> Yup, this should certainly go into RT - but I think this would be the 
> ideal opportunity to audit the BIGNUM code properly for this sort of 
> thing. In particular, it would be nice if the result of this work would 
> also allow us to better constify the BIGNUM API (in CVS HEAD of course, 
> the stable branches would merely benefit from cleaner implementations;-).

In the meantime, the bug report has been submitted.

-Otto
__
OpenSSL Project http://www.openssl.org
Development Mailing List   [EMAIL PROTECTED]
Automated List Manager   [EMAIL PROTECTED]


[openssl.org #697] bn->top related bug fixes

2003-09-25 Thread Otto Moerbeek via RT

HI,

as requested by Nil Larschs, i'm sending this diff to [EMAIL PROTECTED]

For a discussion of these bugs and fixes, see the thread
http://www.mail-archive.com/[EMAIL PROTECTED]/msg16241.html

-Otto

Index: bn_lib.c
===
RCS file: /cvs/src/lib/libssl/src/crypto/bn/bn_lib.c,v
retrieving revision 1.9
diff -u -r1.9 bn_lib.c
--- bn_lib.c12 May 2003 02:18:36 -  1.9
+++ bn_lib.c24 Sep 2003 18:29:57 -
@@ -702,6 +702,9 @@
{ gt=1; lt= -1; }
else{ gt= -1; lt=1; }
 
+   bn_fix_top(a);
+   bn_fix_top(b);
+
if (a->top > b->top) return(gt);
if (a->top < b->top) return(lt);
for (i=a->top-1; i>=0; i--)
Index: bn_print.c
===
RCS file: /cvs/src/lib/libssl/src/crypto/bn/bn_print.c,v
retrieving revision 1.6
diff -u -r1.6 bn_print.c
--- bn_print.c  6 Apr 2003 09:22:53 -   1.6
+++ bn_print.c  24 Sep 2003 18:20:08 -
@@ -79,7 +79,7 @@
}
p=buf;
if (a->neg) *(p++)='-';
-   if (a->top == 0) *(p++)='0';
+   if (BN_is_zero(a)) *(p++)='0';
for (i=a->top-1; i >=0; i--)
{
for (j=BN_BITS2-8; j >= 0; j-=8)
@@ -123,7 +123,7 @@
p=buf;
lp=bn_data;
if (t->neg) *(p++)='-';
-   if (t->top == 0)
+   if (BN_is_zero(t))
{
*(p++)='0';
*(p++)='\0';
@@ -300,7 +300,7 @@
int ret=0;
 
if ((a->neg) && (BIO_write(bp,"-",1) != 1)) goto end;
-   if ((a->top == 0) && (BIO_write(bp,"0",1) != 1)) goto end;
+   if ((BN_is_zero(a)) && (BIO_write(bp,"0",1) != 1)) goto end;
for (i=a->top-1; i >=0; i--)
{
for (j=BN_BITS2-4; j >= 0; j-=4)
Index: bn_word.c
===
RCS file: /cvs/src/lib/libssl/src/crypto/bn/bn_word.c,v
retrieving revision 1.5
diff -u -r1.5 bn_word.c
--- bn_word.c   12 May 2003 02:18:36 -  1.5
+++ bn_word.c   24 Sep 2003 18:31:25 -
@@ -110,6 +110,9 @@
BN_ULONG l;
int i;
 
+   if ((w & BN_MASK2) == 0)
+   return(1);
+
if (a->neg)
{
a->neg=0;
@@ -142,6 +145,9 @@
 int BN_sub_word(BIGNUM *a, BN_ULONG w)
{
int i;
+
+   if ((w & BN_MASK2) == 0)
+   return(1);
 
if (BN_is_zero(a) || a->neg)
{

__
OpenSSL Project http://www.openssl.org
Development Mailing List   [EMAIL PROTECTED]
Automated List Manager   [EMAIL PROTECTED]


Re: BN_add_word bug

2003-09-24 Thread Otto Moerbeek


On Wed, 24 Sep 2003, Nils Larsch wrote:

> BN_cmp has a similiar problem. BN_cmp does not check if the top value
> is really correct (but it uses the top value nonetheless) i.e. leading
> zeros matters for BN_cmp. I think the best solution to avoid this is
> to let BN_add_word (BN_sub_word) immediately return if w == 0 (otherwise
> you must include a bn_fix_top somewhere).
> 
> Nils

OK, that would amount to the fixes below:

- in BN_cmp, call bn_fix_top just before comparing the two tops.

- in bn_print.c, change if(x->top == 0) to if (BN_is_zero(x)) (a few
cases)

- in bn_word.c, add the (w & BN_MASK2) == 0 check for both adding and
subtracting. I'm using the masked value, to be consistent with the code
later on, which also masks w.

Diff against the OpenBSD version of two months ago (to get the 
BN_add_word fix included). Applies without problems to openssl-0.9.7b.

-Otto


Index: bn_lib.c
===
RCS file: /cvs/src/lib/libssl/src/crypto/bn/bn_lib.c,v
retrieving revision 1.9
diff -u -r1.9 bn_lib.c
--- bn_lib.c12 May 2003 02:18:36 -  1.9
+++ bn_lib.c24 Sep 2003 18:29:57 -
@@ -702,6 +702,9 @@
{ gt=1; lt= -1; }
else{ gt= -1; lt=1; }
 
+   bn_fix_top(a);
+   bn_fix_top(b);
+
if (a->top > b->top) return(gt);
if (a->top < b->top) return(lt);
for (i=a->top-1; i>=0; i--)
Index: bn_print.c
===
RCS file: /cvs/src/lib/libssl/src/crypto/bn/bn_print.c,v
retrieving revision 1.6
diff -u -r1.6 bn_print.c
--- bn_print.c  6 Apr 2003 09:22:53 -   1.6
+++ bn_print.c  24 Sep 2003 18:20:08 -
@@ -79,7 +79,7 @@
}
p=buf;
if (a->neg) *(p++)='-';
-   if (a->top == 0) *(p++)='0';
+   if (BN_is_zero(a)) *(p++)='0';
for (i=a->top-1; i >=0; i--)
{
for (j=BN_BITS2-8; j >= 0; j-=8)
@@ -123,7 +123,7 @@
p=buf;
lp=bn_data;
if (t->neg) *(p++)='-';
-   if (t->top == 0)
+   if (BN_is_zero(t))
{
*(p++)='0';
*(p++)='\0';
@@ -300,7 +300,7 @@
int ret=0;
 
if ((a->neg) && (BIO_write(bp,"-",1) != 1)) goto end;
-   if ((a->top == 0) && (BIO_write(bp,"0",1) != 1)) goto end;
+   if ((BN_is_zero(a)) && (BIO_write(bp,"0",1) != 1)) goto end;
for (i=a->top-1; i >=0; i--)
{
for (j=BN_BITS2-4; j >= 0; j-=4)
Index: bn_word.c
===
RCS file: /cvs/src/lib/libssl/src/crypto/bn/bn_word.c,v
retrieving revision 1.5
diff -u -r1.5 bn_word.c
--- bn_word.c   12 May 2003 02:18:36 -  1.5
+++ bn_word.c   24 Sep 2003 18:31:25 -
@@ -110,6 +110,9 @@
BN_ULONG l;
int i;
 
+   if ((w & BN_MASK2) == 0)
+   return(1);
+
if (a->neg)
{
a->neg=0;
@@ -142,6 +145,9 @@
 int BN_sub_word(BIGNUM *a, BN_ULONG w)
{
int i;
+
+   if ((w & BN_MASK2) == 0)
+   return(1);
 
if (BN_is_zero(a) || a->neg)
{
__
OpenSSL Project http://www.openssl.org
Development Mailing List   [EMAIL PROTECTED]
Automated List Manager   [EMAIL PROTECTED]


Re: BN_add_word bug

2003-09-24 Thread Otto Moerbeek


On Wed, 24 Sep 2003, Nils Larsch wrote:

> Otto Moerbeek wrote:
> > Hi,
>
> Moin Otto,
>
> 
> > I've been working with the big number lib from the open ssl crypto
> > library, and I have found the following problem, which is demonstrated by
> > the program below (you may have to fix the includes if you test it on
> > another platform than OpenBSD).
> >
> > Summary:
> >
> > It seems that the code
> >
> > BIGNUM *z = BN_new();
> > BN_set_word(z, 0);
> > BN_add_word(z, 0);
> >
> > results in a corrupt z: top is bumped, where it should not have been. The
> > test program core dumps while printing the number.
>
> No, the above does not corrupt the bignum, the problem (core dump)
> occurs in BN_bn2dec as BN_bn2dec does not correctly check if the value
> to print is zero (a bignum A is zero if A->top == 0 or A->top == 1
> and A->d[0] == 0). Please try this patch:

Hmmm, did not try your patch yet, but here's another interesting case that
doesn't use BN_bn2dec():

int g(void)
{
BIGNUM *a, *b;

a = BN_new();
BN_set_word(a, 0);

b = BN_new();
BN_set_word(b, 0);
BN_add_word(b, 0);

return BN_cmp(a, b);
}

returns -1 ...

Does BN_cmp also has the same problem, or is something else happening
here?

> --- crypto/bn/bn_print_old.c2003-09-24 12:46:24.0 +0200
> +++ crypto/bn/bn_print.c2003-09-24 12:47:05.0 +0200
> @@ -122,7 +122,7 @@
>  p=buf;
>  lp=bn_data;
>  if (t->neg) *(p++)='-';
> -   if (t->top == 0)
> +   if (BN_is_zero(t))
>  {
>  *(p++)='0';
>  *(p++)='\0';
>
> >
> > I've tested this on various versions of OpenBSD, Linux and MacOS 10.
> >
> > The most simple fix could be:
> >
> > Index: lib/libssl/src/crypto/bn/bn_word.c
> > ===
> > RCS file: /cvs/src/lib/libssl/src/crypto/bn/bn_word.c,v
> > retrieving revision 1.5
> > diff -u -r1.5 bn_word.c
> > --- lib/libssl/src/crypto/bn/bn_word.c  12 May 2003 02:18:36 -  1.5
> > +++ lib/libssl/src/crypto/bn/bn_word.c  17 Aug 2003 04:50:15 -
> > @@ -110,6 +110,9 @@
> > BN_ULONG l;
> > int i;
> >
> > +   if ((w & BN_MASK2) == 0)
> > +   return(1);
> > +
> > if (a->neg)
> > {
> > a->neg=0;
>
> Nonetheless I think it makes sense to check if the input word in
> BN_add_word (and BN_sub_word) is zero and immediately return 1
> (btw: wouldn't be a simple 'if (w == 0) return 1;' be sufficient ?).

Don't know for sure, but later in the function the masked value is used,
so for safety reasons, I'm testing the masked value.

-Otto

>
> Nils
>
> __
> OpenSSL Project http://www.openssl.org
> Development Mailing List   [EMAIL PROTECTED]
> Automated List Manager   [EMAIL PROTECTED]
>
__
OpenSSL Project http://www.openssl.org
Development Mailing List   [EMAIL PROTECTED]
Automated List Manager   [EMAIL PROTECTED]


BN_add_word bug

2003-09-23 Thread Otto Moerbeek
Hi,

So far, no reply at all from openssl-team, so I'm sending this to the dev 
list.

BTW: if you try this on OpenBSD -current, wou won't see the bug, 
because it contains the proposed fix.

-Otto

-- Forwarded message --
Date: Wed, 20 Aug 2003 11:10:36 +0200 (CEST)
From: Otto Moerbeek <[EMAIL PROTECTED]>
To: [EMAIL PROTECTED], [EMAIL PROTECTED]
Cc: [EMAIL PROTECTED], [EMAIL PROTECTED]
Subject: BN_add_word bug

Hi,

I've been working with the big number lib from the open ssl crypto
library, and I have found the following problem, which is demonstrated by
the program below (you may have to fix the includes if you test it on 
another platform than OpenBSD).

Summary:

It seems that the code

BIGNUM *z = BN_new();
BN_set_word(z, 0);
BN_add_word(z, 0);

results in a corrupt z: top is bumped, where it should not have been. The 
test program core dumps while printing the number.

I've tested this on various versions of OpenBSD, Linux and MacOS 10.

The most simple fix could be:

Index: lib/libssl/src/crypto/bn/bn_word.c
===
RCS file: /cvs/src/lib/libssl/src/crypto/bn/bn_word.c,v
retrieving revision 1.5
diff -u -r1.5 bn_word.c
--- lib/libssl/src/crypto/bn/bn_word.c  12 May 2003 02:18:36 -  1.5
+++ lib/libssl/src/crypto/bn/bn_word.c  17 Aug 2003 04:50:15 -
@@ -110,6 +110,9 @@
BN_ULONG l;
int i;
 
+   if ((w & BN_MASK2) == 0)
+   return(1);
+
if (a->neg)
{
a->neg=0;

However, I am not sure it fixes 100% of the cases. The one case I found is 
fixed with this patch.

-Otto

 Test program 

#include 
#include 
#include 


void
bp(BIGNUM *a)
{
fprintf(stderr, "top   = %d\n", a->top);
fprintf(stderr, "dmax  = %d\n", a->dmax);
fprintf(stderr, "neg   = %d\n", a->neg);
fprintf(stderr, "flags = %d\n", a->flags);
}

void
f(int i, int x)
{
BIGNUM *a;
char *p;

a = BN_new();

BN_set_word(a, i);

fprintf(stderr, "before adding %d %d\n", i, x);
bp(a);
BN_print_fp(stderr, a);
fprintf(stderr, " (%s)\n", p=BN_bn2dec(a));
OPENSSL_free(p);

BN_add_word(a, x);

fprintf(stderr, "after adding:\n");
bp(a);
BN_print_fp(stderr, a);
fprintf(stderr, " (%s)\n", p=BN_bn2dec(a));
OPENSSL_free(p);

BN_free(a);
}

main()
{
f(1, 1);
f(1, 0);
f(0, 1);
f(0, 0);
}
__
OpenSSL Project http://www.openssl.org
Development Mailing List   [EMAIL PROTECTED]
Automated List Manager   [EMAIL PROTECTED]