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

Add ability to calculate filedigests using Streebog-256 and Streebog-… #1082

Closed
wants to merge 1 commit into from

Conversation

StZhukov
Copy link

…512 RFC 6986.

This patch adds ability to calculate filedigests (hashsums of files inside rpm package) using Streebog-256 and Streebog-512 - hashsum calculating algorithms standartized by GOST R 34.10-2012, Russian national standard.

Streebog (another name is Stribog) has existed for quite long time in libgcrypt, and now, after RPM switched to libgcrypt as the default crypto backend, this patch adds ability to use Streebog-256 and Streebog-512 to calculate file digest.

Our plan is to use these digests to sign them using rpmsign --signfile. First, I would like to receive feedback and get this initial implemetation merged. After that I am planning to extend usage of GOST.

I previously implemented the same in rpm5 (https://abf.io/soft/rpm5/commits/master), and it works. As ROSA is switching from rpm5 to rpm4, we would like to contribute our work to upstream. It may be useful for other people who have to follow some special information security requirements or those who do not trust other algorithms for some reasons.

…512 RFC 6986.

This patch adds ability to calculate filedigests (hashsums of files inside rpm package) using Streebog-256 and Streebog-512 - hashsum calculating algorithms standartized by GOST R 34.10-2012, Russian national standard.

Streebog (another name is Stribog) has existed for quite long time in libgcrypt, and now, after RPM switched to libgcrypt as the default crypto backend, this patch adds ability to use Streebog-256 and Streebog-512 to calculate file digest.

Our plan is to use these digests to sign them using rpmsign --signfile. First, I would like to receive feedback and get this initial implemetation merged. After that I am planning to extend usage of GOST.

I previously implemented the same in rpm5 (https://abf.io/soft/rpm5/commits/master), and it works. As ROSA is switching from rpm5 to rpm4, we would like to contribute our work to upstream. It may be useful for other people who have to follow some special information security requirements or those who do not trust other algorithms for some reasons.
Copy link
Member

@Conan-Kudo Conan-Kudo left a comment

Choose a reason for hiding this comment

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

It's a good first start, just some initial nits...

@@ -395,6 +395,7 @@ AC_SUBST(WITH_OPENSSL_LIB)
WITH_LIBGCRYPT_INCLUDE=
WITH_LIBGCRYPT_LIB=
if test "$with_crypto" = libgcrypt ; then
AC_DEFINE(WITH_LIBGCRYPT, 1, [Build with libgcrypt instead of nss3 support?])
Copy link
Member

Choose a reason for hiding this comment

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

Why the instead of nss3 support?? This could just be Build with libgcrypt as the crypto backend.

Copy link
Author

Choose a reason for hiding this comment

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

Thanks, I will change.

*/
{ "rpmlib(FileDigestsGOST12)", "4.16.0-1",
( RPMSENSE_RPMLIB|RPMSENSE_EQUAL),
N_("file digest can be GOST R 34.11 2012 (STREEBOG256, STREEBOG512)") },
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure how to word this yet, but I don't like this wording much...

Copy link
Author

Choose a reason for hiding this comment

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

Sorry, I did not understand, what do you mean?

Copy link
Member

Choose a reason for hiding this comment

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

We don't track individual file digest algorithms for anything else either, lets keep it that way.
Rpm needs to better deal with unknown algorithms but rpmlib() dependencies are such a clumsy and ugly way...

@@ -266,6 +266,8 @@ typedef enum pgpHashAlgo_e {
PGPHASHALGO_SHA384 = 9, /*!< SHA384 */
PGPHASHALGO_SHA512 = 10, /*!< SHA512 */
PGPHASHALGO_SHA224 = 11, /*!< SHA224 */
PGPHASHALGO_GOST12_256 = 100, /*!< GOST R 34.11-2012 256 */
PGPHASHALGO_GOST12_512 = 101, /*!< GOST R 34.11-2012 512 */
Copy link
Member

Choose a reason for hiding this comment

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

Is there a reason we jump so many values?

Copy link
Contributor

Choose a reason for hiding this comment

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

It should match the hash algorithm id used in pgp.

12 is SHA3-256, 14 is SHA3-512. 100-110 is reserved for Private/Experimental algorithms.

See https://tools.ietf.org/html/draft-ietf-openpgp-rfc4880bis-05

Copy link
Contributor

Choose a reason for hiding this comment

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

(I'd actually prefer to use official values. Is the GOST project in contact with the pgp folks?)

Copy link
Contributor

Choose a reason for hiding this comment

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

@mlschroe What is "the GOST project"? I am not aware of any attempts to include GOST to OpenPGP RFC, but maybe someone tried, I do not know. We in ROSA with @StZhukov first wanted to implement signing RPM payload with GOST as it is signed with gpg, but then we found out that OpenPGP RFC does not support GOST, so implementing it does not make much sense. I would not expect GOST algos to appear in the RFC in the nearest future. Because of all this I would suggest to use values >110 here, which are not specified in the RFC and so will never become used by other algorithms.

Copy link
Contributor

Choose a reason for hiding this comment

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

Dunno, I thought something like http://gostcrypt.github.io/

Copy link
Member

@pmatilai pmatilai left a comment

Choose a reason for hiding this comment

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

Please ask for at least a reservation for the GOST algorithms in OpenPGP RFC, I don't see this being acceptable otherwise.

The other option would be detaching the digest algorithm enumeration used by rpm for non-PGP purposes from the OpenPGP values. This would probably be the most "correct" thing to do anyway, but also the one that requires most work and churn.

@mikhailnov
Copy link
Contributor

The other option would be detaching the digest algorithm enumeration used by rpm for non-PGP purposes from the OpenPGP values

If to make values out of the range specified by the OpenPGP RFC (e.g. 250 and 251 or whatever else), they will still be called PGPHASHALGO_*, but may it break anything elsewhere in RPM?

@mikhailnov
Copy link
Contributor

RPM 5 did it: https://abf.io/soft/rpm5/blob/master/rpmio/rpmiotypes.h#lc-204
Will the same approach be acceptable in rpm4?

@Conan-Kudo
Copy link
Member

RPM 5 did it: https://abf.io/soft/rpm5/blob/master/rpmio/rpmiotypes.h#lc-204
Will the same approach be acceptable in rpm4?

Well, rpm5 compatibility is an explicit non-goal, so it's not likely to sway anyone on this...

@pmatilai
Copy link
Member

pmatilai commented Feb 24, 2020

What I meant by detaching is declare a separate RPMHASHALGO_FOO enumeration that is free of OpenPGP constraints and then adjust the entire codebase to use that as appropriate, but that seems like quite a bit of busywork and churn.

Please try to get GOST included in OpenPGP officially, that's what you really wanted to begin with as per above comments. That'd be the win-win situation here. As per https://tools.ietf.org/html/rfc4880#section-10.3.3:

The initial values for this registry can be found in
Section 9 for the algorithm identifiers and text names, and Section
5.2.2 for the OIDs and expanded signature prefixes. Adding a new
hash algorithm MUST be done through the IETF CONSENSUS method, as
described in https://tools.ietf.org/html/rfc2434

@pmatilai
Copy link
Member

Oh and to clarify, what I mean by "getting into OpenPGP" is at least try to get a reservation for the algorithm(s). There are any number of such reservations in the RFC and one would think that it wouldn't require jumping through too many hoops.

mikhailnov added a commit to mikhailnov/rpm that referenced this pull request Oct 10, 2020
Temporarily use the same OIDs for GOST as we used in RPM 5 to ensure RPM 5
is able to install those packages from rpm5 to boostrap rosa2019.05 based on
packages from rosa2019.0.

This needs to be reworked before upstream inclusion.

Cf. rpm-software-management#1082
mikhailnov added a commit to mikhailnov/rpm that referenced this pull request Oct 11, 2020
Temporarily use the same OIDs for GOST as we used in RPM 5 to ensure RPM 5
is able to install those packages from rpm5 to boostrap rosa2019.05 based on
packages from rosa2019.0.

This needs to be reworked before upstream inclusion.

Cf. rpm-software-management#1082
mikhailnov added a commit to mikhailnov/rpm that referenced this pull request Sep 30, 2021
Temporarily use the same OIDs for GOST as we used in RPM 5 to ensure RPM 5
is able to install those packages from rpm5 to boostrap rosa2019.05 based on
packages from rosa2019.0.

This needs to be reworked before upstream inclusion.

Cf. rpm-software-management#1082
@pmatilai
Copy link
Member

Coming back to this after much too long a while:

Private/experimental values are best left alone in what is a very public project with long-lived artifacts (aka rpms). Rpm is wrong to use the PGP hash algorithm values for entirely different purposes, that's where the problem starts and to fix it, we just need to untangle that connection. Otherwise this will keep coming up again and again.

Closing this but opened #1899 to track the underlying issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants