@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
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
+
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
@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:
@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:
@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:
@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:
@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:
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
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
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
+
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:
@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:
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
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
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
@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
@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:
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:
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);
+
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],
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;
These patches add support for building RPM against OpenSSL 1.1.0 to provide
hashing and signature functionality. It is enabled by passing `--with-openssl`
to `configure`. (For backwards compatibility, it retains the use of
`--with[out]-nss` as a toggle between beecrypt and nss).
I have tested
I just opened PR #129
Reviews and concerns are most welcome.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
The licensing situation is solvable:
https://fedoraproject.org/wiki/Licensing:FAQ?rd=Licensing/FAQ#What.27s_the_deal_with_the_OpenSSL_license.3F
Short version: In Fedora, the legal team has come to the determination that the
usage is within the spirit of the licenses, if not its specific
25 matches
Mail list logo
