Re: [Rpm-maint] [rpm-software-management/rpm] Add OpenSSL crypto backend (#129)
BTW there's a nice little bonus included here: it's *hugely* faster than NSS. On my laptop with signature checking enabled, 'time rpm -qa|wc -l' is 1.9s with NSS, with openssl it's down to 1.3s. Plain digest verification is faster too but not quite so dramatically. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/129#issuecomment-280310030___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add OpenSSL crypto backend (#129)
Closed #129. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/129#event-964758921___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add OpenSSL crypto backend (#129)
Thanks a lot for all the effort that went into this! Merged. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/129#issuecomment-280296406___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add OpenSSL crypto backend (#129)
pmatilai approved this pull request. At this point, ACK for the whole lot. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/129#pullrequestreview-21020680___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add OpenSSL crypto backend (#129)
Conan-Kudo approved this pull request. The changes look good to me for Linux, and it properly fails to allow usage of OpenSSL backend from the configure step on macOS. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/129#pullrequestreview-21022178___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add OpenSSL crypto backend (#129)
My plan was to let @ffesti conduct the actual merge ceremony, web buttons doing stuff to code make me nervous :) -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/129#issuecomment-278659644___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add OpenSSL crypto backend (#129)
@sgallagher Before having @pmatilai merge it, can you squash your fixup commits into the non fixup ones? We don't have autosquashing for them here... -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/129#issuecomment-278658202___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add OpenSSL crypto backend (#129)
Yeah, I'm going to squash them all down to a single commit. One moment. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/129#issuecomment-278658414___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add OpenSSL crypto backend (#129)
Looks good to me, thanks. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/129#issuecomment-278657373___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add OpenSSL crypto backend (#129)
@pmatilai OK, I added some text to the INSTALL file. Please proofread it. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/129#issuecomment-278648068___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add OpenSSL crypto backend (#129)
@sgallagher pushed 1 commit. 907a1a3 Add OpenSSL to INSTALL -- You are receiving this because you are subscribed to this thread. View it on GitHub: https://github.com/rpm-software-management/rpm/pull/129/files/992e94cbacdbafd4b42db24ba275f5bffc77b8f6..907a1a3cdfcd55a85506f0621738bcd82dcaaa9c ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add OpenSSL crypto backend (#129)
More or less. It wouldn't hurt to add a small blurb to INSTALL explaining the new --with-crypto option plus a disclaimer about the openssl license issue. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/129#issuecomment-278565345___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add OpenSSL crypto backend (#129)
@pmatilai @ffesti OK, so is this good to merge at this point? -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/129#issuecomment-278416863___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add OpenSSL crypto backend (#129)
It mostly does. There is no case where it could be triggered in the current code anymore. So I am ok with it as is. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/129#issuecomment-278392261___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add OpenSSL crypto backend (#129)
@t8m Just to confirm, does the current version correctly address your concerns about overwriting the BIGNUM values? -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/129#issuecomment-278388852___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add OpenSSL crypto backend (#129)
t8m approved this pull request. This looks good and it simplifies the patch, there is no point in using OpenSSL allocators for memory that is not later freed inside OpenSSL. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/129#pullrequestreview-20781012___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add OpenSSL crypto backend (#129)
I squashed the patches as I said I was going to and then added a fixup patch for the allocators so it's easy to review. Thanks again to everyone who has chimed in on this. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/129#issuecomment-278347275___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add OpenSSL crypto backend (#129)
Thanks, but you made me realize one problematic thing with that patch: it was now a situation where on OpenSSL 1.0.2, we are allocating with `malloc()` and freeing with `OPENSSL_free()`, which is not correctly paired. We really need to be doing one or the other, and it looks to me like there really isn't any advantage to using the OpenSSL versions where I have been anyway. So I'll strike them completely and simplify the code. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/129#issuecomment-278340621___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add OpenSSL crypto backend (#129)
Just FWIW, my previous comment was written before seeing your latest fixup, so it was more about general principle than the actual fixup. Didn't expect you to fix it that fast :) -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/129#issuecomment-278339101___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add OpenSSL crypto backend (#129)
Hmm, well I was trying to remain consistent, but you may be right; the places where I'm using `OPENSSL_zalloc()` would all be happily satisfied by using regular malloc or the RPM allocators, so I think I'll just rework the patches to use those instead. Also, I noted yesterday that the long series of FIXUP patches I've been tacking on here don't actually apply cleanly. I think I'm going to just squash everything up to this point into a single patch and provide further changes as fixups. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/129#issuecomment-278336521___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add OpenSSL crypto backend (#129)
Mmh, I'd rather not have hundreds of lines of additional configure goo just to check for incompatibilities between a couple of versions of an optional crypto backend, lets try to keep the checks in check, so to speak. But if OPENSSL_malloc() and OPENSSL_free() are not compatible with common malloc() and free() then OPENSSL_zalloc() almost certainly should not be falling back to plain old malloc() + memset(), but OPENSSL_malloc(). The other question is, does openssl actually require the use of its allocators for these purposes? Life would be simpler if not... -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/129#issuecomment-278335434___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add OpenSSL crypto backend (#129)
@sgallagher pushed 1 commit. f59f329 fixup! Add initial OpenSSL support -- You are receiving this because you are subscribed to this thread. View it on GitHub: https://github.com/rpm-software-management/rpm/pull/129/files/f70f78e6242b1a04a49cb6d679e096829ec3f544..f59f329148bfd9dc96d755a3b134cfd898c1e464 ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add OpenSSL crypto backend (#129)
Actually, that warning isn't harmless. I need to add a configure check for that. It's strange, neither `OPENSSL_malloc()` nor `OPENSSL_free()` are listed in the official 1.0.2 [docs](https://www.openssl.org/docs/man1.0.2/crypto/). That's why I created their compatibility functions. I need to be more specific, it seems. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/129#issuecomment-278330830___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add OpenSSL crypto backend (#129)
mlschroe commented on this pull request. > +switch (num) { +case 0: +if (!bn) { +bn = sig->bn = BN_new(); +} +if (!bn) return 1; + +/* Create a BIGNUM from the signature pointer. + Note: this assumes big-endian data as required + by the PGP multiprecision integer format + (RFC4880, Section 3.2) + This will be useful later, as we can + retrieve this value with appropriate + padding. */ +bn = BN_bin2bn(p+2, mlen, bn); +if (!bn) return 1; (That's a different kind of padding, it's how the provided hash is converted to a bignum. See https://tools.ietf.org/html/rfc2313 sections 8.1 and 10.1 for the gory details.) -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/129___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add OpenSSL crypto backend (#129)
Right, padding seems to be working now too and not seeing anything weird in brief testing (throw a big pile of both good and fuzzed packages at it, testsuite passes). I'm getting this (apparently harmless) warning on F25 (so openssl-1.0.2j): ``` digest_openssl.c:26:0: warning: "OPENSSL_free" redefined # define OPENSSL_free free In file included from /usr/include/openssl/bio.h:69:0, from /usr/include/openssl/evp.h:75, from digest_openssl.c:3: /usr/include/openssl/crypto.h:390:0: note: this is the location of the previous definition # define OPENSSL_free(addr) CRYPTO_free(addr) ``` Some other random notes, more on the stylistic side: - You don't need to check for NULL from the rpm allocators xcalloc() and xmalloc() aka rcalloc() and rmalloc() and the like, they exit() on OOM so NULL is not a possible return value. Checking nevertheless is mostly harmless but then it may give somebody the idea that it CAN fail. Regular malloc() etc where used are different of course. - Generally a single point of exit is preferred in functions, in particular those where resources are allocated and need to be freed on errors, pgpVerifySigRSA() is a good example of doing just that. There are some functions - at least rpmDigestDup() and rpmDigestInit() - that would probably be cleaner and simpler if they followed that "goto err" idiom. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/129#issuecomment-278272825___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add OpenSSL crypto backend (#129)
I believe I have addressed all of the review comments thus far. I also ran the code through a Coverity static analysis, which came up clean. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/129#issuecomment-278201836___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add OpenSSL crypto backend (#129)
sgallagher commented on this pull request. > +switch (num) { +case 0: +if (!bn) { +bn = sig->bn = BN_new(); +} +if (!bn) return 1; + +/* Create a BIGNUM from the signature pointer. + Note: this assumes big-endian data as required + by the PGP multiprecision integer format + (RFC4880, Section 3.2) + This will be useful later, as we can + retrieve this value with appropriate + padding. */ +bn = BN_bin2bn(p+2, mlen, bn); +if (!bn) return 1; @mlschroe Thanks, I wasn't entirely sure where to get the correct padding length. I will try that and see if it addresses the example package @pmatilai provided. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/129___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add OpenSSL crypto backend (#129)
sgallagher commented on this pull request. > +int DSA_SIG_set0(DSA_SIG *sig, BIGNUM *r, BIGNUM *s) +{ +if (!sig) return 0; + +if (r) { +sig->r = r; +} + +if (s) { +sig->s = s; +} + +return 1; +} +#endif /* HAVE_DSA_SIG_SET0 */ + OK, I think I finally see what you mean. I was thrown off because the comment was associated with the code where it's assigned to the final object (which can only happen once), but the issue was before this. I will fix it. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/129___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add OpenSSL crypto backend (#129)
sgallagher commented on this pull request. > +EVP_PKEY_free(key->evp_pkey); +key->evp_pkey = NULL; +RSA_free(rsa); +} + +return 1; +} + +static int pgpSetKeyMpiRSA(pgpDigAlg pgpkey, int num, const uint8_t *p) +{ +size_t mlen = pgpMpiLen(p) - 2; +struct pgpDigKeyRSA_s *key = pgpkey->data; + +if(!key) { +key = pgpkey->data = OPENSSL_secure_zalloc(sizeof(*key)); +if (!key) return 1; OK, I will drop the secure memory entirely. I was being overcautious. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/129___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add OpenSSL crypto backend (#129)
sgallagher commented on this pull request. > +switch (num) { +case 0: +if (!bn) { +bn = sig->bn = BN_new(); +} +if (!bn) return 1; + +/* Create a BIGNUM from the signature pointer. + Note: this assumes big-endian data as required + by the PGP multiprecision integer format + (RFC4880, Section 3.2) + This will be useful later, as we can + retrieve this value with appropriate + padding. */ +bn = BN_bin2bn(p+2, mlen, bn); +if (!bn) return 1; Beecrypt is definitely doing some sort of padding: https://github.com/sgallagher/rpm/blob/openssl/rpmio/digest_beecrypt.c#L288 -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/129___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add OpenSSL crypto backend (#129)
pmatilai commented on this pull request. > +switch (num) { +case 0: +if (!bn) { +bn = sig->bn = BN_new(); +} +if (!bn) return 1; + +/* Create a BIGNUM from the signature pointer. + Note: this assumes big-endian data as required + by the PGP multiprecision integer format + (RFC4880, Section 3.2) + This will be useful later, as we can + retrieve this value with appropriate + padding. */ +bn = BN_bin2bn(p+2, mlen, bn); +if (!bn) return 1; Oh, my recollection of beecrypt requiring padding is from some ancient comment by somebody, somewhere, so either I'm misremembering, things have changed since then or the commenter was wrong. Or any combination of the three. Sorry, didn't intend to spread misinformation. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/129___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add OpenSSL crypto backend (#129)
mlschroe commented on this pull request. > +switch (num) { +case 0: +if (!bn) { +bn = sig->bn = BN_new(); +} +if (!bn) return 1; + +/* Create a BIGNUM from the signature pointer. + Note: this assumes big-endian data as required + by the PGP multiprecision integer format + (RFC4880, Section 3.2) + This will be useful later, as we can + retrieve this value with appropriate + padding. */ +bn = BN_bin2bn(p+2, mlen, bn); +if (!bn) return 1; beecrypt needs manual zero-padding? Where in the code would that be? Anyway, the current code doesn't do padding as sig->len is used as length. If openssl needs padding (which it seems to do), you must use EVP_PKEY_size(pkey_ctx) instead. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/129___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add OpenSSL crypto backend (#129)
t8m commented on this pull request. > +switch (num) { +case 0: +if (!bn) { +bn = sig->bn = BN_new(); +} +if (!bn) return 1; + +/* Create a BIGNUM from the signature pointer. + Note: this assumes big-endian data as required + by the PGP multiprecision integer format + (RFC4880, Section 3.2) + This will be useful later, as we can + retrieve this value with appropriate + padding. */ +bn = BN_bin2bn(p+2, mlen, bn); +if (!bn) return 1; I'd probably try removing the conversion to BIGNUM and back if it solves the padding problem. If not then it needs to be investigated how to handle the padding properly. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/129___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add OpenSSL crypto backend (#129)
t8m commented on this pull request. > +int DSA_SIG_set0(DSA_SIG *sig, BIGNUM *r, BIGNUM *s) +{ +if (!sig) return 0; + +if (r) { +sig->r = r; +} + +if (s) { +sig->s = s; +} + +return 1; +} +#endif /* HAVE_DSA_SIG_SET0 */ + No, I mean when you assign the sig->r values you overwrite the previous values that could be there which means the previous value will be leaked. If rpm guarantees that pgpSet(Key|Sig)Mpi* functions won't be called on the same key/sig twice with the same index it is not strictly necessary to handle this, because you will have the previous value always set to NULL. But I do not know whether rpm really guarantees that when reading some malformed signature/key data. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/129___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add OpenSSL crypto backend (#129)
t8m commented on this pull request. > +EVP_PKEY_free(key->evp_pkey); +key->evp_pkey = NULL; +RSA_free(rsa); +} + +return 1; +} + +static int pgpSetKeyMpiRSA(pgpDigAlg pgpkey, int num, const uint8_t *p) +{ +size_t mlen = pgpMpiLen(p) - 2; +struct pgpDigKeyRSA_s *key = pgpkey->data; + +if(!key) { +key = pgpkey->data = OPENSSL_secure_zalloc(sizeof(*key)); +if (!key) return 1; As Panu says - as there are no secret/private key operations at all, there is no point in using the secure memory in the patch at all. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/129___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add OpenSSL crypto backend (#129)
pmatilai commented on this pull request. > +EVP_PKEY_free(key->evp_pkey); +key->evp_pkey = NULL; +RSA_free(rsa); +} + +return 1; +} + +static int pgpSetKeyMpiRSA(pgpDigAlg pgpkey, int num, const uint8_t *p) +{ +size_t mlen = pgpMpiLen(p) - 2; +struct pgpDigKeyRSA_s *key = pgpkey->data; + +if(!key) { +key = pgpkey->data = OPENSSL_secure_zalloc(sizeof(*key)); +if (!key) return 1; FWIW, rpm only ever deals with public keys, signing is done with gpg. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/129___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add OpenSSL crypto backend (#129)
pmatilai commented on this pull request. > +switch (num) { +case 0: +if (!bn) { +bn = sig->bn = BN_new(); +} +if (!bn) return 1; + +/* Create a BIGNUM from the signature pointer. + Note: this assumes big-endian data as required + by the PGP multiprecision integer format + (RFC4880, Section 3.2) + This will be useful later, as we can + retrieve this value with appropriate + padding. */ +bn = BN_bin2bn(p+2, mlen, bn); +if (!bn) return 1; Oh and FWIW, padding is a relatively rare condition. On my F25 laptop there are 3366 packages installed and 19 of them are "padding case", one of them being (funny coincidence) that nss-softokn-3.28.1-1.0.fc25.x86_64. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/129___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add OpenSSL crypto backend (#129)
pmatilai commented on this pull request. > +switch (num) { +case 0: +if (!bn) { +bn = sig->bn = BN_new(); +} +if (!bn) return 1; + +/* Create a BIGNUM from the signature pointer. + Note: this assumes big-endian data as required + by the PGP multiprecision integer format + (RFC4880, Section 3.2) + This will be useful later, as we can + retrieve this value with appropriate + padding. */ +bn = BN_bin2bn(p+2, mlen, bn); +if (!bn) return 1; NSS and beecrypt do need manual zero-padding, I don't know about openssl but maybe its smarter. What I can tell you that the current code doesn't handle padding correctly. Here's a reproducer package from the original NSS bug: https://dl.fedoraproject.org/pub/archive/fedora/linux/releases/11/Fedora/x86_64/os/Packages/libuser-python-0.56.9-3.x86_64.rpm but if you prefer a newer one, nss-softokn-3.28.1-1.0.fc25.x86_64 also requires zero-padding (with NSS) and fails with the current openssl implementation. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/129___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add OpenSSL crypto backend (#129)
sgallagher commented on this pull request. > +switch (num) { +case 0: +if (!bn) { +bn = sig->bn = BN_new(); +} +if (!bn) return 1; + +/* Create a BIGNUM from the signature pointer. + Note: this assumes big-endian data as required + by the PGP multiprecision integer format + (RFC4880, Section 3.2) + This will be useful later, as we can + retrieve this value with appropriate + padding. */ +bn = BN_bin2bn(p+2, mlen, bn); +if (!bn) return 1; Mostly I was copying that from the beecrypt and NSS implementations, where the signature was padded before comparison. Are you saying I can skip the padding entirely at https://github.com/rpm-software-management/rpm/pull/129/files/a7bc1e416e2f0f193be63317701bd3b18e2934b6#diff-763b93abc6d6f5549dafd08d9d07R523 ? This is definitely a place where I don't actually know what I'm doing... -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/129___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add OpenSSL crypto backend (#129)
sgallagher commented on this pull request. > +EVP_PKEY_free(key->evp_pkey); +key->evp_pkey = NULL; +RSA_free(rsa); +} + +return 1; +} + +static int pgpSetKeyMpiRSA(pgpDigAlg pgpkey, int num, const uint8_t *p) +{ +size_t mlen = pgpMpiLen(p) - 2; +struct pgpDigKeyRSA_s *key = pgpkey->data; + +if(!key) { +key = pgpkey->data = OPENSSL_secure_zalloc(sizeof(*key)); +if (!key) return 1; I figured that, given that RPM interaction is a computationally-rare event, it was easier on me to just assume everything should be secure-allocated rather than trying to figure out when it was appropriate and when it was safe to skip it. If you are telling me that I should just not use it at all in this code, I'll drop it. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/129___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add OpenSSL crypto backend (#129)
sgallagher commented on this pull request. > +int DSA_SIG_set0(DSA_SIG *sig, BIGNUM *r, BIGNUM *s) +{ +if (!sig) return 0; + +if (r) { +sig->r = r; +} + +if (s) { +sig->s = s; +} + +return 1; +} +#endif /* HAVE_DSA_SIG_SET0 */ + I'm not really sure what you mean by "release" them? The way this function is expected to work is that it updates any values that aren't NULL and does not change any that are. Or at least, that's the way that the OpenSSL 1.1.0 version implements them. Also, these are "set" not "assigned", so it's not expected that this function should be owning their memory management either. Meaning I shouldn't be calling BN_free() before assigning the new value. Am I way off-base here? -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/129___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add OpenSSL crypto backend (#129)
t8m commented on this pull request. > +int DSA_SIG_set0(DSA_SIG *sig, BIGNUM *r, BIGNUM *s) +{ +if (!sig) return 0; + +if (r) { +sig->r = r; +} + +if (s) { +sig->s = s; +} + +return 1; +} +#endif /* HAVE_DSA_SIG_SET0 */ + In above functions you do not release previous component BIGNUMs before assigning new values. You should add that. > +struct pgpDigKeyRSA_s *key = pgpkey->data; + +if(!key) { +key = pgpkey->data = OPENSSL_secure_zalloc(sizeof(*key)); +if (!key) return 1; +} + +switch(num) { +case 0: +/* Modulus */ +key->nbytes = mlen; +/* Create a BIGNUM from the pointer. + Note: this assumes big-endian data. This + could be a bug on little-endian systems. */ +key->n = BN_bin2bn(p+2, mlen, NULL); +if (!key->n) return 1; You do not handle a theoretical double call with the same num on the same pgpkey - it will leak memory. I do not know if this can happen (for example with malformed data) though. > +EVP_PKEY_free(key->evp_pkey); +key->evp_pkey = NULL; +RSA_free(rsa); +} + +return 1; +} + +static int pgpSetKeyMpiRSA(pgpDigAlg pgpkey, int num, const uint8_t *p) +{ +size_t mlen = pgpMpiLen(p) - 2; +struct pgpDigKeyRSA_s *key = pgpkey->data; + +if(!key) { +key = pgpkey->data = OPENSSL_secure_zalloc(sizeof(*key)); +if (!key) return 1; As the keys are public-only is it worth it using the secure memory allocator? > +switch (num) { +case 0: +if (!bn) { +bn = sig->bn = BN_new(); +} +if (!bn) return 1; + +/* Create a BIGNUM from the signature pointer. + Note: this assumes big-endian data as required + by the PGP multiprecision integer format + (RFC4880, Section 3.2) + This will be useful later, as we can + retrieve this value with appropriate + padding. */ +bn = BN_bin2bn(p+2, mlen, bn); +if (!bn) return 1; It's unclear to me why do you convert the RSA signature to BIGNUM and then back when calling the verification. Wouldn't it be better to just store the data directly? > +BIGNUM *r; +BIGNUM *s; + +DSA_SIG *dsa_sig; +}; + +static int constructDSASignature(struct pgpDigSigDSA_s *sig) +{ +int rc; + +if (sig->dsa_sig) { +/* We've already constructed it, so just reuse it */ +return 1; +} + +/* Create the DSA key */ Nitpick: you mean 'Create the DSA signature' here -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/129#pullrequestreview-20301305___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add OpenSSL crypto backend (#129)
@t8m Sorry, could you be more specific? I'm not sure which functions are risky or which references need to be freed. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/129#issuecomment-277728498___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add OpenSSL crypto backend (#129)
t8m requested changes on this pull request. The various _set0_ functions you add for compatibility with OpenSSL 1.0.2 do not properly handle multiple calls of the same function on the object - the old references are not freed. I understand that in the current patch this probably isn't a problem but it might happen in future. It might be better to release the old references before assigning new ones. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/129#pullrequestreview-20297097___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add OpenSSL crypto backend (#129)
@Conan-Kudo I just pushed a couple additional configure.ac changes that should ensure that it's using at least OpenSSL 1.0 -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/129#issuecomment-277719485___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add OpenSSL crypto backend (#129)
@sgallagher pushed 2 commits. de07401 fixup! Add initial OpenSSL support a7bc1e4 fixup! Add compatibility layer for OpenSSL 1.0.2 -- You are receiving this because you are subscribed to this thread. View it on GitHub: https://github.com/rpm-software-management/rpm/pull/129/files/d8e6b09913a4b383ad0dc0156e2539083f1fcb39..a7bc1e416e2f0f193be63317701bd3b18e2934b6 ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add OpenSSL crypto backend (#129)
@sgallagher Yes. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/129#issuecomment-277678714___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add OpenSSL crypto backend (#129)
@Conan-Kudo Thanks, I'll see if I can make the configure checks a little more strict. Just to confirm: those other two bullet points were you asserting that things worked correctly, right? (It's a little confusing when corrective feedback is in the same list). -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/129#issuecomment-277678554___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add OpenSSL crypto backend (#129)
So, I've had a chance to test this on MacOS... A few points of feedback: * The configure script _does not_ catch that OpenSSL 0.9.8zh is incompatible, and the build fails due to undeclared things (`EVP_PKEY_CTX` and `pkey_ctx` were mentioned before it bailed out). It needs some better checks to ensure it only works with compatible OpenSSL versions. * It does build properly with the new flag when I select `beecrypt` (as I have already built beecrypt for RPM before). * The `--with-beecrypt` error returns a useful error, telling the user to switch to `--with-crypto=beecrypt`. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/129#issuecomment-277547616___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add OpenSSL crypto backend (#129)
OK, new version of my patches, now supporting both OpenSSL 1.1.0 _and_ 1.0.2 I implemented `--with-crypto=CRYPTO_LIB` and made `--with-beecrypt` report an error. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/129#issuecomment-275119921___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add OpenSSL crypto backend (#129)
I don't have any strong opinions on that, but in general being explicit about failures never hurts. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/129#issuecomment-273793208___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add OpenSSL crypto backend (#129)
OK, I will make that change. Would you like me to include a canary for --with-nss to fail the build if it is specified? That might make the transition easier. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/129#issuecomment-273790373___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add OpenSSL crypto backend (#129)
Yeah, --with-crypto=(beecrypt|nss|openssl|...) would be nice. Compatibility in the build system is not particularly important - rpm is not something joerandom is likely to build anyway and we expect people upgrading rpm for distros to pay attention. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/129#issuecomment-273789370___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add OpenSSL crypto backend (#129)
@Conan-Kudo I'm willing to do that, if that's what the RPM upstream maintainers would prefer. I'd like to head from @pmatilai and @ffesti before I do, though. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/129#issuecomment-273785627___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add OpenSSL crypto backend (#129)
Conan-Kudo requested changes on this pull request. I'd prefer to have autotools change so that it would detect and auto-select based on what's available. Since the usage of these are mutually exclusive, it would also make sense to change the switches so that it reflects this more correct behavior (maybe `--with-digest-backend=(nss|openssl|beecrypt)`). While I get the idea of backward compatibility, I do not think it is as important with build system changes, as the changes can just be documented so that as packagers upgrade RPM, they can change accordingly. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/129#pullrequestreview-17312872___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add OpenSSL crypto backend (#129)
@sgallagher pushed 1 commit. 910c59a fixup! Add initial OpenSSL support -- You are receiving this because you are subscribed to this thread. View it on GitHub: https://github.com/rpm-software-management/rpm/pull/129/files/123fc576f016807953512765819574329f8fafc8..910c59af3bb5bcbe1a93ca019846a9837bfe5b53 ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add OpenSSL crypto backend (#129)
t8m commented on this pull request. > +} + +int rpmDigestFinal(DIGEST_CTX ctx, void ** datap, size_t *lenp, int asAscii) +{ +int ret; +unsigned char *digest = NULL; +unsigned int digestlen; + +if (ctx == NULL) return -1; + +digestlen = EVP_MD_CTX_size(ctx->md_ctx); +digest = xcalloc(digestlen, sizeof(*digest)); +if (digest == NULL) return -1; + +ret = EVP_DigestFinal_ex(ctx->md_ctx, digest, &digestlen); +if (ret != 1) goto done; Yes, in most places there might be both 0 and -1 returns where 0 usually indicates "regular" failures such as hash or signature verification failure and -1 some kind of "fatal" error - such as unknown hash algorithm or memory allocation error. So the safest thing for functions returning 1 as success is to use `ret != 1` or even `ret <= 0` as test for failure. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/129___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add OpenSSL crypto backend (#129)
sgallagher commented on this pull request. > +else { +/* ASCII requested */ +if (lenp) *lenp = (2*digestlen) + 1; +if (datap) { +const uint8_t * s = (const uint8_t *) digest; +*datap = pgpHexStr(s, digestlen); +} +} + +ret = 1; + +done: +if (digest) { +/* Zero the digest, just in case it's sensitive */ +memset(digest, 0, digestlen); + free(digest); Somehow a tab snuck in there. Must have been originally some copy-pasted code because I am tab-averse. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/129___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add OpenSSL crypto backend (#129)
sgallagher commented on this pull request. > +WITH_OPENSSL_INCLUDE= +WITH_OPENSSL_LIB= +if test "$with_openssl" = yes; then +# If we have pkgconfig make sure CPPFLAGS are setup correctly for the OpenSSL +# -I include path. +AC_PATH_TOOL([PKGCONFIG], [pkg-config], [no], [$PATH:/usr/bin:/usr/local/bin]) +if test "x$PKGCONFIG" != "xno"; then + CPPFLAGS="$CPPFLAGS $($PKGCONFIG --cflags libcrypto)" +fi + +AC_CHECK_HEADERS([openssl/evp.h], [], [ + AC_MSG_ERROR([missing required OpenSSL header]) +]) + +AC_CHECK_LIB(crypto, EVP_DigestInit_ex, [ + WITH_OPENSSL_LIB=$($PKGCONFIG --libs libcrypto) Yeah, that's a bug. I'll fix it. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/129___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add OpenSSL crypto backend (#129)
sgallagher commented on this pull request. > +} + +int rpmDigestFinal(DIGEST_CTX ctx, void ** datap, size_t *lenp, int asAscii) +{ +int ret; +unsigned char *digest = NULL; +unsigned int digestlen; + +if (ctx == NULL) return -1; + +digestlen = EVP_MD_CTX_size(ctx->md_ctx); +digest = xcalloc(digestlen, sizeof(*digest)); +if (digest == NULL) return -1; + +ret = EVP_DigestFinal_ex(ctx->md_ctx, digest, &digestlen); +if (ret != 1) goto done; In this case, that would work, but there are many places in OpenSSL where success == 1 and any other value indicates an error. I kept it the same here for consistency. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/129___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add OpenSSL crypto backend (#129)
ignatenkobrain commented on this pull request. > +} + +int rpmDigestFinal(DIGEST_CTX ctx, void ** datap, size_t *lenp, int asAscii) +{ +int ret; +unsigned char *digest = NULL; +unsigned int digestlen; + +if (ctx == NULL) return -1; + +digestlen = EVP_MD_CTX_size(ctx->md_ctx); +digest = xcalloc(digestlen, sizeof(*digest)); +if (digest == NULL) return -1; + +ret = EVP_DigestFinal_ex(ctx->md_ctx, digest, &digestlen); +if (ret != 1) goto done; shouldn't comparison be `if (!ret)`? I have not found much docs about return values, but I guess it is supposed to be used in such manner) -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/129#pullrequestreview-17257705___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add OpenSSL crypto backend (#129)
ignatenkobrain commented on this pull request. > +else { +/* ASCII requested */ +if (lenp) *lenp = (2*digestlen) + 1; +if (datap) { +const uint8_t * s = (const uint8_t *) digest; +*datap = pgpHexStr(s, digestlen); +} +} + +ret = 1; + +done: +if (digest) { +/* Zero the digest, just in case it's sensitive */ +memset(digest, 0, digestlen); + free(digest); I think something went wrong with style ;) -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/129#pullrequestreview-17257123___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add OpenSSL crypto backend (#129)
ignatenkobrain commented on this pull request. > +WITH_OPENSSL_INCLUDE= +WITH_OPENSSL_LIB= +if test "$with_openssl" = yes; then +# If we have pkgconfig make sure CPPFLAGS are setup correctly for the OpenSSL +# -I include path. +AC_PATH_TOOL([PKGCONFIG], [pkg-config], [no], [$PATH:/usr/bin:/usr/local/bin]) +if test "x$PKGCONFIG" != "xno"; then + CPPFLAGS="$CPPFLAGS $($PKGCONFIG --cflags libcrypto)" +fi + +AC_CHECK_HEADERS([openssl/evp.h], [], [ + AC_MSG_ERROR([missing required OpenSSL header]) +]) + +AC_CHECK_LIB(crypto, EVP_DigestInit_ex, [ + WITH_OPENSSL_LIB=$($PKGCONFIG --libs libcrypto) I think this will crash if --with-openssl is set, but pkg-config is not found -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/129#pullrequestreview-17256328___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint