Quantcast

ECDSA_SIG_new and ECDSA_SIG_free details

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
18 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

ECDSA_SIG_new and ECDSA_SIG_free details

Ken Goldman-2
1 - Is this a bit of a bug?

ECDSA_SIG_free() frees the r and s BIGNUMs before is frees the structure
itself.  However, ECDSA_SIG_new() doesn't set r and s to
NULL.  It calls zalloc, which sets them to 0x00 bytes.

OK, in most platforms, the NULL pointer is an all 0x00 bytes value, but
it's not guaranteed by the C standard.

E.g., http://c-faq.com/null/confusion4.html


2 - It would be nice if the man page advised that ECDSA_SIG_free() frees
the two r and s BIGNUMs before is frees the structure iteslf

--
openssl-users mailing list
To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-users
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: ECDSA_SIG_new and ECDSA_SIG_free details

Viktor Dukhovni

> On Jan 3, 2017, at 2:55 PM, Ken Goldman <[hidden email]> wrote:
>
> 1 - Is this a bit of a bug?
>
> ECDSA_SIG_free() frees the r and s BIGNUMs before is frees the structure itself.  However, ECDSA_SIG_new() doesn't set r and s to
> NULL.  It calls zalloc, which sets them to 0x00 bytes.
>
> OK, in most platforms, the NULL pointer is an all 0x00 bytes value, but it's not guaranteed by the C standard.
>
> E.g., http://c-faq.com/null/confusion4.html

OpenSSL does not support platforms where the memory representation of the
NULL pointer contains non-zero bytes. IIRC there are even tests for this.

> 2 - It would be nice if the man page advised that ECDSA_SIG_free() frees the two r and s BIGNUMs before is frees the structure itself.

Presumably the structure "owns" its R and S values.  If this needs
to be documented, that documentation should be in the "setter"
functions that take control of the values.

--
        Viktor.

--
openssl-users mailing list
To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-users
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: ECDSA_SIG_new and ECDSA_SIG_free details

Salz, Rich
> OpenSSL does not support platforms where the memory representation of
> the NULL pointer contains non-zero bytes. IIRC there are even tests for this.

Yes, the basic platform sanity tests, test/sanitytest.c
--
openssl-users mailing list
To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-users
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: ECDSA_SIG_new and ECDSA_SIG_free details

Stephan Mühlstrasser
In reply to this post by Viktor Dukhovni
Am 03.01.17 um 21:26 schrieb Viktor Dukhovni:

>
>> On Jan 3, 2017, at 2:55 PM, Ken Goldman <[hidden email]> wrote:
>>
>> 1 - Is this a bit of a bug?
>>
>> ECDSA_SIG_free() frees the r and s BIGNUMs before is frees the structure itself.  However, ECDSA_SIG_new() doesn't set r and s to
>> NULL.  It calls zalloc, which sets them to 0x00 bytes.
>>
>> OK, in most platforms, the NULL pointer is an all 0x00 bytes value, but it's not guaranteed by the C standard.
>>
>> E.g., http://c-faq.com/null/confusion4.html
>
> OpenSSL does not support platforms where the memory representation of the
> NULL pointer contains non-zero bytes. IIRC there are even tests for this.

Could someone from the OpenSSL team please explain the rationale for
this decision? What is the problem with using assignments with 0 or NULL
to initialize pointers?

--
Stephan
--
openssl-users mailing list
To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-users
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: ECDSA_SIG_new and ECDSA_SIG_free details

Salz, Rich
> > OpenSSL does not support platforms where the memory representation of
> > the NULL pointer contains non-zero bytes. IIRC there are even tests for
> this.
>
> Could someone from the OpenSSL team please explain the rationale for this
> decision? What is the problem with using assignments with 0 or NULL to
> initialize pointers?

I think you are confused.

There is no problem with what you posted.  The issue is that if NULL != 0, OpenSSL won't necessarily work.

