linux-integrity.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* ima-evm-utils:  library version
@ 2019-07-24 12:51 Mimi Zohar
  2019-07-24 17:28 ` Vitaly Chikunov
  0 siblings, 1 reply; 6+ messages in thread
From: Mimi Zohar @ 2019-07-24 12:51 UTC (permalink / raw)
  To: Petr Vorel, BrunoE.O.Meneguele, Dmitry Eremin-Solenikov, Vitaly Chikunov
  Cc: linux-integrity

Hi -

In preparing the ima-evm-utils v1.2 release, I noticed that the
library version was never updated.  It is still "0.0.0".  Should I set
it to something?  If so, what versioning scheme do you recommend -
using the libtool current[:revision[:age]], prepending the release
version on the .so, or suffixing the release version on the .so?

The other option is to leave the version as 0.0.0 and let the distro
package maintainers deal with it.

Posting a patch that sets the library version would be most welcome.

thanks!

Mimi


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: ima-evm-utils:  library version
  2019-07-24 12:51 ima-evm-utils: library version Mimi Zohar
@ 2019-07-24 17:28 ` Vitaly Chikunov
  2019-07-24 18:04   ` Bruno E. O. Meneguele
  2019-07-24 19:17   ` Vitaly Chikunov
  0 siblings, 2 replies; 6+ messages in thread
From: Vitaly Chikunov @ 2019-07-24 17:28 UTC (permalink / raw)
  To: Mimi Zohar
  Cc: Petr Vorel, BrunoE.O.Meneguele, Dmitry Eremin-Solenikov, linux-integrity

Mimi,

On Wed, Jul 24, 2019 at 08:51:38AM -0400, Mimi Zohar wrote:
> 
> In preparing the ima-evm-utils v1.2 release, I noticed that the
> library version was never updated.  It is still "0.0.0".  Should I set
> it to something?  If so, what versioning scheme do you recommend -
> using the libtool current[:revision[:age]], prepending the release
> version on the .so, or suffixing the release version on the .so?

libtool rules should be followed:

  https://www.gnu.org/software/libtool/manual/html_node/Updating-version-info.html

I think you should change 0.0.0 to 1.0.0 just before release. Since we are
changed ABI of calc_keyid_v2 (RSA *key -> EVP_PKEY *pkey). (There is also
changes to read_pub_pkey and get_filesize.)

> The other option is to leave the version as 0.0.0 and let the distro
> package maintainers deal with it.

I think you should update it properly.

> Posting a patch that sets the library version would be most welcome.

diff --git a/src/Makefile.am b/src/Makefile.am
index 9c037e2..b794c50 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -4,7 +4,7 @@ libimaevm_la_SOURCES = libimaevm.c
 libimaevm_la_CPPFLAGS = $(AM_CPPFLAGS) $(LIBCRYPTO_CFLAGS)
 # current[:revision[:age]]
 # result: [current-age].age.revision
-libimaevm_la_LDFLAGS = -version-info 0:0:0
+libimaevm_la_LDFLAGS = -version-info 1:0:0
 libimaevm_la_LIBADD =  $(LIBCRYPTO_LIBS)

Thanks,

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: ima-evm-utils:  library version
  2019-07-24 17:28 ` Vitaly Chikunov
@ 2019-07-24 18:04   ` Bruno E. O. Meneguele
  2019-07-25  0:36     ` Mimi Zohar
  2019-07-24 19:17   ` Vitaly Chikunov
  1 sibling, 1 reply; 6+ messages in thread
From: Bruno E. O. Meneguele @ 2019-07-24 18:04 UTC (permalink / raw)
  To: Mimi Zohar, Petr Vorel, Dmitry Eremin-Solenikov, linux-integrity

