entrouvert
/
lasso
16
0
Fork 0
Code Issues Pull Requests Releases Activity

misc: check for signature parameter before sigalg (#89371) #13

Merged
bdauvergne merged 1 commits from wip/89371-When-using-lasso-as-IDP-missing into main 2024-04-15 17:25:30 +02:00
Conversation 3 Commits 1 Files Changed 2 +10 -3
bdauvergne commented 2024-04-10 19:02:02 +02:00
Owner
Copy Link
No description provided.
bdauvergne added 1 commit 2024-04-10 19:02:02 +02:00
tnoel approved these changes 2024-04-11 10:58:15 +02:00
yweber reviewed 2024-04-11 11:25:11 +02:00
yweber left a comment
Owner
Copy Link

Est-ce que ça serait une bonne idée d'ajouter un test ?

Par exemple

diff --git a/tests/random_tests.c b/tests/random_tests.c
index ff60e73b..532989b5 100644
--- a/tests/random_tests.c
+++ b/tests/random_tests.c
@@ -292,6 +292,8 @@ START_TEST(test07_saml2_query_verify_signature)
         * changed to ; */
        const char query2[] = "Signature=Zfz3DE1VMV3thaV4FWpH0fkWsBMzAFJcfvVWAbo0a3cY48Et%2BXUcbr1nvOJUJmhGoie0pQ4%2BcD9ToQlSk7BbJSBCct%2FQQgn2QNkX%2F1lk4v8RU8p5ptJRJ2iPLb8nC6WZhs81HoihQePSuj7Qe5bRUsDKvnWMq6OkD%2Fe6YO77dMXregTcfmnkrXqRb2T6TFfqyOz9i0%2FjmISsmj%2F3kEEfUzVA4LEbeEgiJDj1hec4XW26gQTih53v0sYukq4Eyb4zS2jVd3apUUxUrjn1NUpr7Z7dZ7w5MQlgZ8aw1xFDE8BkxymvIjwf8ciyx6sfTKbCRsoS9E0pQB1vxvh6OMt1Ww%3D%3D;SAMLRequest=fVHJasMwEP0Vo3tqRXY2YRvcOIFAl9CUHnopwpkkAllyNeMuf1%2FZaSG95PrmLfNmMlSNaWXZ0ck%2BwXsHSNFXYyzKYZCzzlvpFGqUVjWAkmq5K%2B%2FvpLjhsvWOXO0Mu5BcVyhE8KSdZdGmytnbNEmTBV%2Bli9ulKMt5KlbVfDkbizWfcVEmUxa9gMfAz1mQBxFiBxuLpCwFiIvxiE9H48mz4FJMZJq8sqgKHbRVNKhORK2MY71vJzFqezSw00f7GPLXztcw9M7ZQRmE3n0bFtQf8IcUWV9JDqm%2B%2BPXCYNUAqb0ilcWXhOx8zIdQe1NtndH1dx%2FTKLp%2BlR7R%2B9FhoMq2b4wEllhUGuM%2Blx4UhZ3Id8Di4pz5%2F2fFDw%3D%3D;RelayState=fake;SigAlg=http%3A%2F%2Fwww.w3.org%2F2001%2F04%2Fxmldsig-more%23rsa-sha256";
        const char query3[] = "SAMLRequest=fVHJasMwEP0Vo3tqRXY2YRvcOIFAl9CUHnopwpkkAllyNeMuf1%2FZaSG95PrmLfNmMlSNaWXZ0ck%2BwXsHSNFXYyzKYZCzzlvpFGqUVjWAkmq5K%2B%2FvpLjhsvWOXO0Mu5BcVyhE8KSdZdGmytnbNEmTBV%2Bli9ulKMt5KlbVfDkbizWfcVEmUxa9gMfAz1mQBxFiBxuLpCwFiIvxiE9H48mz4FJMZJq8sqgKHbRVNKhORK2MY71vJzFqezSw00f7GPLXztcw9M7ZQRmE3n0bFtQf8IcUWV9JDqm%2B%2BPXCYNUAqb0ilcWXhOx8zIdQe1NtndH1dx%2FTKLp%2BlR7R%2B9FhoMq2b4wEllhUGuM%2Blx4UhZ3Id8Di4pz5%2F2fFDw%3D%3D&RelayState=fake&SigAlg=http%3A%2F%2Fwww.w3.org%2F2001%2F04%2Fxmldsig-more%23rsa-sha256&Signature=rUJ%2B9wVSvdGSmZWGuGXgudAPV5KBxRfxRKraBWGIslBz2XreyNbQjSA47DhIfi%2Bxf0awIIGkKcieN3Qd5sqVn4wvFU8fsmfqrdtouYi46aKsj4W91N19TxJ%2BCgrP7ygVEGDaGdc%2BrCQC3%2FuoYTELXq0gYP7tHaXA%2FCaZHfx5Z159crpRxS6eabZ6BGf4ImxiKhE1FuYzKHeISEV1iSyvgx5%2FE8ydSO%2FSP6yA5Rck4JxVJWH6ImbswCVQ80qfqR4NoJ%2BxiZqilbDJnQaSKZggx%2FgjNVoX%2FMVW1FqEmgJNcZpSjNUQqy9u4veSllpxPc2aB%2FpiUjzpbq9XzyFDOQfkUQ%3D%3D";
+       /* Deleting SigAlg & Signature fields */
+       const char query4[] = "SAMLRequest=fVHJasMwEP0Vo3tqRXY2YRvcOIFAl9CUHnopwpkkAllyNeMuf1%2FZaSG95PrmLfNmMlSNaWXZ0ck%2BwXsHSNFXYyzKYZCzzlvpFGqUVjWAkmq5K%2B%2FvpLjhsvWOXO0Mu5BcVyhE8KSdZdGmytnbNEmTBV%2Bli9ulKMt5KlbVfDkbizWfcVEmUxa9gMfAz1mQBxFiBxuLpCwFiIvxiE9H48mz4FJMZJq8sqgKHbRVNKhORK2MY71vJzFqezSw00f7GPLXztcw9M7ZQRmE3n0bFtQf8IcUWV9JDqm%2B%2BPXCYNUAqb0ilcWXhOx8zIdQe1NtndH1dx%2FTKLp%2BlR7R%2B9FhoMq2b4wEllhUGuM%2Blx4UhZ3Id8Di4pz5%2F2fFDw%3D%3D&RelayState=fake";
        /* sp5-saml2 key */
        const char pkey[] = "-----BEGIN CERTIFICATE-----\n\
 MIIDnjCCAoagAwIBAgIBATANBgkqhkiG9w0BAQUFADBUMQswCQYDVQQGEwJGUjEP\n\
@@ -324,6 +326,11 @@ LlTxKnCrWAXftSm1rNtewTsF\n\
        /* test reordering and semi-colon separator support */
        ck_assert_msg(lasso_saml2_query_verify_signature(query2, key) == 0, "Disordered signature was not validated");
        ck_assert_msg(lasso_saml2_query_verify_signature(query3, key) != 0, "Altered signature was validated");
+       /* test error codes */
+       ck_assert_msg(lasso_saml2_query_verify_signature(query3, key) == LASSO_DS_ERROR_INVALID_SIGNATURE,
+                       "Altered signature do not lead to invalid signature");
+       ck_assert_msg(lasso_saml2_query_verify_signature(query4, key) == LASSO_DS_ERROR_SIGNATURE_NOT_FOUND,
+                       "Bad error code when missing signature");
        xmlSecKeyDestroy(key);
 }
 END_TEST