See test/sanitytest.c for what we check.
--
openssl-users mailing list
To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-users
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: ECDSA_SIG_new and ECDSA_SIG_free details

Jeffrey Walton-3
In reply to this post by Stephan Mühlstrasser
> Could someone from the OpenSSL team please explain the rationale for this
> decision? What is the problem with using assignments with 0 or NULL to
> initialize pointers?

I'm not from the team, so take it for what its worth...

On some systems, NULL is _not_ 0. NULL can be anywhere in memory the
architecture wants it to be. It can be in a high page in memory, too.
One of my instructors in college was telling me about a system he
worked on where NULL was an address in the last page in memory, so it
took a value like `0xffffffff`.

Jeff
--
openssl-users mailing list
To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-users
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: ECDSA_SIG_new and ECDSA_SIG_free details - NULL vs zeros

Ken Goldman-2
In reply to this post by Stephan Mühlstrasser
On 1/11/2017 10:32 AM, Stephan Mühlstrasser wrote:
>>
>> OpenSSL does not support platforms where the memory representation of the
>> NULL pointer contains non-zero bytes. IIRC there are even tests for this.
>
> Could someone from the OpenSSL team please explain the rationale for
> this decision? What is the problem with using assignments with 0 or NULL
> to initialize pointers?

I suspect that it was a shortcut, where they used memset() on an entire
structure, and it hopefully set pointers to NULL.

What I pointed out is that if NULL is not all zeros, this breaks.

~~~  BTW ~~~

Compilers know this.  So

        char *ptr = NULL;

and

        char *ptr = 0;

are equivalent, even on platforms where NULL is not all zeros.

It's when you cast the ptr to an integer first that it fails.


--
openssl-users mailing list
To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-users
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: ECDSA_SIG_new and ECDSA_SIG_free details

Jakob Bohm-7
In reply to this post by Stephan Mühlstrasser
On 11/01/2017 16:32, Stephan Mühlstrasser wrote:

> Am 03.01.17 um 21:26 schrieb Viktor Dukhovni:
>>
>>> On Jan 3, 2017, at 2:55 PM, Ken Goldman <[hidden email]> wrote:
>>>
>>> 1 - Is this a bit of a bug?
>>>
>>> ECDSA_SIG_free() frees the r and s BIGNUMs before is frees the
>>> structure itself.  However, ECDSA_SIG_new() doesn't set r and s to
>>> NULL.  It calls zalloc, which sets them to 0x00 bytes.
>>>
>>> OK, in most platforms, the NULL pointer is an all 0x00 bytes value,
>>> but it's not guaranteed by the C standard.
>>>
>>> E.g., http://c-faq.com/null/confusion4.html
>>
>> OpenSSL does not support platforms where the memory representation of
>> the
>> NULL pointer contains non-zero bytes. IIRC there are even tests for
>> this.
>
> Could someone from the OpenSSL team please explain the rationale for
> this decision? What is the problem with using assignments with 0 or
> NULL to initialize pointers?
>
I am not from the OpenSSL team either, but the probable thinking is
like this:

The C (and C++) standard allows a number of things that are very rare
in practice, while providing very few guarantees about important
aspects of semantics.  Therefore most major cross-platform projects
will usually take a number of common-sense aspects that are already
common among the desired platforms and elevate them into "project-
specific" requirements, foregoing the option to port the code to
platforms that differ in those aspects.  This provides a more robust
and usable "dialect" of the C/C++ language, allowing code to not do
things that are clumsy, slow, error-prone or otherwise difficult.

For the specific requirement of NULL pointers being encoded as an
all-0 bit pattern (and the similar requirement for 0 integral values),
this allows the use of zero-initializing memory allocators/handlers to
forego the need to explicitly initialize NULL (and 0) field and array
element values.

