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.

Sign up for GitHub

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

Jump to bottom

xmlsec-nss: RSA-PKCS 1.5 key transport is not supported #1

Closed
lsh123 opened this issue Jan 28, 2016 · 1 comment
Closed

xmlsec-nss: RSA-PKCS 1.5 key transport is not supported #1

lsh123 opened this issue Jan 28, 2016 · 1 comment

Comments

@lsh123
Copy link
Owner

lsh123 commented Jan 28, 2016

RSA Encryption/Decryption using PKCS#1 v1.5 padding not currently
exposed in NSS. This causes some tests to fail.

NSS bug: http://bugzilla.mozilla.org/show_bug.cgi?id=214236

Migrated from: https://bugzilla.gnome.org/show_bug.cgi?id=118628
lsh123 added a commit that referenced this issue Dec 26, 2016
@lsh123
pass #1
12eff5e
This was referenced Feb 17, 2017
Closed
Closed
Closed
Closed
Closed
vmiklos pushed a commit to vmiklos/xmlsec that referenced this issue Mar 6, 2017
nss: fix assert when building against debug nss
05ae853 
Due to reading uninitialized memory. gdb says:

    Assertion failure: dest == NULL || dest->data == NULL, at secasn1e.c:1483
    Program received signal SIGABRT, Aborted.
    0x00007ffff74748d7 in raise () from /lib64/libc.so.6
    (gdb) up
    lsh123#1  0x00007ffff7475caa in abort () from /lib64/libc.so.6
    (gdb)
    lsh123#2  0x00007fffe57f96ae in PR_Assert (s=0x7fffe1cbf298 "dest == NULL || dest->data == NULL", file=0x7fffe1cbef60 "secasn1e.c", ln=1483) at ../../../../pr/src/io/prlog.c:553
    553         abort();
    (gdb)
    lsh123#3  0x00007fffe1cb1941 in SEC_ASN1EncodeItem_Util (poolp=0x0, dest=0x7fffffff95f0, src=0x7fffffff9530, theTemplate=0x7fffe55ae180 <DSA_SignatureTemplate>) at secasn1e.c:1483
    1483        PORT_Assert(dest == NULL || dest->data == NULL);
