x509.h includes cert.h. But that doesn't know of LO using
xmlsecurity/source/xmlsec/nss/nssrenam.h, which has a "#define
CERT_DecodeDERCertificate __CERT_DecodeDERCertificate". So the PCH
doesn't know of this rename and the compiler fails.
move the include line into the file that needs it and the --enable-pch=full
build works ok
Change-Id: I247bd219cf47964490ded439ad51bd8e8e120c48
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/127744
Reviewed-by: Jan-Marek Glogowski <glogow@fbihome.de>
Reviewed-by: Caolán McNamara <caolanm@redhat.com>
Tested-by: Jenkins
We have two environments where the signature and the stream bytes are
the same, still in one case the signature verification succeeds and in
the other case the hash doesn't match.
Log the signature as parsed into a DOM node (recursively), just case
something goes wrong during extracting a single signature from the
signatures list XML.
Change-Id: I54af71fdeb63d8ef44342f106746f938fa51f29a
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/127991
Reviewed-by: Miklos Vajna <vmiklos@collabora.com>
Tested-by: Jenkins
* remove GPGME_CAN_EXPORT_MINIMAL_KEY, upstream now has support for
key export flags in c++ wrapper (gpgmepp >= 1.14)
* therefore, external/gpgmepp/add-minimal-keyexport.patch now fully
obsolete, tweaked xmlsecurity code to use upstream function
* bits of external/gpgmepp/find-libgpg-error-libassuan.patch are
upstream now (configure and makefile pieces, though we keep
configure.ac changes for the while - to not pick up system versions
too easily)
* external/gpgmepp/gpgme.git-fe2892618c20cd40c342cce26ffb6ac4644fd3c3.patch.1
was from upstream anyway, removed
Change-Id: I991c20c0eeff0f9135e97c991afcb905be55a959
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/127665
Tested-by: Jenkins
Reviewed-by: Thorsten Behrens <thorsten.behrens@allotropia.de>
We can already see if a signature verification fails and what is the
content of the ZIP streams we hash.
Show what is the expected hash as well.
Change-Id: Ibc67b7de0e8d03e06da1b86b6e8a7b2b2e613882
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/127934
Reviewed-by: Miklos Vajna <vmiklos@collabora.com>
Tested-by: Jenkins
We only get:
warn:xmlsecurity.xmlsec:32272:32272:xmlsecurity/source/xmlsec/errorcallback.cxx:53: digests.c:226: xmlSecNssDigestVerify() 'sha1' '' 12 'data and digest do not match'
when one of the XML streams have a bad hash. Add some logging to help
figuring out (without a debugger) which stream is at fault.
Change-Id: Ib5f39df87bcdaaac1a21eb560c8f775c42a4079f
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/127885
Reviewed-by: Miklos Vajna <vmiklos@collabora.com>
Tested-by: Jenkins
Signing OOXML with 3 or more times didn't work as other ids
("idPackageObject", "idOfficeObject", ...) were not uniqe. This
change makes those ids unique by appending the signature id. The
signature ID is now generated for OOXML too, while previously it
was a hardcoded string ("idPackageSignature").
The test for signing multiple OOXML was written before, but didn't
catch the issues because it didn't assert the status of the
document after loading it again. This is which is now fixed (and
also added changed for the ODF test case).
Change-Id: Ifa20ea17498b117a4c57f6eddf82f8e83bc640bc
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/124571
Tested-by: Jenkins
Reviewed-by: Tomaž Vajngerl <quikee@gmail.com>
Signing the document 3 or more times produces an invalid signature.
The cause of this is that xmlsec is confused because we have 3
signatures, which all have the same SignedProperties with the ID
"idSignedProperties", but it expect them to be unique.
This issue is fixed by making the ID unique with adding the ID of
the Signature to the SignedProperties ID, so this makes them unique
inside the same Signature.
Also UnsignedProperties have a unique ID usign the same approach,
but they aren't referenced - luckily.
Change-Id: I53c7249a82fc0623586548db9fa25bdc0e7c4101
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/124278
Tested-by: Jenkins
Reviewed-by: Tomaž Vajngerl <quikee@gmail.com>
It turns out that the 2 backends NSS and MS CryptoAPI generate different
string representations of the same Distinguished Name in at least one
corner case, when a value contains a quote " U+0022.
The CryptoAPI function to generate the strings is:
CertNameToStr(..., CERT_X500_NAME_STR | CERT_NAME_STR_REVERSE_FLAG, ...)
This is documented on MSDN:
https://docs.microsoft.com/en-us/windows/win32/api/wincrypt/nf-wincrypt-certnametostra#CERT_X500_NAME_STR
NSS appears to implement RFC 1485, at least that's what the internal
function is named after, or perhaps one of its several successor RFCs
(not clear currently if there's a relevant difference).
This is now causing trouble if a certificate with such a DN is used in a
signature, created on WNT but then verified on another platform, because
commit 5af5ea893b
introduced consistency checks that compare the DNs that occur as strings
in META-INF/documentsignatures.xml:
xmlsecurity/source/helper/xmlsignaturehelper.cxx:672: X509Data cannot be parsed
The reason is that in XSecController::setX509Data() the value read from
the X509IssuerSerial element (a string generated by CryptoAPI) doesn't
match the value generated by NSS from the certificate parsed from the
X509Certificate element, so these are erroneously interpreted as 2
distinct certificates.
Try to make the EqualDistinguishedNames() more flexible so that it can
try also a converted variant of the DN.
(libxmlsec's NSS backend also complains that it cannot parse the DN:
x509vfy.c:607: xmlSecNssX509NameRead() '' '' 12 'invalid data for 'char': actual=34 and expected comma ',''
but it manages to validate the signature despite this.)
Change-Id: I4f72900738d1f5313146bbda7320a8f44319ebc8
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/124287
Tested-by: Jenkins
Reviewed-by: Michael Stahl <michael.stahl@allotropia.de>
Tests added in commit 40d70d427e don't
actually run on WNT but that wasn't obvious because commit
149df1fec6 prevented running them in
Jenkins on master, they failed only in the libreoffice-7-1 backport.
xmlsecurity/qa/unit/signing/signing.cxx(631) : error : Assertion
Test name: testODFDoubleX509Certificate::TestBody
assertion failed
- Expression: (nActual == SignatureState::NOTVALIDATED || nActual == SignatureState::OK)
- 2
This is an oddity where NSS claims the signature in the document is
valid but CryptoAPI claims it is invalid; the hashes passed into the
validation functions are the same. Just allow BROKEN as an additional
result value on WNT.
xmlsecurity/qa/unit/signing/signing.cxx(550) : error : Assertion
Test name: testODFX509CertificateChain::TestBody
equality assertion failed
- Expected: 0
- Actual : 1
The problem here is that with NSS the tests use a custom NSS database
in test/signing-keys so we need to make these certificates available for
CryptoAPI too.
The following one-liner converts the NSS database to a PKCS#7 that can
be loaded by CrytpAPI:
> openssl crl2pkcs7 -nocrl -certfile <(certutil -d sql:test/signing-keys -L | awk '/^[^ ].*,[^ ]*,/ { printf "%s", $1; for (i = 2; i < NF; i++) { printf " %s", $i; } printf "\n"; }' | while read name; do certutil -L -d sql:test/signing-keys -a -n "${name}" ; done) > test/signing-keys/test.p7b
Then one might naively assume that something like this would allow these
certificates to be added temporarily as trusted CAs:
+ HCERTSTORE hRoot = CertOpenSystemStoreW( 0, L"Root" ) ;
+ HCERTSTORE const hExtra = CertOpenStore(
+ CERT_STORE_PROV_FILENAME_A,
+ PKCS_7_ASN_ENCODING | X509_ASN_ENCODING,
+ NULL,
+ CERT_STORE_OPEN_EXISTING_FLAG | CERT_STORE_READONLY_FLAG,
+ path);
+ if (hExtra != NULL && hRoot != NULL)
+ {
+ BOOL ret = CertAddStoreToCollection(
+ hRoot,
+ hExtra,
+ CERT_PHYSICAL_STORE_ADD_ENABLE_FLAG,
+ 0);
+ SAL_DEBUG("XXX hExtra done " << ret);
+ }
There is no error from this, but it doesn't work.
Instead, check if CertGetCertificateChain() sets the
CERT_TRUST_IS_UNTRUSTED_ROOT flag and then look up the certificate
manually in the extra PKCS#7 store.
Change-Id: Ic9865e0b5783211c2128ce0327c4583b7784ff62
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/123667
Tested-by: Jenkins
Reviewed-by: Miklos Vajna <vmiklos@collabora.com>
These 2 calls to CertAddStoreToCollection follow calls where
m_hCertStore was already added and according to the comments they
should add the other store instead.
(regression from commit 813e1f5a8a)
Change-Id: If375f603647a702feb0ca8f272126a15d5d0e906
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/123666
Tested-by: Jenkins
Reviewed-by: Miklos Vajna <vmiklos@collabora.com>
The scenarios are:
1. Calling sequence's begin() and end() in pairs to pass to algorithms
(both calls use getArray(), which does the COW checks)
2. In addition to #1, calling end() again when checking result of find
algorithms, and/or begin() to calculate result's distance
3. Using non-const sequences in range-based for loops, which internally
do #1
4. Assigning sequence to another sequence variable, and then modifying
one of them
In many cases, the sequences could be made const, or treated as const
for the purposes of the algorithms (using std::as_const, std::cbegin,
and std::cend). Where algorithm modifies the sequence, it was changed
to only call getArray() once. For that, css::uno::toNonConstRange was
introduced, which returns a struct (sublclass of std::pair) with two
iterators [begin, end], that are calculated using one call to begin()
and one call to getLength().
To handle #4, css::uno::Sequence::swap was introduced, that swaps the
internal pointer to uno_Sequence. So when a local Sequence variable
should be assigned to another variable, and the latter will be modified
further, it's now possible to use swap instead, so the two sequences
are kept independent.
The modified places were found by temporarily removing non-const end().
Change-Id: I8fe2787f200eecb70744e8b77fbdf7a49653f628
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/123542
Tested-by: Jenkins
Reviewed-by: Mike Kaganski <mike.kaganski@collabora.com>
...compared to a full-blown O[U]String, for temporary objects holding an
O[U]StringConcat result that can then be used as a std::[u16]string_view.
It's instructive to see how some invocations of operator ==, operator !=, and
O[U]StringBuffer::insert with an O[U]StringConcat argument required implicit
materialization of an O[U]String temporary, and how that expensive operation has
now been made explicit with the explicit O[U]StringConcatenation ctor.
(The additional operator == and operator != overloads are necessary because the
overloads taking two std::[u16]string_view parameters wouldn't even be found
here with ADL. And the OUString-related ones would cause ambiguities in at
least sal/qa/rtl/strings/test_oustring_stringliterals.cxx built with
RTL_STRING_UNITTEST, so have simply been disabled for that special test-code
case.)
Change-Id: Id29799fa8da21a09ff9794cbc7cc9b366e6803b8
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/122890
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
Reviewed-by: Stephan Bergmann <sbergman@redhat.com>
- Revise uses of getSomething to use getFromUnoTunnel
Where that is impossible, use getSomething_cast to unify casting,
and minimize number of places doing low-level transformations.
The change keeps the existing tunnel references that last for the
duration of the pointers' life, because sometimes destroying such
reference may destroy the pointed object, and result in use after
free.
Change-Id: I291c33223582c34cd2c763aa8aacf0ae899ca4c0
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/122101
Tested-by: Jenkins
Reviewed-by: Mike Kaganski <mike.kaganski@collabora.com>
- Change implementations of getSomething to use getSomethingImpl
Or where that's impossible, use getSomething_cast to unify this and
reduce number of places where we reinterpret_cast.
All static methods getting tunnel ids were renamed to getUnoTunnelId,
to comply with the convention used in <comphelper/servicehelper.hxx>.
TODO (in separate commits):
- Revise uses of getSomething to use getFromUnoTunnel
Change-Id: Ifde9e214b52e5df678de71fcc32d2199c82e85cf
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/122100
Tested-by: Jenkins
Reviewed-by: Mike Kaganski <mike.kaganski@collabora.com>
- Replace all uses of OImplementationId in core with UnoIdInit
- Deprecate OImplementationId in <cppuhelper/typeprovider.hxx>
OImplementationId uses broken double checked locking; additionally,
it uses it at the first call to getImplementationId, not when the
object is constructed. This implementation can't be changed, cince
it's part of published API; it can't rely on C++11, which would be
required for use of thread-safe statics and move the initialization
to ctor.
The class has obsolete _bUseEthernetAddress member, that is unused
and ignored since 4e9fa7e339. No need
to implement it when replacing its uses to UnoIdInit.
The deprecation is the API CHANGE. No published API is introduced to
replace it; 3rd-party code should seek alternative solutions, or just
keep using the deprecated functionality.
TODO (in separate commits):
- Change implementations of getSomething to use getSomethingImpl
- Revise uses of getSomething to use getFromUnoTunnel
Change-Id: I8b6e684e5389bc0d5bb3b7f21f72a4c8f684107d
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/122077
Tested-by: Jenkins
Reviewed-by: Mike Kaganski <mike.kaganski@collabora.com>
The header got some changes:
1. Move UnoTunnelIdInit and isUnoTunnelId into 'comphelper' namespace
2. Rename UnoTunnelIdInit to UnoIdInit, as a precondition to replace
of uses of OImplementationId with it, including in XTypeProvider
3. Introduce convenience functions 'getSomething_cast' to cast between
sal_Int64 and object pointers uniformly.
4. Rename getUnoTunnelImplementation to getFromUnoTunnel, both to make
it a bit shorter, and to reflect its function better. Templatize it
to take also css::uno::Any for convenience.
5. Introduce getSomethingImpl, inspired by sw::UnoTunnelImpl; allow it
handle cases both with and without fallback to parent.
6. Adjust UNO3_GETIMPLEMENTATION_* macros
TODO (in separate commits):
- Drop sw::UnoTunnelImpl and sw::UnoTunnelGetImplementation
- Replace all uses of OImplementationId in core with UnoIdInit
- Deprecate OImplementationId in <cppuhelper/typeprovider.hxx>
- Change implementations of getSomething to use getSomethingImpl
- Revise uses of getSomething to use getFromUnoTunnel
Change-Id: If4a3cb024130f1f552f988f0479589da1cd066e7
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/122022
Tested-by: Jenkins
Reviewed-by: Mike Kaganski <mike.kaganski@collabora.com>
Caolán McNamara