From e95120b2f88b32f04f427bed2c66a957015999a0 Mon Sep 17 00:00:00 2001 From: SujathaH Date: Mon, 16 Oct 2023 16:09:32 +0530 Subject: [PATCH 1/5] Do not support shortening of HMAC code --- .../jenkins/security/HMACConfidentialKey.java | 5 +++ .../security/HMACConfidentialKeyTest.java | 37 +++++++++++++++++++ 2 files changed, 42 insertions(+) diff --git a/core/src/main/java/jenkins/security/HMACConfidentialKey.java b/core/src/main/java/jenkins/security/HMACConfidentialKey.java index 9faf2f629f84..a6c4633ce649 100644 --- a/core/src/main/java/jenkins/security/HMACConfidentialKey.java +++ b/core/src/main/java/jenkins/security/HMACConfidentialKey.java @@ -98,6 +98,11 @@ public boolean checkMac(String message, String mac) { } private byte[] chop(byte[] mac) { + //don't shorten the mac code on FIPS mode + //if length supplied is less than original mac code length on FIPS, throw exception + if (FIPS140.useCompliantAlgorithms() && length < mac.length) { + throw new IllegalArgumentException("Supplied length can't be less than " + mac.length + " on FIPS mode"); + } if (mac.length <= length) return mac; // already too short byte[] b = new byte[length]; diff --git a/core/src/test/java/jenkins/security/HMACConfidentialKeyTest.java b/core/src/test/java/jenkins/security/HMACConfidentialKeyTest.java index 93b5244f76b1..04c2a65249b0 100644 --- a/core/src/test/java/jenkins/security/HMACConfidentialKeyTest.java +++ b/core/src/test/java/jenkins/security/HMACConfidentialKeyTest.java @@ -1,8 +1,10 @@ package jenkins.security; + import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; import java.util.Set; import java.util.TreeSet; @@ -38,4 +40,39 @@ public void loadingExistingKey() { } } + + @Test + public void testMacWithShortenedCodeOnFips() { + System.setProperty("jenkins.security.FIPS140.COMPLIANCE", "true"); + HMACConfidentialKey key1 = new HMACConfidentialKey("test", 16); + try { + String str = key1.mac("Hello World"); + } catch (IllegalArgumentException exception) { + assertEquals("Supplied length can't be less than 32 on FIPS mode", exception.getMessage()); + } + } + + @Test + public void testMacWithShortenedCodeOnNonFips() { + System.setProperty("jenkins.security.FIPS140.COMPLIANCE", "false"); + HMACConfidentialKey key1 = new HMACConfidentialKey("test", 16); + try { + String str = key1.mac("Hello World"); + assertTrue(str, str.matches("[0-9A-Fa-f]{32}")); + } catch (IllegalArgumentException exception) { + fail("Found IllegalArgumentException"); + } + } + + @Test + public void testCompleteMaCodeOnNonFips() { + System.setProperty("jenkins.security.FIPS140.COMPLIANCE", "true"); + HMACConfidentialKey key1 = new HMACConfidentialKey("test", 32); + try { + String str = key1.mac("Hello World"); + assertTrue(str, str.matches("[0-9A-Fa-f]{64}")); + } catch (IllegalArgumentException exception) { + fail("Found IllegalArgumentException"); + } + } } From dd258b1e586280119bf66dc98728586e11063b67 Mon Sep 17 00:00:00 2001 From: SujathaH Date: Tue, 17 Oct 2023 15:02:20 +0530 Subject: [PATCH 2/5] Moved FIPS related unit test cases to a new file --- .../security/HMACConfidentialKeyTest.java | 37 ++----------------- .../security/HMACConfidentialKeyFIPSTest.java | 34 +++++++++++++++++ 2 files changed, 37 insertions(+), 34 deletions(-) create mode 100644 test/src/test/java/hudson/security/HMACConfidentialKeyFIPSTest.java diff --git a/core/src/test/java/jenkins/security/HMACConfidentialKeyTest.java b/core/src/test/java/jenkins/security/HMACConfidentialKeyTest.java index 04c2a65249b0..68bd70a7c3b9 100644 --- a/core/src/test/java/jenkins/security/HMACConfidentialKeyTest.java +++ b/core/src/test/java/jenkins/security/HMACConfidentialKeyTest.java @@ -1,10 +1,8 @@ package jenkins.security; - import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; -import static org.junit.Assert.fail; import java.util.Set; import java.util.TreeSet; @@ -40,39 +38,10 @@ public void loadingExistingKey() { } } - - @Test - public void testMacWithShortenedCodeOnFips() { - System.setProperty("jenkins.security.FIPS140.COMPLIANCE", "true"); - HMACConfidentialKey key1 = new HMACConfidentialKey("test", 16); - try { - String str = key1.mac("Hello World"); - } catch (IllegalArgumentException exception) { - assertEquals("Supplied length can't be less than 32 on FIPS mode", exception.getMessage()); - } - } - @Test - public void testMacWithShortenedCodeOnNonFips() { - System.setProperty("jenkins.security.FIPS140.COMPLIANCE", "false"); + public void testTruncatedMacOnNonFips() { HMACConfidentialKey key1 = new HMACConfidentialKey("test", 16); - try { - String str = key1.mac("Hello World"); - assertTrue(str, str.matches("[0-9A-Fa-f]{32}")); - } catch (IllegalArgumentException exception) { - fail("Found IllegalArgumentException"); - } - } - - @Test - public void testCompleteMaCodeOnNonFips() { - System.setProperty("jenkins.security.FIPS140.COMPLIANCE", "true"); - HMACConfidentialKey key1 = new HMACConfidentialKey("test", 32); - try { - String str = key1.mac("Hello World"); - assertTrue(str, str.matches("[0-9A-Fa-f]{64}")); - } catch (IllegalArgumentException exception) { - fail("Found IllegalArgumentException"); - } + String str = key1.mac("Hello World"); + assertTrue(str, str.matches("[0-9A-Fa-f]{32}")); } } diff --git a/test/src/test/java/hudson/security/HMACConfidentialKeyFIPSTest.java b/test/src/test/java/hudson/security/HMACConfidentialKeyFIPSTest.java new file mode 100644 index 000000000000..e0a09cfdd89b --- /dev/null +++ b/test/src/test/java/hudson/security/HMACConfidentialKeyFIPSTest.java @@ -0,0 +1,34 @@ +package hudson.security; + + +import static org.junit.Assert.assertThrows; + +import org.junit.ClassRule; +import org.junit.Test; +import org.junit.rules.TestRule; +import org.jvnet.hudson.test.FlagRule; + +import jenkins.security.HMACConfidentialKey; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; + +public class HMACConfidentialKeyFIPSTest { + @ClassRule + // do not use the FIPS140 class here as that initializes the field before we set the property! + public static TestRule flagRule = FlagRule.systemProperty("jenkins.security.FIPS140.COMPLIANCE", "true"); + + @Test + public void testTruncatedMacOnFips() { + HMACConfidentialKey key1 = new HMACConfidentialKey("test", 16); + IllegalArgumentException iae = assertThrows(IllegalArgumentException.class, () -> key1.mac("Hello World")); + assertEquals("Supplied length can't be less than 32 on FIPS mode", iae .getMessage()); + } + + @Test + public void testCompleteMacOnNonFips() { + HMACConfidentialKey key1 = new HMACConfidentialKey("test", 32); + String str = key1.mac("Hello World"); + assertTrue(str, str.matches("[0-9A-Fa-f]{64}")); + } +} From 54477f0bfa833e3f77e7a59234320ec9cb5e3b8a Mon Sep 17 00:00:00 2001 From: SujathaH Date: Tue, 17 Oct 2023 16:19:34 +0530 Subject: [PATCH 3/5] Moved FIPS related unit test cases to a new file --- .../java/jenkins/security/HMACConfidentialKeyTest.java | 5 ++++- .../hudson/security/HMACConfidentialKeyFIPSTest.java | 9 +++++---- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/core/src/test/java/jenkins/security/HMACConfidentialKeyTest.java b/core/src/test/java/jenkins/security/HMACConfidentialKeyTest.java index 68bd70a7c3b9..a635ed67707e 100644 --- a/core/src/test/java/jenkins/security/HMACConfidentialKeyTest.java +++ b/core/src/test/java/jenkins/security/HMACConfidentialKeyTest.java @@ -1,5 +1,7 @@ package jenkins.security; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.matchesPattern; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; @@ -42,6 +44,7 @@ public void loadingExistingKey() { public void testTruncatedMacOnNonFips() { HMACConfidentialKey key1 = new HMACConfidentialKey("test", 16); String str = key1.mac("Hello World"); - assertTrue(str, str.matches("[0-9A-Fa-f]{32}")); + String pattern = "[0-9A-Fa-f]{32}"; + assertThat(str, matchesPattern(pattern)); } } diff --git a/test/src/test/java/hudson/security/HMACConfidentialKeyFIPSTest.java b/test/src/test/java/hudson/security/HMACConfidentialKeyFIPSTest.java index e0a09cfdd89b..08e6173371a7 100644 --- a/test/src/test/java/hudson/security/HMACConfidentialKeyFIPSTest.java +++ b/test/src/test/java/hudson/security/HMACConfidentialKeyFIPSTest.java @@ -1,6 +1,7 @@ package hudson.security; - +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.matchesPattern; import static org.junit.Assert.assertThrows; import org.junit.ClassRule; @@ -11,7 +12,6 @@ import jenkins.security.HMACConfidentialKey; import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertTrue; public class HMACConfidentialKeyFIPSTest { @ClassRule @@ -26,9 +26,10 @@ public void testTruncatedMacOnFips() { } @Test - public void testCompleteMacOnNonFips() { + public void testCompleteMacOnFips() { HMACConfidentialKey key1 = new HMACConfidentialKey("test", 32); String str = key1.mac("Hello World"); - assertTrue(str, str.matches("[0-9A-Fa-f]{64}")); + String pattern = "[0-9A-Fa-f]{64}"; + assertThat(str, matchesPattern(pattern)); } } From 1a473c606d778a1eee13bcafd3ea7681c1afa302 Mon Sep 17 00:00:00 2001 From: SujathaH Date: Tue, 17 Oct 2023 16:22:36 +0530 Subject: [PATCH 4/5] Moved FIPS related unit test cases to a new file --- .../test/java/hudson/security/HMACConfidentialKeyFIPSTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/src/test/java/hudson/security/HMACConfidentialKeyFIPSTest.java b/test/src/test/java/hudson/security/HMACConfidentialKeyFIPSTest.java index 08e6173371a7..1c20612df83a 100644 --- a/test/src/test/java/hudson/security/HMACConfidentialKeyFIPSTest.java +++ b/test/src/test/java/hudson/security/HMACConfidentialKeyFIPSTest.java @@ -22,7 +22,7 @@ public class HMACConfidentialKeyFIPSTest { public void testTruncatedMacOnFips() { HMACConfidentialKey key1 = new HMACConfidentialKey("test", 16); IllegalArgumentException iae = assertThrows(IllegalArgumentException.class, () -> key1.mac("Hello World")); - assertEquals("Supplied length can't be less than 32 on FIPS mode", iae .getMessage()); + assertEquals("Supplied length can't be less than 32 on FIPS mode", iae.getMessage()); } @Test From 773082874e4db6e43a3a2d72097c81dfba46f440 Mon Sep 17 00:00:00 2001 From: SujathaH Date: Tue, 17 Oct 2023 16:59:54 +0530 Subject: [PATCH 5/5] Fixed checkstyle issues in junit --- .../java/hudson/security/HMACConfidentialKeyFIPSTest.java | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/test/src/test/java/hudson/security/HMACConfidentialKeyFIPSTest.java b/test/src/test/java/hudson/security/HMACConfidentialKeyFIPSTest.java index 1c20612df83a..3c0f71b4e459 100644 --- a/test/src/test/java/hudson/security/HMACConfidentialKeyFIPSTest.java +++ b/test/src/test/java/hudson/security/HMACConfidentialKeyFIPSTest.java @@ -2,17 +2,15 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.matchesPattern; +import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertThrows; +import jenkins.security.HMACConfidentialKey; import org.junit.ClassRule; import org.junit.Test; import org.junit.rules.TestRule; import org.jvnet.hudson.test.FlagRule; -import jenkins.security.HMACConfidentialKey; - -import static org.junit.Assert.assertEquals; - public class HMACConfidentialKeyFIPSTest { @ClassRule // do not use the FIPS140 class here as that initializes the field before we set the property!