[-- Attachment #1: Type: text/plain, Size: 2310 bytes --]

Hi Mimi,

On Wed, Jul 24, 2019 at 08:28:01PM +0300, Vitaly Chikunov wrote:
> Mimi,
> 
> On Wed, Jul 24, 2019 at 08:51:38AM -0400, Mimi Zohar wrote:
> > 
> > In preparing the ima-evm-utils v1.2 release, I noticed that the
> > library version was never updated.  It is still "0.0.0".  Should I set
> > it to something?  If so, what versioning scheme do you recommend -
> > using the libtool current[:revision[:age]], prepending the release
> > version on the .so, or suffixing the release version on the .so?
> 
> libtool rules should be followed:
> 
>   https://www.gnu.org/software/libtool/manual/html_node/Updating-version-info.html
> 
> I think you should change 0.0.0 to 1.0.0 just before release. Since we are
> changed ABI of calc_keyid_v2 (RSA *key -> EVP_PKEY *pkey). (There is also
> changes to read_pub_pkey and get_filesize.)
> 

Yep, I agree with that: libtool scheme for sure, thus the linker can
easily handle the dependency without the need for manual relinkage from
our users whenever possible, i.e. "current" and "age" getting increased
by 1 means the interface is backward compatible to the last release and
don't require a new linkage step of user's tool.

> > The other option is to leave the version as 0.0.0 and let the distro
> > package maintainers deal with it.
> 
> I think you should update it properly.
> 

Not every packager is aware of tool's internals/source code, and let
them face possible user crashes due to "invalid interface calls" is
pretty bad to the tool community itself.

> > Posting a patch that sets the library version would be most welcome.
> 
> diff --git a/src/Makefile.am b/src/Makefile.am
> index 9c037e2..b794c50 100644
> --- a/src/Makefile.am
> +++ b/src/Makefile.am
> @@ -4,7 +4,7 @@ libimaevm_la_SOURCES = libimaevm.c
>  libimaevm_la_CPPFLAGS = $(AM_CPPFLAGS) $(LIBCRYPTO_CFLAGS)
>  # current[:revision[:age]]
>  # result: [current-age].age.revision
> -libimaevm_la_LDFLAGS = -version-info 0:0:0
> +libimaevm_la_LDFLAGS = -version-info 1:0:0
>  libimaevm_la_LIBADD =  $(LIBCRYPTO_LIBS)
> 
> Thanks,

And I also agree with his patch, changing -version-info to 1:0:0,
bumping "current" number, since the interface was indeed changed since
v1.1 release of ima-evm-utils.

Thanks for catching that :))

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: ima-evm-utils:  library version
  2019-07-24 17:28 ` Vitaly Chikunov
  2019-07-24 18:04   ` Bruno E. O. Meneguele
@ 2019-07-24 19:17   ` Vitaly Chikunov
  2019-07-24 22:27     ` Mimi Zohar
  1 sibling, 1 reply; 6+ messages in thread
From: Vitaly Chikunov @ 2019-07-24 19:17 UTC (permalink / raw)
  To: Mimi Zohar, Petr Vorel, BrunoE.O.Meneguele,
	Dmitry Eremin-Solenikov, linux-integrity

Btw,

On Wed, Jul 24, 2019 at 08:28:01PM +0300, Vitaly Chikunov wrote:
> On Wed, Jul 24, 2019 at 08:51:38AM -0400, Mimi Zohar wrote:
> > 
> > In preparing the ima-evm-utils v1.2 release, I noticed that the
> > library version was never updated.  It is still "0.0.0".  Should I set
> > it to something?  If so, what versioning scheme do you recommend -
> > using the libtool current[:revision[:age]], prepending the release
> > version on the .so, or suffixing the release version on the .so?
> 
> libtool rules should be followed:
> 
>   https://www.gnu.org/software/libtool/manual/html_node/Updating-version-info.html
> 
> I think you should change 0.0.0 to 1.0.0 just before release. Since we are
> changed ABI of calc_keyid_v2 (RSA *key -> EVP_PKEY *pkey). (There is also
> changes to read_pub_pkey and get_filesize.)

