Darryl Miles wrote:

> SHA1_Drop() seems like a bad name to me, maybe a better name would be
> SHA1_Clear() or SHA1_Reset() if the intention is to cause the internal
> state of the SHA1 to reset back to SHA1_Init() but it MUST be used on
> an already initialized SHA1 object.



Ah, I should have made that clearer. The intention of SHA1_Drop() is not
a full reset, but only an alternative cleanup point when SHA1_Final()
will never be called. The application doesn't get any functionality out
of it, but we get a chance to clean up.

The idea is future-proofing: SHA1_Drop() is currently a No-Op, but in
the future for whatever reason a SHA_CTX might want keep dynamically
allocated memory. That memory would always get a chance to be
deallocated because the application will call either SHA1_Final() or
SHA1_Drop().



> This leaves open the obvious API usage of wanting to jump start
> SHA1_Init() and SHA1_Load_State() if this is needed then consider
> creating a SHA1_Init_FromState() for this purpose (SHA1_Init_State()
> feels too vague and potentially misunderstood).



The intended use of the patch is to call SHA1_Load_State() directly, not
SHA1_Init() followed by SHA1_Load_State().

In other words:

* The application starts by freely choosing either SHA1_Init() or
SHA1_Load_State()
* The application ends by freely choosing either SHA1_Final() or
SHA1_Drop()




> I'm not sure if your method of versioning the state information is
> sufficient, I would like to propose that this problem domain be left
> mainly upto the application to deal with. My concern stems from
> different machine types, endianess and natural bitwidth (32/64bit)
> issues.
>
> If OpenSSL is to provide some form of API contract for the
> representation of "State" to the application that a new function
> method be used "u_int32_t SHA1_State_Version(void);". This would also
> be combined with CPU type + endian information as appropiate.
>
> It would be upto the application to store, know and check the 'State
> Version' that it is using is compatible with the runtime version of
> OpenSSL that is in use. rather than set a policy on how this 'State
> Version' information is stored along with the 'State Data'.
> SHA1_XXXX_State() function should simply deal with 'State Data'.



I beg to differ.

Leaving this up to the application is another way of saying that
inter-operability will happen not at all or badly.

In particular, the patch currently handles endianness and word size,
which the application inherently can not, because the saved state will
always be an opaque binary blob as far as the users are concerned. They
don't know anything about the internal format of SHA1, nor should they.

--
With regard to the SHA1_State_Version() call: it is a good addition in
the sense of 'SHA1_State_Default_Version()'.

But it does not really replace the 'requested_version' parameter of
SHA1_Save_State() in the current patch: if a distributed process linked
to a future newer version of OpenSSL needs to inter-operate with a
process on another machine linked to an older version of OpenSSL, the
current patch allows the first process to request an older version of
the SHA1 state blob format so that the older OpenSSL will be able to
Load() it.


Regards,
Nanno


__________________________________________________ ____________________
OpenSSL Project http://www.openssl.org
Development Mailing List openssl-dev@openssl.org
Automated List Manager majordomo@openssl.org