Note that C++ has a similar, but weaker, requirements that the source
code literal 0 is a valid syntax for null pointer constants and that
many (but not all) uninitialized pointer and numeric fields should be
implicitly initialized to NULL / 0 by compiler generated code, which
doesn't cover the case of memory blocks that don't get to run the
(implicit) C++ constructor.

Enjoy

Jakob
--
Jakob Bohm, CIO, Partner, WiseMo A/S.  https://www.wisemo.com
Transformervej 29, 2860 Søborg, Denmark.  Direct +45 31 13 16 10
This public discussion message is non-binding and may contain errors.
WiseMo - Remote Service Management for PCs, Phones and Embedded

--
openssl-users mailing list
To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-users
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: ECDSA_SIG_new and ECDSA_SIG_free details - NULL vs zeros

Salz, Rich
In reply to this post by Ken Goldman-2
> I suspect that it was a shortcut, where they used memset() on an entire
> structure, and it hopefully set pointers to NULL.
>
> What I pointed out is that if NULL is not all zeros, this breaks.

And OpenSSL does not work on those platforms.  It is part of the test suite to check for this.  See test/sanitytest.c

--
openssl-users mailing list
To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-users
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: ECDSA_SIG_new and ECDSA_SIG_free details

Michael Wojcik
In reply to this post by Jeffrey Walton-3
> From: openssl-users [mailto:[hidden email]] On Behalf
> Of Jeffrey Walton
> Sent: Wednesday, January 11, 2017 11:19
> To: OpenSSL Users
> Subject: Re: [openssl-users] ECDSA_SIG_new and ECDSA_SIG_free details
>
> > Could someone from the OpenSSL team please explain the rationale for
> this
> > decision? What is the problem with using assignments with 0 or NULL to
> > initialize pointers?
>
> I'm not from the team, so take it for what its worth...
>
> On some systems, NULL is _not_ 0. NULL can be anywhere in memory the
> architecture wants it to be. It can be in a high page in memory, too.

This comment is misleading (as was Rich Salz's reply). Jeffrey's question, as phrased, is correct.

The definition of the NULL macro is mandated by the C standard. It is either an integer constant that evaluates to 0, or such a constant cast to void*.

The representation in memory of a null pointer need not be all-bits-zero. (The representation in memory of an integer constant with the value zero can either be all-bits-zero or, in the unlikely case of sign-magnitude integers, a sign bit of 1 followed by all-other-bits-zero.)

In other words, *in C source code* a null pointer is *always* created by assigning the NULL macro, or a literal 0, or such a thing cast to an appropriate pointer type, to a pointer variable. Even if the representation of a null pointer in memory is not all-bits-zero. Similarly, comparing a null pointer to a literal 0 evaluates as true, regardless of the representation of null pointers. That's required by the C standard; if it doesn't work, you're using a language which is almost but not quite entirely unlike C.

However: Operations such as memset(object_pointer, 0, size) may NOT create null pointers, because memset simply sets the underlying bits without any knowledge of types or representations.

And that's why many programs don't work on implementations where the null pointer representation is not all-bits-zero: because they use shortcuts like memset and calloc to "clear" pointers (generally as part of structs containing pointer fields), rather than doing it properly.

Doing it properly, incidentally, looks like this:

   static const struct foo foo0; /* implicitly equivalent to ... foo0 = {0} */
   ...
   struct foo *bar = malloc(sizeof *bar);
   *bar = foo0;

Unfortunately writing proper C is a rare skill - relatively few C programmers have ever even read the language specification - and much C code is saddled with lots of ancient technical debt. Also, of course, it often doesn't make economic sense to accommodate rare implementations. How many C programs work correctly on implementations where CHAR_BIT > 8?

Michael Wojcik
Distinguished Engineer, Micro Focus



--
openssl-users mailing list
To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-users
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: ECDSA_SIG_new and ECDSA_SIG_free details

Salz, Rich
> The representation in memory of a null pointer need not be all-bits-zero.
> (The representation in memory of an integer constant with the value zero
> can either be all-bits-zero or, in the unlikely case of sign-magnitude integers,
> a sign bit of 1 followed by all-other-bits-zero.)

And, again, openssl will not work on those platforms and we have a test to catch it.

> Doing it properly, incidentally, looks like this:

Look at apps/rehash.c :)