Speaking about ABI:

   src/ima-evm-utils (tests)$ readelf --dyn-syms src/.libs/libimaevm.so | egrep -vw 'UND|_edata|_fini|_init|_end|__bss_start'
   Num:    Value          Size Type    Bind   Vis      Ndx Name
    74: 0000000000003bf6  1047 FUNC    GLOBAL DEFAULT   12 sign_hash_v1
    75: 0000000000002c5b   783 FUNC    GLOBAL DEFAULT   12 read_pub_pkey
    77: 000000000000400d   836 FUNC    GLOBAL DEFAULT   12 sign_hash_v2
    78: 0000000000004351    56 FUNC    GLOBAL DEFAULT   12 sign_hash
    81: 0000000000003795   198 FUNC    GLOBAL DEFAULT   12 key2bin
    83: 00000000000025f3  1640 FUNC    GLOBAL DEFAULT   12 ima_calc_hash
    84: 0000000000003204   232 FUNC    GLOBAL DEFAULT   12 get_hash_algo
    85: 00000000000032ec   836 FUNC    GLOBAL DEFAULT   12 verify_hash
    87: 0000000000003630   357 FUNC    GLOBAL DEFAULT   12 ima_verify_signature
    88: 000000000000385b   204 FUNC    GLOBAL DEFAULT   12 calc_keyid_v1
    89: 0000000000205d20   144 OBJECT  GLOBAL DEFAULT   20 hash_algo_name
    90: 0000000000003927   308 FUNC    GLOBAL DEFAULT   12 calc_keyid_v2
    91: 0000000000002566    34 FUNC    GLOBAL DEFAULT   12 dump
    92: 0000000000003a5b   411 FUNC    GLOBAL DEFAULT   12 init_public_keys
    93: 0000000000205c80   160 OBJECT  GLOBAL DEFAULT   20 pkey_hash_algo
    94: 00000000002062c0    32 OBJECT  GLOBAL DEFAULT   24 params
    95: 0000000000205be0   160 OBJECT  GLOBAL DEFAULT   20 pkey_hash_algo_kern
    96: 0000000000002588   107 FUNC    GLOBAL DEFAULT   12 get_hash_algo_by_id
    97: 0000000000002f6a   113 FUNC    GLOBAL DEFAULT   12 read_pub_key
    98: 0000000000002509    93 FUNC    GLOBAL DEFAULT   12 do_dump

This looks not very good. Names like `dump', `do_dump', `params' aren't good
for public ABI. And should be prefixed, or removed. Probably, some (or all)
others too. Prefix could be something like `ima_', like in `ima_calc_hash'.

Thanks,

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: ima-evm-utils:  library version
  2019-07-24 19:17   ` Vitaly Chikunov
@ 2019-07-24 22:27     ` Mimi Zohar
  0 siblings, 0 replies; 6+ messages in thread
From: Mimi Zohar @ 2019-07-24 22:27 UTC (permalink / raw)
  To: Vitaly Chikunov, Petr Vorel, BrunoE.O.Meneguele,
	Dmitry Eremin-Solenikov, linux-integrity