lsh123 pushed a commit that referenced this issue Mar 6, 2017
@vmiklos @lsh123
nss: fix assert when building against debug nss (#91)
a7528e7 
Due to reading uninitialized memory. gdb says:

    Assertion failure: dest == NULL || dest->data == NULL, at secasn1e.c:1483
    Program received signal SIGABRT, Aborted.
    0x00007ffff74748d7 in raise () from /lib64/libc.so.6
    (gdb) up
    #1  0x00007ffff7475caa in abort () from /lib64/libc.so.6
    (gdb)
    #2  0x00007fffe57f96ae in PR_Assert (s=0x7fffe1cbf298 "dest == NULL || dest->data == NULL", file=0x7fffe1cbef60 "secasn1e.c", ln=1483) at ../../../../pr/src/io/prlog.c:553
    553         abort();
    (gdb)
    #3  0x00007fffe1cb1941 in SEC_ASN1EncodeItem_Util (poolp=0x0, dest=0x7fffffff95f0, src=0x7fffffff9530, theTemplate=0x7fffe55ae180 <DSA_SignatureTemplate>) at secasn1e.c:1483
    1483        PORT_Assert(dest == NULL || dest->data == NULL);
vmiklos pushed a commit to vmiklos/xmlsec that referenced this issue Apr 20, 2017
@vmiklos
nss: fix assert when building against debug nss (lsh123#91)
eda3128 
Due to reading uninitialized memory. gdb says:

    Assertion failure: dest == NULL || dest->data == NULL, at secasn1e.c:1483
    Program received signal SIGABRT, Aborted.
    0x00007ffff74748d7 in raise () from /lib64/libc.so.6
    (gdb) up
    lsh123#1  0x00007ffff7475caa in abort () from /lib64/libc.so.6
    (gdb)
    lsh123#2  0x00007fffe57f96ae in PR_Assert (s=0x7fffe1cbf298 "dest == NULL || dest->data == NULL", file=0x7fffe1cbef60 "secasn1e.c", ln=1483) at ../../../../pr/src/io/prlog.c:553
    553         abort();
    (gdb)
    lsh123#3  0x00007fffe1cb1941 in SEC_ASN1EncodeItem_Util (poolp=0x0, dest=0x7fffffff95f0, src=0x7fffffff9530, theTemplate=0x7fffe55ae180 <DSA_SignatureTemplate>) at secasn1e.c:1483
    1483        PORT_Assert(dest == NULL || dest->data == NULL);
vmiklos added a commit to vmiklos/xmlsec that referenced this issue Jun 16, 2018
@vmiklos
mscng signatures: don't free hash/hashobject memory before closing th…
210e26d 
…e hash handle

The new order of cleanup matches MSDN documentation and fixes this
drmemory error:

	Error lsh123#1: UNADDRESSABLE ACCESS of freed memory: reading 0x0092c5e0-0x0092c5e4 4 byte(s)
	# 0 bcrypt.dll!BCryptDestroyHash              +0x74     (0x72701ad3 <bcrypt.dll+0x1ad3>)
	# 1 bcrypt.dll!BCryptDestroyHash              +0xe      (0x72701a6e <bcrypt.dll+0x1a6e>)
	# 2 libxmlsec-mscng.dll!xmlSecMSCngSignatureFinalize [xmlsec\src\mscng\signatures.c:274]
	# 3 libxmlsec.dll!xmlSecTransformDestroy       [xmlsec\src\transforms.c:1271]
	# 4 libxmlsec.dll!xmlSecTransformCtxReset      [xmlsec\src\transforms.c:394]
	# 5 libxmlsec.dll!xmlSecTransformCtxFinalize   [xmlsec\src\transforms.c:361]
	# 6 libxmlsec.dll!xmlSecDSigCtxFinalize        [xmlsec\src\xmldsig.c:178]
	# 7 xmlSecAppVerifyFile                        [xmlsec\apps\xmlsec.c:1324]
	# 8 main                                       [xmlsec\apps\xmlsec.c:1091]
	Note: @0:00:02.891 in thread 2640
	Note: prev lower malloc:  0x0092c5a0-0x0092c5bc
	Note: 0x0092c5e0-0x0092c5e4 overlaps memory 0x0092c5e0-0x0092c692 that was freed here:
	Note: # 0 replace_free                               [d:\drmemory_package\common\alloc_replace.c:2706]
	Note: # 1 libxmlsec-mscng.dll!xmlSecMSCngSignatureFinalize [xmlsec\src\mscng\signatures.c:270]
	Note: # 2 libxmlsec.dll!xmlSecTransformDestroy       [xmlsec\src\transforms.c:1271]
	Note: # 3 libxmlsec.dll!xmlSecTransformCtxReset      [xmlsec\src\transforms.c:394]
	Note: # 4 libxmlsec.dll!xmlSecTransformCtxFinalize   [xmlsec\src\transforms.c:361]
	Note: # 5 libxmlsec.dll!xmlSecDSigCtxFinalize        [xmlsec\src\xmldsig.c:178]
	Note: # 6 xmlSecAppVerifyFile                        [xmlsec\apps\xmlsec.c:1324]
	Note: # 7 main                                       [xmlsec\apps\xmlsec.c:1091]
	Note: instruction: cmp    (%eax) $0x00000018
vmiklos added a commit to vmiklos/xmlsec that referenced this issue Jun 17, 2018
@vmiklos
mscng signatures: don't free hash/hashobject memory before closing th…
ce8c6f8 
…e hash handle

The new order of cleanup matches MSDN documentation and fixes this
drmemory error:

	Error lsh123#1: UNADDRESSABLE ACCESS of freed memory: reading 0x0092c5e0-0x0092c5e4 4 byte(s)
	# 0 bcrypt.dll!BCryptDestroyHash              +0x74     (0x72701ad3 <bcrypt.dll+0x1ad3>)
	# 1 bcrypt.dll!BCryptDestroyHash              +0xe      (0x72701a6e <bcrypt.dll+0x1a6e>)
	# 2 libxmlsec-mscng.dll!xmlSecMSCngSignatureFinalize [xmlsec\src\mscng\signatures.c:274]
	# 3 libxmlsec.dll!xmlSecTransformDestroy       [xmlsec\src\transforms.c:1271]
	# 4 libxmlsec.dll!xmlSecTransformCtxReset      [xmlsec\src\transforms.c:394]
	# 5 libxmlsec.dll!xmlSecTransformCtxFinalize   [xmlsec\src\transforms.c:361]
	# 6 libxmlsec.dll!xmlSecDSigCtxFinalize        [xmlsec\src\xmldsig.c:178]
	# 7 xmlSecAppVerifyFile                        [xmlsec\apps\xmlsec.c:1324]
	# 8 main                                       [xmlsec\apps\xmlsec.c:1091]
	Note: @0:00:02.891 in thread 2640
	Note: prev lower malloc:  0x0092c5a0-0x0092c5bc
	Note: 0x0092c5e0-0x0092c5e4 overlaps memory 0x0092c5e0-0x0092c692 that was freed here:
	Note: # 0 replace_free                               [d:\drmemory_package\common\alloc_replace.c:2706]
	Note: # 1 libxmlsec-mscng.dll!xmlSecMSCngSignatureFinalize [xmlsec\src\mscng\signatures.c:270]
	Note: # 2 libxmlsec.dll!xmlSecTransformDestroy       [xmlsec\src\transforms.c:1271]
	Note: # 3 libxmlsec.dll!xmlSecTransformCtxReset      [xmlsec\src\transforms.c:394]
	Note: # 4 libxmlsec.dll!xmlSecTransformCtxFinalize   [xmlsec\src\transforms.c:361]
	Note: # 5 libxmlsec.dll!xmlSecDSigCtxFinalize        [xmlsec\src\xmldsig.c:178]
	Note: # 6 xmlSecAppVerifyFile                        [xmlsec\apps\xmlsec.c:1324]
	Note: # 7 main                                       [xmlsec\apps\xmlsec.c:1091]
	Note: instruction: cmp    (%eax) $0x00000018
vmiklos added a commit to vmiklos/xmlsec that referenced this issue Sep 7, 2018
@vmiklos
nss: fix undefined behavior due to too large shift
e470f35 
When building with clang -fsanitize=undefined, ubsan says:

x509.c:1750:46: runtime error: shift exponent 64 is too large for 64-bit type 'PRUint64' (aka 'unsigned long')
    #0 0x444d45 in xmlSecNssASN1IntegerWrite src/nss/x509.c:1750:46
    lsh123#1 0x4443ec in xmlSecNssX509IssuerSerialNodeWrite src/nss/x509.c:1259:11
    lsh123#2 0x4403ba in xmlSecNssKeyDataX509XmlWrite src/nss/x509.c:769:19
    lsh123#3 0x45962a in xmlSecKeyInfoNodeWrite src/keyinfo.c:180:19
    lsh123#4 0x480149 in xmlSecDSigCtxProcessKeyInfoNode src/xmldsig.c:807:15
    lsh123#5 0x47c774 in xmlSecDSigCtxProcessSignatureNode src/xmldsig.c:506:11
    lsh123#6 0x47bfb2 in xmlSecDSigCtxSign src/xmldsig.c:289:11

And indeed shifting a 64bit value by 64 bits happens in practice there
as num->len is 9. At the same time (at least in case of the test) in all
3 cases the value that would be shifted is 0.

Avoid undefined behavior by simply not shifting if the value is 0
anyway.

Testcase: make check-crypto-nss XMLSEC_TEST_NAME="aleksey-xmldsig-01/x509data-sn-test"
@vmiklos vmiklos mentioned this issue Sep 7, 2018
Merged
lsh123 pushed a commit that referenced this issue Sep 8, 2018
@vmiklos @lsh123
nss: fix undefined behavior due to too large shift (#221)
10c9708 
When building with clang -fsanitize=undefined, ubsan says:

x509.c:1750:46: runtime error: shift exponent 64 is too large for 64-bit type 'PRUint64' (aka 'unsigned long')
    #0 0x444d45 in xmlSecNssASN1IntegerWrite src/nss/x509.c:1750:46
    #1 0x4443ec in xmlSecNssX509IssuerSerialNodeWrite src/nss/x509.c:1259:11
    #2 0x4403ba in xmlSecNssKeyDataX509XmlWrite src/nss/x509.c:769:19
    #3 0x45962a in xmlSecKeyInfoNodeWrite src/keyinfo.c:180:19
    #4 0x480149 in xmlSecDSigCtxProcessKeyInfoNode src/xmldsig.c:807:15
    #5 0x47c774 in xmlSecDSigCtxProcessSignatureNode src/xmldsig.c:506:11
    #6 0x47bfb2 in xmlSecDSigCtxSign src/xmldsig.c:289:11

And indeed shifting a 64bit value by 64 bits happens in practice there
as num->len is 9. At the same time (at least in case of the test) in all
3 cases the value that would be shifted is 0.

Avoid undefined behavior by simply not shifting if the value is 0
anyway.

Testcase: make check-crypto-nss XMLSEC_TEST_NAME="aleksey-xmldsig-01/x509data-sn-test"
@lsh123 lsh123 mentioned this issue Nov 24, 2022
Merged
@lsh123
Copy link
Owner Author

lsh123 commented Nov 24, 2022

PCKS 1.5 was implemented but was missing ability to use it for non-key material. Closing since PKCS 1.5 is considered insecure and is deprecated now.

@lsh123 lsh123 closed this as completed Nov 24, 2022
@thalman thalman mentioned this issue Apr 5, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@lsh123