--
openssl-users mailing list
To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-users
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: ECDSA_SIG_new and ECDSA_SIG_free details

Viktor Dukhovni
In reply to this post by Michael Wojcik
On Wed, Jan 11, 2017 at 05:27:47PM +0000, Michael Wojcik wrote:

> Unfortunately writing proper C is a rare skill - relatively few C
> programmers have ever even read the language specification - and much C
> code is saddled with lots of ancient technical debt. Also, of course, it
> often doesn't make economic sense to accommodate rare implementations.

In the case of OpenSSL, the issue was well understood, and upon
consideration a decision was made to not support platforms where
the memory representation of NULL is not zero.  A test was written
to make sure that non-conformant platforms are detected.

By way of contrast, the Postfix project supports non-zero NULLs,
and explicitly initializes pointer-valued member fields in structures.

Neither project simply ignores the issue.

--
        Viktor.
--
openssl-users mailing list
To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-users
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: ECDSA_SIG_new and ECDSA_SIG_free details

Erwann Abalea-4
In reply to this post by Jeffrey Walton-3
ISO/C 2011, clause 6.3.2.3:
----
An integer constant expression with the value 0, or such an expression cast to type void *, is called a null pointer constant. If a null pointer constant is converted to a pointer type, the resulting pointer, called a null pointer, is guaranteed to compare unequal to any object or function.

Conversion of a null pointer to another pointer type yields a null pointer of that type. Any two null pointers shall compare equal.
----

int *var1 = 0;
int *var2 = (void*)0;

result in var1 and var2 to both be null pointers (the null pointer constant being « 0 » or « (void*)0 »).

This doesn’t matter if your specific machine encodes null pointers as ‘0xffffffff'.

On your specific machine, however:

int *var1;
int *var2 = 0;
memset(var1, 0, sizeof(var1));

won’t make var1 be a null pointer, but var2 will internally contain this 0xffffffff, and will be a null pointer.


Cordialement,
Erwann Abalea

> Le 11 janv. 2017 à 17:18, Jeffrey Walton <[hidden email]> a écrit :
>
>> Could someone from the OpenSSL team please explain the rationale for this
>> decision? What is the problem with using assignments with 0 or NULL to
>> initialize pointers?
>
> I'm not from the team, so take it for what its worth...
>
> On some systems, NULL is _not_ 0. NULL can be anywhere in memory the
> architecture wants it to be. It can be in a high page in memory, too.
> One of my instructors in college was telling me about a system he
> worked on where NULL was an address in the last page in memory, so it
> took a value like `0xffffffff`.
>
> Jeff
> --
> openssl-users mailing list
> To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-users
>

--
openssl-users mailing list
To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-users
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: ECDSA_SIG_new and ECDSA_SIG_free details

Stephan Mühlstrasser
In reply to this post by Salz, Rich
Am 11.01.17 um 17:09 schrieb Salz, Rich:

>>> OpenSSL does not support platforms where the memory representation of
>>> the NULL pointer contains non-zero bytes. IIRC there are even tests for
>> this.
>>
>> Could someone from the OpenSSL team please explain the rationale for this
>> decision? What is the problem with using assignments with 0 or NULL to
>> initialize pointers?
>
> I think you are confused.
>
> There is no problem with what you posted.  The issue is that if NULL != 0, OpenSSL won't necessarily work.
>
> See test/sanitytest.c for what we check.

I'm not confused. The other answers in the thread already explained in
detail that NULL and 0 are equivalent for initializing pointers and that
memset() is not a portable way to initialize pointers, and I am aware of
that.