On Wed, 2019-07-24 at 22:17 +0300, Vitaly Chikunov wrote:
> Btw,
> 
> On Wed, Jul 24, 2019 at 08:28:01PM +0300, Vitaly Chikunov wrote:
> > On Wed, Jul 24, 2019 at 08:51:38AM -0400, Mimi Zohar wrote:
> > > 
> > > In preparing the ima-evm-utils v1.2 release, I noticed that the
> > > library version was never updated.  It is still "0.0.0".  Should I set
> > > it to something?  If so, what versioning scheme do you recommend -
> > > using the libtool current[:revision[:age]], prepending the release
> > > version on the .so, or suffixing the release version on the .so?
> > 
> > libtool rules should be followed:
> > 
> >   https://www.gnu.org/software/libtool/manual/html_node/Updating-version-info.html
> > 
> > I think you should change 0.0.0 to 1.0.0 just before release. Since we are
> > changed ABI of calc_keyid_v2 (RSA *key -> EVP_PKEY *pkey). (There is also
> > changes to read_pub_pkey and get_filesize.)
> 
> Speaking about ABI:
> 
>    src/ima-evm-utils (tests)$ readelf --dyn-syms src/.libs/libimaevm.so | egrep -vw 'UND|_edata|_fini|_init|_end|__bss_start'
>    Num:    Value          Size Type    Bind   Vis      Ndx Name
>     74: 0000000000003bf6  1047 FUNC    GLOBAL DEFAULT   12 sign_hash_v1
>     75: 0000000000002c5b   783 FUNC    GLOBAL DEFAULT   12 read_pub_pkey
>     77: 000000000000400d   836 FUNC    GLOBAL DEFAULT   12 sign_hash_v2
>     78: 0000000000004351    56 FUNC    GLOBAL DEFAULT   12 sign_hash
>     81: 0000000000003795   198 FUNC    GLOBAL DEFAULT   12 key2bin
>     83: 00000000000025f3  1640 FUNC    GLOBAL DEFAULT   12 ima_calc_hash
>     84: 0000000000003204   232 FUNC    GLOBAL DEFAULT   12 get_hash_algo
>     85: 00000000000032ec   836 FUNC    GLOBAL DEFAULT   12 verify_hash
>     87: 0000000000003630   357 FUNC    GLOBAL DEFAULT   12 ima_verify_signature
>     88: 000000000000385b   204 FUNC    GLOBAL DEFAULT   12 calc_keyid_v1
>     89: 0000000000205d20   144 OBJECT  GLOBAL DEFAULT   20 hash_algo_name
>     90: 0000000000003927   308 FUNC    GLOBAL DEFAULT   12 calc_keyid_v2
>     91: 0000000000002566    34 FUNC    GLOBAL DEFAULT   12 dump
>     92: 0000000000003a5b   411 FUNC    GLOBAL DEFAULT   12 init_public_keys
>     93: 0000000000205c80   160 OBJECT  GLOBAL DEFAULT   20 pkey_hash_algo
>     94: 00000000002062c0    32 OBJECT  GLOBAL DEFAULT   24 params
>     95: 0000000000205be0   160 OBJECT  GLOBAL DEFAULT   20 pkey_hash_algo_kern
>     96: 0000000000002588   107 FUNC    GLOBAL DEFAULT   12 get_hash_algo_by_id
>     97: 0000000000002f6a   113 FUNC    GLOBAL DEFAULT   12 read_pub_key
>     98: 0000000000002509    93 FUNC    GLOBAL DEFAULT   12 do_dump
> 
> This looks not very good. Names like `dump', `do_dump', `params' aren't good
> for public ABI. And should be prefixed, or removed. Probably, some (or all)
> others too. Prefix could be something like `ima_', like in `ima_calc_hash'.

At least sign_hash_v1() and sign_hash_v2() can be addressed by making
them static.  Looking to see if there are others.

Mimi


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: ima-evm-utils:  library version
  2019-07-24 18:04   ` Bruno E. O. Meneguele
@ 2019-07-25  0:36     ` Mimi Zohar
  0 siblings, 0 replies; 6+ messages in thread
From: Mimi Zohar @ 2019-07-25  0:36 UTC (permalink / raw)
  To: Bruno E. O. Meneguele, Petr Vorel, Dmitry Eremin-Solenikov,
	linux-integrity

On Wed, 2019-07-24 at 15:04 -0300, Bruno E. O. Meneguele wrote:
> On Wed, Jul 24, 2019 at 08:28:01PM +0300, Vitaly Chikunov wrote:

> > diff --git a/src/Makefile.am b/src/Makefile.am
> > index 9c037e2..b794c50 100644
> > --- a/src/Makefile.am
> > +++ b/src/Makefile.am
> > @@ -4,7 +4,7 @@ libimaevm_la_SOURCES = libimaevm.c
> >  libimaevm_la_CPPFLAGS = $(AM_CPPFLAGS) $(LIBCRYPTO_CFLAGS)
> >  # current[:revision[:age]]
> >  # result: [current-age].age.revision
> > -libimaevm_la_LDFLAGS = -version-info 0:0:0
> > +libimaevm_la_LDFLAGS = -version-info 1:0:0
> >  libimaevm_la_LIBADD =  $(LIBCRYPTO_LIBS)
> > 
> > Thanks,
> 
> And I also agree with his patch, changing -version-info to 1:0:0,
> bumping "current" number, since the interface was indeed changed since
> v1.1 release of ima-evm-utils.
> 
> Thanks for catching that :))

Thanks!  We'll use the libtool versioning.

Mimi


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2019-07-25  0:37 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-24 12:51 ima-evm-utils: library version Mimi Zohar
2019-07-24 17:28 ` Vitaly Chikunov
2019-07-24 18:04   ` Bruno E. O. Meneguele
2019-07-25  0:36     ` Mimi Zohar
2019-07-24 19:17   ` Vitaly Chikunov
2019-07-24 22:27     ` Mimi Zohar

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).