Est-ce que ça serait une bonne idée d'ajouter un test ? Par exemple ```diff diff --git a/tests/random_tests.c b/tests/random_tests.c index ff60e73b..532989b5 100644 --- a/tests/random_tests.c +++ b/tests/random_tests.c @@ -292,6 +292,8 @@ START_TEST(test07_saml2_query_verify_signature) * changed to ; */ const char query2[] = "Signature=Zfz3DE1VMV3thaV4FWpH0fkWsBMzAFJcfvVWAbo0a3cY48Et%2BXUcbr1nvOJUJmhGoie0pQ4%2BcD9ToQlSk7BbJSBCct%2FQQgn2QNkX%2F1lk4v8RU8p5ptJRJ2iPLb8nC6WZhs81HoihQePSuj7Qe5bRUsDKvnWMq6OkD%2Fe6YO77dMXregTcfmnkrXqRb2T6TFfqyOz9i0%2FjmISsmj%2F3kEEfUzVA4LEbeEgiJDj1hec4XW26gQTih53v0sYukq4Eyb4zS2jVd3apUUxUrjn1NUpr7Z7dZ7w5MQlgZ8aw1xFDE8BkxymvIjwf8ciyx6sfTKbCRsoS9E0pQB1vxvh6OMt1Ww%3D%3D;SAMLRequest=fVHJasMwEP0Vo3tqRXY2YRvcOIFAl9CUHnopwpkkAllyNeMuf1%2FZaSG95PrmLfNmMlSNaWXZ0ck%2BwXsHSNFXYyzKYZCzzlvpFGqUVjWAkmq5K%2B%2FvpLjhsvWOXO0Mu5BcVyhE8KSdZdGmytnbNEmTBV%2Bli9ulKMt5KlbVfDkbizWfcVEmUxa9gMfAz1mQBxFiBxuLpCwFiIvxiE9H48mz4FJMZJq8sqgKHbRVNKhORK2MY71vJzFqezSw00f7GPLXztcw9M7ZQRmE3n0bFtQf8IcUWV9JDqm%2B%2BPXCYNUAqb0ilcWXhOx8zIdQe1NtndH1dx%2FTKLp%2BlR7R%2B9FhoMq2b4wEllhUGuM%2Blx4UhZ3Id8Di4pz5%2F2fFDw%3D%3D;RelayState=fake;SigAlg=http%3A%2F%2Fwww.w3.org%2F2001%2F04%2Fxmldsig-more%23rsa-sha256"; const char query3[] = "SAMLRequest=fVHJasMwEP0Vo3tqRXY2YRvcOIFAl9CUHnopwpkkAllyNeMuf1%2FZaSG95PrmLfNmMlSNaWXZ0ck%2BwXsHSNFXYyzKYZCzzlvpFGqUVjWAkmq5K%2B%2FvpLjhsvWOXO0Mu5BcVyhE8KSdZdGmytnbNEmTBV%2Bli9ulKMt5KlbVfDkbizWfcVEmUxa9gMfAz1mQBxFiBxuLpCwFiIvxiE9H48mz4FJMZJq8sqgKHbRVNKhORK2MY71vJzFqezSw00f7GPLXztcw9M7ZQRmE3n0bFtQf8IcUWV9JDqm%2B%2BPXCYNUAqb0ilcWXhOx8zIdQe1NtndH1dx%2FTKLp%2BlR7R%2B9FhoMq2b4wEllhUGuM%2Blx4UhZ3Id8Di4pz5%2F2fFDw%3D%3D&RelayState=fake&SigAlg=http%3A%2F%2Fwww.w3.org%2F2001%2F04%2Fxmldsig-more%23rsa-sha256&Signature=rUJ%2B9wVSvdGSmZWGuGXgudAPV5KBxRfxRKraBWGIslBz2XreyNbQjSA47DhIfi%2Bxf0awIIGkKcieN3Qd5sqVn4wvFU8fsmfqrdtouYi46aKsj4W91N19TxJ%2BCgrP7ygVEGDaGdc%2BrCQC3%2FuoYTELXq0gYP7tHaXA%2FCaZHfx5Z159crpRxS6eabZ6BGf4ImxiKhE1FuYzKHeISEV1iSyvgx5%2FE8ydSO%2FSP6yA5Rck4JxVJWH6ImbswCVQ80qfqR4NoJ%2BxiZqilbDJnQaSKZggx%2FgjNVoX%2FMVW1FqEmgJNcZpSjNUQqy9u4veSllpxPc2aB%2FpiUjzpbq9XzyFDOQfkUQ%3D%3D"; + /* Deleting SigAlg & Signature fields */ + const char query4[] = "SAMLRequest=fVHJasMwEP0Vo3tqRXY2YRvcOIFAl9CUHnopwpkkAllyNeMuf1%2FZaSG95PrmLfNmMlSNaWXZ0ck%2BwXsHSNFXYyzKYZCzzlvpFGqUVjWAkmq5K%2B%2FvpLjhsvWOXO0Mu5BcVyhE8KSdZdGmytnbNEmTBV%2Bli9ulKMt5KlbVfDkbizWfcVEmUxa9gMfAz1mQBxFiBxuLpCwFiIvxiE9H48mz4FJMZJq8sqgKHbRVNKhORK2MY71vJzFqezSw00f7GPLXztcw9M7ZQRmE3n0bFtQf8IcUWV9JDqm%2B%2BPXCYNUAqb0ilcWXhOx8zIdQe1NtndH1dx%2FTKLp%2BlR7R%2B9FhoMq2b4wEllhUGuM%2Blx4UhZ3Id8Di4pz5%2F2fFDw%3D%3D&RelayState=fake"; /* sp5-saml2 key */ const char pkey[] = "-----BEGIN CERTIFICATE-----\n\ MIIDnjCCAoagAwIBAgIBATANBgkqhkiG9w0BAQUFADBUMQswCQYDVQQGEwJGUjEP\n\ @@ -324,6 +326,11 @@ LlTxKnCrWAXftSm1rNtewTsF\n\ /* test reordering and semi-colon separator support */ ck_assert_msg(lasso_saml2_query_verify_signature(query2, key) == 0, "Disordered signature was not validated"); ck_assert_msg(lasso_saml2_query_verify_signature(query3, key) != 0, "Altered signature was validated"); + /* test error codes */ + ck_assert_msg(lasso_saml2_query_verify_signature(query3, key) == LASSO_DS_ERROR_INVALID_SIGNATURE, + "Altered signature do not lead to invalid signature"); + ck_assert_msg(lasso_saml2_query_verify_signature(query4, key) == LASSO_DS_ERROR_SIGNATURE_NOT_FOUND, + "Bad error code when missing signature"); xmlSecKeyDestroy(key); } END_TEST ```
bdauvergne commented 2024-04-11 12:16:39 +02:00
Author
Owner
Copy Link

