Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cades #206

Closed
wants to merge 6 commits into from
Closed

Cades #206

wants to merge 6 commits into from

Conversation

opensignature
Copy link
Contributor

No description provided.

@BenBE
Copy link
Contributor

BenBE commented Jan 4, 2015

The loop source now looks better.

Yet SHA-256 isn't the only hash supported for that field if I see the definition in RFC 5126 sections 5.7.3.3 and 5.8.1 for OtherHash correctly.

@opensignature
Copy link
Contributor Author

thanks Benny,

what other algorithms you suggest to include, in addition sha256, sha512 and whirlpool?
Do you think it is correct to add hash here crypto/x509v3/v3_purp.c ?
ifndef OPENSSL_NO_SHA256
X509_digest + (x, EVP_sha256 (), x-> sha256_hash, NULL);
endif

Antonio

@BenBE
Copy link
Contributor

BenBE commented Jan 6, 2015

Hi Antonio,

Am 06.01.2015 um 11:46 schrieb opensignature:

thanks Benny,

what other algorithms you suggest to include, in addition sha256, sha512 and whirlpool?
I think SHA2-384 and SHA3-* (Keccak) should join the pool too. SHA2-224
is up for discussion, but as outlined below I'd prefer to skip it.

While one might argue that any hash function actually could be placed
here (including MD4 and MD5) it won't do much good in the context of RFC
5126. Thus given a baseline security margin of 128 bit I'd exclude all
the short ones (MD4, MD5, RIPE-MD160).

Based on that one could discuss if generally allowing all hashes
supported by OpenSSL starting at 256 bits of hash length (128 bit
collission resistence).

Do you think it is correct to add hash here crypto/x509v3/v3_purp.c ?

ifndef OPENSSL_NO_SHA256

X509_digest + (x, EVP_sha256 (), x-> sha256_hash, NULL);

endif

I'm not sure on this because that's a part of the source I haven't been
looking into yet. But given the other additions you made in the
structures I feel like omitting this line leaves parts of the structure
uninitialized.

Antonio
Regards,
BenBE

@opensignature
Copy link
Contributor Author

ok BenBE, I added SHA2-384 and rewrote the code part, now it is not required to change v3_purp.c

@richsalz richsalz added the issue: feature request The issue was opened to request a feature label Mar 19, 2016
@mattcaswell mattcaswell added this to the Post 1.1.0 milestone May 5, 2016
sizeof(cert->sha1_hash)))
#ifndef OPENSSL_NO_SHA256
|| (cid->hash->length == sizeof(md_sha256)
&& !memcmp(cid->hash->data, md_sha256,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dont use "!memcmp", please use "memcmp... == 0"

@richsalz
Copy link
Contributor

See also https://rt.openssl.org/Ticket/Display.html?id=3131
(user guest pass guest)

@hdatma
Copy link

hdatma commented Sep 13, 2017

The patches did not hit the release.

@richsalz
Copy link
Contributor

This particular set did not. But the RT and the referenced #2119 PR did. And requested changes were never made, so this was closed. What functionality is still missing? Please open an issue.

@hdatma
Copy link

hdatma commented Sep 13, 2017

The whole CAdES functionality appears to be missing, beginning with its documentation, in the latest openssl release.

https://github.com/openssl/openssl/search?utf8=%E2%9C%93&q=cades&type=

@richsalz
Copy link
Contributor

This PR never had CAdES in it. Nobody has ever provided code to do CAdES. This PR only did different digest mechanisms.

@hdatma
Copy link

hdatma commented Sep 13, 2017

The code for CadES is 3 years old...
https://rt.openssl.org/Ticket/Display.html?id=3131

@richsalz
Copy link
Contributor

To repeat myself. That code needed to have changes made. It was never done. So that three year old ticket was closed. The digest work and the signingCertificateV2 work are there. CaDES is not. If there is something you need that is missing, please open a new issue and provide specifics. Ideally, write the code and make a pull request.

I don't see the point in further discussion here.

@hdatma
Copy link

hdatma commented Sep 14, 2017

The digest work and the signingCertificateV2 work are there.

To repeat myself, the documentation is missing. How do you sign using signingCertificateV2?

This is the command I use. The resulting file does not comply with CAdES.

openssl smime -sign -md sha256 -binary -in $file -signer $certificate -inkey $key -outform DER -out ${file}.p7m -nodetach

@richsalz
Copy link
Contributor

Nobody said we did CAdES signing. We implemented some of the infrastructure. Nobody contributed documentation or code to expose it to the smime app. Please help and do so.

@opensignature
Copy link
Contributor Author

Hi everybody,
part of code for CAdES I wrote it.
If there is anyone who assists me (test, code optimization, etc.) I try to rewrite it based on the last release of Openssl.

@snhenson
Copy link
Contributor

I'd say this should also be made to work with CMS and not just the older PKCS#7.

@opensignature
Copy link
Contributor Author

The patch I wrote (and I hope to make a new pull request) was just about to work with CMS:
openssl cms -sign -md sha256 -binary -in $file -signer $certificate -inkey $key -outform DER -out ${file}.p7m -nodetach -cades
https://www.blia.it/firmadigitale/cades_openssl_1_1_0.patch.gz

mcr pushed a commit to mcr/openssl that referenced this pull request Jun 22, 2021
Move rb_global_variable call to directly after assignment
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
issue: feature request The issue was opened to request a feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants