diff --git a/src/main/kotlin/core/packetproxy/extensions/securityheaders/checks/CookieCheck.kt b/src/main/kotlin/core/packetproxy/extensions/securityheaders/checks/CookieCheck.kt index 579b5510..29b00ac4 100644 --- a/src/main/kotlin/core/packetproxy/extensions/securityheaders/checks/CookieCheck.kt +++ b/src/main/kotlin/core/packetproxy/extensions/securityheaders/checks/CookieCheck.kt @@ -16,6 +16,8 @@ package packetproxy.extensions.securityheaders.checks import packetproxy.extensions.securityheaders.SecurityCheck +import packetproxy.extensions.securityheaders.SecurityCheck.HighlightSegment +import packetproxy.extensions.securityheaders.SecurityCheck.HighlightType import packetproxy.extensions.securityheaders.SecurityCheckResult import packetproxy.http.HttpHeader @@ -27,14 +29,27 @@ class CookieCheck : SecurityCheck { /** Check if a specific cookie line has the Secure flag */ @JvmStatic fun hasSecureFlag(cookieLine: String): Boolean { - return cookieLine.lowercase().contains("secure") + val cookieContent = cookieLine.substringAfter(":", cookieLine).trim() + + if (cookieContent.isEmpty()) { + return false + } + + val parts = cookieContent.split(";") + + return parts + .drop(1) + .map { it.trim().lowercase() } + .any { attr -> + val attrName = attr.split("=").first().trim() + attrName == "secure" + } } } override val name: String = "Cookies" override val columnName: String = "Cookies" override val failMessage: String = "Set-Cookie is missing 'Secure' flag" - override val greenPatterns: List = listOf("set-cookie:", "secure") override fun check(header: HttpHeader, context: MutableMap): SecurityCheckResult { val setCookies = header.getAllValue("Set-Cookie") @@ -50,13 +65,11 @@ class CookieCheck : SecurityCheck { val displayBuilder = StringBuilder() for (cookie in setCookies) { - if (!cookie.lowercase().contains(" secure")) { + if (!hasSecureFlag(cookie)) { allSecure = false } - // Truncate for display - val truncated = if (cookie.length > 100) cookie.substring(0, 100) + "..." else cookie - displayBuilder.append(truncated).append("; ") + displayBuilder.append(cookie).append("; ") } val displayValue = displayBuilder.toString() @@ -72,4 +85,23 @@ class CookieCheck : SecurityCheck { override fun matchesHeaderLine(headerLine: String): Boolean { return headerLine.startsWith("set-cookie:") } + + /** + * Highlight each Set-Cookie line based on whether it has the Secure flag. + * - Secure flag present: GREEN (secure) + * - Secure flag missing: RED (insecure) + */ + override fun getHighlightSegments( + headerLine: String, + result: SecurityCheckResult?, + ): List { + if (!matchesHeaderLine(headerLine.lowercase())) { + return emptyList() + } + + val hasSecure = hasSecureFlag(headerLine) + val highlightType = if (hasSecure) HighlightType.GREEN else HighlightType.RED + + return listOf(HighlightSegment(0, headerLine.length, highlightType)) + } } diff --git a/src/main/kotlin/core/packetproxy/extensions/securityheaders/ui/SecurityHeadersDetailPanel.kt b/src/main/kotlin/core/packetproxy/extensions/securityheaders/ui/SecurityHeadersDetailPanel.kt index 2c9b19ac..5b253b35 100644 --- a/src/main/kotlin/core/packetproxy/extensions/securityheaders/ui/SecurityHeadersDetailPanel.kt +++ b/src/main/kotlin/core/packetproxy/extensions/securityheaders/ui/SecurityHeadersDetailPanel.kt @@ -21,9 +21,11 @@ import javax.swing.JSplitPane import javax.swing.JTextPane import javax.swing.text.SimpleAttributeSet import javax.swing.text.StyledDocument +import packetproxy.common.FontManager import packetproxy.extensions.securityheaders.SecurityCheck import packetproxy.extensions.securityheaders.SecurityCheckResult import packetproxy.extensions.securityheaders.checks.CookieCheck +import packetproxy.gui.WrapEditorKit import packetproxy.http.HttpHeader /** @@ -38,17 +40,8 @@ class SecurityHeadersDetailPanel(private val securityChecks: List init { textStyles = TextStyles() - headerPane = - JTextPane().apply { - isEditable = false - background = Color.WHITE - } - - detailArea = - JTextPane().apply { - isEditable = false - background = Color.WHITE - } + headerPane = createWrappedTextPane() + detailArea = createWrappedTextPane() } fun createPanel(): JSplitPane { @@ -109,6 +102,15 @@ class SecurityHeadersDetailPanel(private val securityChecks: List } } + private fun createWrappedTextPane(): JTextPane { + return JTextPane().apply { + isEditable = false + background = Color.WHITE + setEditorKit(WrapEditorKit(byteArrayOf())) + font = FontManager.getInstance().font + } + } + private fun collectHighlightSegments( line: String, results: Map, diff --git a/src/test/kotlin/packetproxy/extensions/securityheaders/CookieCheckTest.kt b/src/test/kotlin/packetproxy/extensions/securityheaders/CookieCheckTest.kt index e344c256..5a21f9bd 100644 --- a/src/test/kotlin/packetproxy/extensions/securityheaders/CookieCheckTest.kt +++ b/src/test/kotlin/packetproxy/extensions/securityheaders/CookieCheckTest.kt @@ -18,6 +18,7 @@ package packetproxy.extensions.securityheaders import org.junit.jupiter.api.Assertions.* import org.junit.jupiter.api.BeforeEach import org.junit.jupiter.api.Test +import packetproxy.extensions.securityheaders.SecurityCheck.HighlightType import packetproxy.extensions.securityheaders.checks.CookieCheck class CookieCheckTest { @@ -98,11 +99,13 @@ class CookieCheckTest { @Test fun testCheck_SecureAtBeginning_Fail() { - // Malformed: "Secure" at beginning without space prefix + // Malformed: "Secure" at beginning, but this is not a valid cookie format + // RFC 6265 requires first part to be "name=value", so "Secure" is not recognized as an + // attribute val header = TestHttpHeader.withSetCookie("Secure; session=abc123") val result = check.check(header, context) - // Implementation checks for " secure" (with space), so this fails + // "Secure" is not recognized as an attribute (first part must be name=value), so this fails assertTrue(result.isFail) } @@ -112,7 +115,76 @@ class CookieCheckTest { val header = TestHttpHeader.withSetCookie("data=this_is_secure_data") val result = check.check(header, context) - assertTrue(result.isFail) // " secure" (with space) not found + assertTrue(result.isFail) // Secure attribute not found + } + + // ===== False Positive Prevention Tests ===== + + @Test + fun testCheck_CookieNameSecure_WithoutSecureAttribute_Fail() { + // Cookie name is "Secure" but no Secure attribute + val header = + TestHttpHeader.withSetCookie("Secure=Secure%3Dtrue; Path=/; HttpOnly; SameSite=Lax") + val result = check.check(header, context) + + assertTrue(result.isFail) + } + + @Test + fun testCheck_CookieNameSecureLowercase_WithoutSecureAttribute_Fail() { + // Cookie name is "secure" but no Secure attribute + val header = TestHttpHeader.withSetCookie("secure=Secure%3D1; Path=/; HttpOnly; SameSite=Lax") + val result = check.check(header, context) + + assertTrue(result.isFail) + } + + @Test + fun testCheck_CookieNameSecureSession_WithoutSecureAttribute_Fail() { + // Cookie name contains "Secure" but no Secure attribute + val header = + TestHttpHeader.withSetCookie("Secure_session=SECURE_FLAG; Path=/; HttpOnly; SameSite=Lax") + val result = check.check(header, context) + + assertTrue(result.isFail) + } + + @Test + fun testCheck_CookieNameSecureId_WithoutSecureAttribute_Fail() { + // Cookie name contains "Secure" but no Secure attribute + val header = + TestHttpHeader.withSetCookie("SecureId=SecureToken; Path=/; HttpOnly; SameSite=Lax") + val result = check.check(header, context) + + assertTrue(result.isFail) + } + + @Test + fun testCheck_CookieNameSecureCookie_WithoutSecureAttribute_Fail() { + // Cookie name contains "Secure" but no Secure attribute + val header = + TestHttpHeader.withSetCookie("SecureCookie=secure_value; Path=/; HttpOnly; SameSite=Lax") + val result = check.check(header, context) + + assertTrue(result.isFail) + } + + @Test + fun testCheck_CookieNameSecure_WithSecureAttribute_Ok() { + // Cookie name is "Secure" AND Secure attribute is present + val header = TestHttpHeader.withSetCookie("Secure=value; Secure; Path=/") + val result = check.check(header, context) + + assertTrue(result.isOk) + } + + @Test + fun testCheck_CookieNameSecureId_WithSecureAttribute_Ok() { + // Cookie name contains "Secure" AND Secure attribute is present + val header = TestHttpHeader.withSetCookie("SecureId=value; Path=/; Secure; HttpOnly") + val result = check.check(header, context) + + assertTrue(result.isOk) } @Test @@ -179,16 +251,17 @@ class CookieCheckTest { assertTrue(cookies.isEmpty()) } - // ===== Display Value Truncation ===== + // ===== Display Value ===== @Test - fun testCheck_LongCookieValue_Truncated() { + fun testCheck_LongCookieValue_NotTruncated() { val longValue = "a".repeat(100) - val header = TestHttpHeader.withSetCookie("session=$longValue; Secure") + val cookie = "session=$longValue; Secure" + val header = TestHttpHeader.withSetCookie(cookie) val result = check.check(header, context) assertTrue(result.isOk) - assertTrue(result.displayValue.contains("...")) + assertTrue(result.displayValue.contains(cookie)) } // ===== Static hasSecureFlag Method ===== @@ -209,9 +282,65 @@ class CookieCheckTest { } @Test - fun testHasSecureFlag_SecureInValue_True() { - // Note: This is a known limitation - it checks for substring - assertTrue(CookieCheck.hasSecureFlag("set-cookie: data=secure_value")) + fun testHasSecureFlag_SecureInValue_False() { + // "secure" appears in cookie value, not as attribute - should return false + assertFalse(CookieCheck.hasSecureFlag("set-cookie: data=secure_value")) + } + + @Test + fun testHasSecureFlag_CookieNameSecure_WithoutAttribute_False() { + // Cookie name is "Secure" but no Secure attribute + assertFalse(CookieCheck.hasSecureFlag("set-cookie: Secure=value; Path=/")) + } + + @Test + fun testHasSecureFlag_CookieNameSecure_WithAttribute_True() { + // Cookie name is "Secure" AND Secure attribute is present + assertTrue(CookieCheck.hasSecureFlag("set-cookie: Secure=value; Secure; Path=/")) + } + + @Test + fun testHasSecureFlag_CookieNameSecureId_WithoutAttribute_False() { + // Cookie name contains "Secure" but no Secure attribute + assertFalse(CookieCheck.hasSecureFlag("set-cookie: SecureId=token; Path=/")) + } + + @Test + fun testHasSecureFlag_CookieNameSecureId_WithAttribute_True() { + // Cookie name contains "Secure" AND Secure attribute is present + assertTrue(CookieCheck.hasSecureFlag("set-cookie: SecureId=token; Path=/; Secure")) + } + + @Test + fun testHasSecureFlag_SecureAttributeWithValue_True() { + // Secure attribute with value (should still match) + assertTrue(CookieCheck.hasSecureFlag("set-cookie: session=abc; Secure=true")) + } + + @Test + fun testHasSecureFlag_MultipleAttributes_True() { + // Multiple attributes including Secure + assertTrue(CookieCheck.hasSecureFlag("set-cookie: session=abc; Path=/; Secure; HttpOnly")) + } + + @Test + fun testHasSecureFlag_NoAttributes_False() { + // Cookie without any attributes + assertFalse(CookieCheck.hasSecureFlag("set-cookie: session=abc")) + } + + @Test + fun testHasSecureFlag_WithoutSetCookiePrefix_True() { + // Cookie content without "Set-Cookie:" prefix + assertTrue(CookieCheck.hasSecureFlag("session=abc; Secure")) + } + + @Test + fun testHasSecureFlag_CaseInsensitive_True() { + // Secure attribute in different cases + assertTrue(CookieCheck.hasSecureFlag("set-cookie: session=abc; SECURE")) + assertTrue(CookieCheck.hasSecureFlag("set-cookie: session=abc; SeCuRe")) + assertTrue(CookieCheck.hasSecureFlag("set-cookie: session=abc; secure")) } // ===== matchesHeaderLine ===== @@ -230,4 +359,112 @@ class CookieCheckTest { fun testMatchesHeaderLine_EmptyString_False() { assertFalse(check.matchesHeaderLine("")) } + + // ===== getHighlightSegments ===== + + @Test + fun testGetHighlightSegments_WithSecure_Green() { + val line = "set-cookie: session=abc123; Secure; HttpOnly" + val segments = check.getHighlightSegments(line, null) + + assertEquals(1, segments.size) + assertEquals(0, segments[0].start) + assertEquals(line.length, segments[0].end) + assertEquals(HighlightType.GREEN, segments[0].type) + } + + @Test + fun testGetHighlightSegments_WithoutSecure_Red() { + val line = "set-cookie: session=abc123; HttpOnly" + val segments = check.getHighlightSegments(line, null) + + assertEquals(1, segments.size) + assertEquals(0, segments[0].start) + assertEquals(line.length, segments[0].end) + assertEquals(HighlightType.RED, segments[0].type) + } + + @Test + fun testGetHighlightSegments_NonSetCookieHeader_Empty() { + val line = "cookie: session=abc123" + val segments = check.getHighlightSegments(line, null) + + assertTrue(segments.isEmpty()) + } + + @Test + fun testGetHighlightSegments_SecureUpperCase_Green() { + val line = "set-cookie: session=abc123; SECURE" + val segments = check.getHighlightSegments(line, null) + + assertEquals(1, segments.size) + assertEquals(HighlightType.GREEN, segments[0].type) + } + + @Test + fun testGetHighlightSegments_SecureInValue_Red() { + // "secure" in value should be RED (not an attribute) + val line = "set-cookie: data=this_is_secure_data" + val segments = check.getHighlightSegments(line, null) + + assertEquals(1, segments.size) + assertEquals(HighlightType.RED, segments[0].type) + } + + @Test + fun testGetHighlightSegments_CookieNameSecure_WithoutAttribute_Red() { + // Cookie name is "Secure" but no Secure attribute + val line = "set-cookie: Secure=value; Path=/" + val segments = check.getHighlightSegments(line, null) + + assertEquals(1, segments.size) + assertEquals(HighlightType.RED, segments[0].type) + } + + @Test + fun testGetHighlightSegments_CookieNameSecure_WithAttribute_Green() { + // Cookie name is "Secure" AND Secure attribute is present + val line = "set-cookie: Secure=value; Secure; Path=/" + val segments = check.getHighlightSegments(line, null) + + assertEquals(1, segments.size) + assertEquals(HighlightType.GREEN, segments[0].type) + } + + @Test + fun testGetHighlightSegments_CookieNameSecureId_WithoutAttribute_Red() { + // Cookie name contains "Secure" but no Secure attribute + val line = "set-cookie: SecureId=token; Path=/" + val segments = check.getHighlightSegments(line, null) + + assertEquals(1, segments.size) + assertEquals(HighlightType.RED, segments[0].type) + } + + @Test + fun testGetHighlightSegments_CookieNameSecureId_WithAttribute_Green() { + // Cookie name contains "Secure" AND Secure attribute is present + val line = "set-cookie: SecureId=token; Path=/; Secure" + val segments = check.getHighlightSegments(line, null) + + assertEquals(1, segments.size) + assertEquals(HighlightType.GREEN, segments[0].type) + } + + @Test + fun testGetHighlightSegments_IndependentOfResult() { + // Highlight is based on Secure flag presence, not on check result + val lineWithSecure = "set-cookie: session=abc123; Secure" + val lineWithoutSecure = "set-cookie: session=abc123; HttpOnly" + + // Even with FAIL result, line with Secure should be GREEN + val failResult = SecurityCheckResult.fail("test", "test") + val segmentsWithSecure = check.getHighlightSegments(lineWithSecure, failResult) + assertEquals(HighlightType.GREEN, segmentsWithSecure[0].type) + + // Even with OK result, line without Secure should be RED + val okResult = SecurityCheckResult.ok("test", "test") + val segmentsWithoutSecure = check.getHighlightSegments(lineWithoutSecure, okResult) + assertEquals(HighlightType.RED, segmentsWithoutSecure[0].type) + } }