Est-ce que ça serait une bonne idée d'ajouter un test ?

Par exemple

Je veux bien mais ton diff ne s'applique pas sur ma branche.

        xmlSecKeyDestroy(key);
 }
 END_TEST
patching file tests/random_tests.c
Hunk #1 FAILED at 292.
Hunk #2 FAILED at 324.
2 out of 2 hunks FAILED -- saving rejects to file tests/random_tests.c.rej
> Est-ce que ça serait une bonne idée d'ajouter un test ? > > Par exemple > > Je veux bien mais ton diff ne s'applique pas sur ma branche. ``` xmlSecKeyDestroy(key); } END_TEST patching file tests/random_tests.c Hunk #1 FAILED at 292. Hunk #2 FAILED at 324. 2 out of 2 hunks FAILED -- saving rejects to file tests/random_tests.c.rej ```
yweber commented 2024-04-11 12:28:39 +02:00
Owner
Copy Link

Je veux bien mais ton diff ne s'applique pas sur ma branche.

Mouarf, désolé, j'ai refais le diff en étant sur ta branche, mais c'est bizarre, je ne vois pas vraiment de différence avec le précédent.

C'est mieux ?

> Je veux bien mais ton diff ne s'applique pas sur ma branche. Mouarf, désolé, j'ai refais le diff en étant sur ta branche, mais c'est bizarre, je ne vois pas vraiment de différence avec le précédent. C'est mieux ?
test.patch
3.3 KiB
bdauvergne force-pushed wip/89371-When-using-lasso-as-IDP-missing from 16d7266902 to fe27e52da0 2024-04-15 15:38:21 +02:00 Compare
bdauvergne commented 2024-04-15 15:38:33 +02:00
Author
Owner
Copy Link

Je veux bien mais ton diff ne s'applique pas sur ma branche.

Mouarf, désolé, j'ai refais le diff en étant sur ta branche, mais c'est bizarre, je ne vois pas vraiment de différence avec le précédent.

C'est mieux ?

Ouaip.

> > Je veux bien mais ton diff ne s'applique pas sur ma branche. > > Mouarf, désolé, j'ai refais le diff en étant sur ta branche, mais c'est bizarre, je ne vois pas vraiment de différence avec le précédent. > > C'est mieux ? Ouaip.
bdauvergne requested review from yweber 2024-04-15 15:38:53 +02:00
yweber approved these changes 2024-04-15 16:28:48 +02:00
bdauvergne merged commit fe27e52da0 into main 2024-04-15 17:25:30 +02:00
bdauvergne deleted branch wip/89371-When-using-lasso-as-IDP-missing 2024-04-15 17:25:30 +02:00
Sign in to join this conversation.
Reviewers
No reviewers
tnoel
yweber
Labels
Clear labels
No items
No Label
Milestone
Clear milestone
No items
No Milestone
Assignees
Clear assignees
No Assignees
3 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: entrouvert/lasso#13
No description provided.
Delete Branch "wip/89371-When-using-lasso-as-IDP-missing"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?