My question was meant to ask why the pointers are initialized with
memset() instead of initializing them by an assignment with NULL or 0.
Was this a deliberate decision for some reason, or did it just creep in
and no one cares now to fix it? Would the OpenSSL team accept pull
requests that fix this?

--
Stephan
--
openssl-users mailing list
To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-users
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: ECDSA_SIG_new and ECDSA_SIG_free details

Salz, Rich
> My question was meant to ask why the pointers are initialized with
> memset() instead of initializing them by an assignment with NULL or 0.
> Was this a deliberate decision for some reason, or did it just creep in and no
> one cares now to fix it? Would the OpenSSL team accept pull requests that
> fix this?

It was a mix of what was done, and then a conscious decision to do things that way.

As for the PR, well, maybe...  We'd need to know details of which machine "test/sanitytest.c" fails on, and how popular it is to see if it's worthwhile.
--
openssl-users mailing list
To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-users
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: ECDSA_SIG_new and ECDSA_SIG_free details

Stephan Mühlstrasser
Am 12.01.17 um 13:19 schrieb Salz, Rich:

>> My question was meant to ask why the pointers are initialized with
>> memset() instead of initializing them by an assignment with NULL or 0.
>> Was this a deliberate decision for some reason, or did it just creep in and no
>> one cares now to fix it? Would the OpenSSL team accept pull requests that
>> fix this?
>
> It was a mix of what was done, and then a conscious decision to do things that way.
>
> As for the PR, well, maybe...  We'd need to know details of which machine "test/sanitytest.c" fails on, and how popular it is to see if it's worthwhile.
>

Thanks for the clarification.

I think IBM iSeries is affected by this, but I still have to verify this.

--
Stephan
--
openssl-users mailing list
To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-users
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: ECDSA_SIG_new and ECDSA_SIG_free details

Michael Wojcik
> From: openssl-users [mailto:[hidden email]] On Behalf
> Of Stephan Mühlstrasser
> Sent: Thursday, January 12, 2017 07:50
>
> I think IBM iSeries is affected by this, but I still have to verify this.

It's been years since I worked on the iSeries (in fact, it was mostly prior to the rename, so it was still OS/400 then); but IIRC the null-pointer representation was all-bits-zero for all of the IBM C implementations (EPM C, System/C, and ILE C).

The '400 / iSeries has an interesting pointer representation - 16 bytes, with a validity bit that can't be altered by user-mode code, to prevent constructing arbitrary pointers. It's a capabiltiy architecture, more or less. But in order to improve compatibility with pre-standard and poor C code, the C implementations recognize all-bits-zero in a pointer-type object as a null pointer. And attempting to dereference (via the MI MATPTR instruction) such an object raises the appropriate machine check (or program check? it's been a while).


In my experience, the real problems caused by non-conforming C code are more subtle. One thing that definitely *will* bite C programs on iSeries, for example, is failing to correctly declare a function that returns a pointer type, such as malloc - because an undeclared function is assumed to return int, and sizeof(int) < sizeof(void*) in those implementations.

And don't even get me started on calling undeclared functions on Itanium-based implementations...

Michael Wojcik
Distinguished Engineer, Micro Focus



--
openssl-users mailing list
To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-users
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: ECDSA_SIG_new and ECDSA_SIG_free details

Jeremy Farrell
In reply to this post by Salz, Rich
On 12/01/2017 12:19, Salz, Rich wrote:
It was a mix of what was done, and then a conscious decision to do things that way.

As for the PR, well, maybe...  We'd need to know details of which machine "test/sanitytest.c" fails on, and how popular it is to see if it's worthwhile.

That would be inefficient churning given the number of changes to replace conforming null pointer initialization with memset/calloc that have gone in since this decision was made. The decision sticks in the throat a bit for us standard nerds and old-timers who remember machines where the null pointer was not all-bits-zero, but it's decades since I heard of such a machine at large in the real world.
-- 
J. J. Farrell
Not speaking for Oracle

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