Closed powershellshock closed 7 years ago
Hi,
Thanks for the massive PR, it will take me a few days to work through all of the changes and validate them. Currently there are no massive issues that are jumping out at me, but I can see a few minor changes that I will want to make.
I can also see some other potential optimizations that we can also implement.
I am going to merge this into a new branch called version 3. As this is such a dramatic change, I think it is time for a version increment!
The env:userdomain var does not necessarily match the computer domain name in enterprise Windows environments. The logged on user could be in a different AD domain than the computer. The IP global properties domain is always the domain of the computer.
If you still want to use env:computer for the his portion of fqdn that would be ok but it is possible for env vars to be modified per session whereas the IP global properties are consistent across all sessions.
Please excuse the brevity of this message from my phone.
From: Kieran Jacobsen notifications@github.com Sent: Sunday, April 23, 2017 2:56:47 AM To: poshsecurity/Posh-SYSLOG Cc: Jared Poeppelman; Author Subject: Re: [poshsecurity/Posh-SYSLOG] TCP, pipelining, msg sizes, fqdn and static IPs (#7)
@kjacobsen commented on this pull request.
Evaluate the facility and severity based on the enum types
- $Facility_Number = $Facility.value__
- $Severity_Number = $Severity.value__
- Write-Verbose -Message "Syslog Facility, $Facility_Number, Severity is $Severity_Number"
- Windows should always, in the worst case, have a result at 3, the hostname or computer name from which this command is run.
>
Get the IP global proprties of the local machine, to determine if an FQDN is available
$ComputerName = [System.Net.Dns]::GetHostByName($Env:COMPUTERNAME).HostName
([System.Net.NetworkInformation.IPGlobalProperties]::GetIPGlobalProperties().Hostname) + '.' + ([System.Net.NetworkInformation.IPGlobalProperties]::GetIPGlobalProperties().DomainName)
- $IpGlobalProps = [System.Net.NetworkInformation.IPGlobalProperties]::GetIPGlobalProperties()
What are the advantages of using [System.Net.NetworkInformation.IPGlobalProperties]::GetIPGlobalProperties() over the Environment Variable?
— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHubhttps://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fposhsecurity%2FPosh-SYSLOG%2Fpull%2F7%23pullrequestreview-34172900&data=02%7C01%7Cjpoeppel%40microsoft.com%7Ca4351482ca044a68ef1708d48a15e984%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636285274093105616&sdata=lBQuqdwXeF1N7HAbLdGQKzpBNV5CmhQpvmqbpOyvAoI%3D&reserved=0, or mute the threadhttps://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FASk6i89DoLe02CXJNfhmNJiDpi1Li3bJks5ryvYvgaJpZM4NCO02&data=02%7C01%7Cjpoeppel%40microsoft.com%7Ca4351482ca044a68ef1708d48a15e984%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636285274093105616&sdata=5ecWoFbHSeNpFO0qKdtY1pKlIyGvefYW%2FasKhF6LRIc%3D&reserved=0.
I guess the issue with moving away from the Environment variable is that we will need to rethink how we perform our testing. The Pester tests for the determination of the hostname to send relied on the fact we could change the environment variable for that session.
Looking on Google, seems there could be other issues with using the environment variable: https://stackoverflow.com/questions/12268885/powershell-get-fqdn-hostname
I think the most correct approach is probably something like
$Win32_Computer = Get-WmiObject win32_computersystem
# Is part of domain?
$Win32_Computer.partofdomain
#FQDN
$Win32_Computer.DNSHostName+$Win32_Computer.Domain
Only things that worry me when I really think about this is what if the network interface has its own domain name specified, or if the machine isn't domain joined but a hostname has been specified via DHCP or even manually via system properties.
I guess we could handle those when we hit those issues.
Now that is faster, and more testible, I took a look at the logic behind the static ip address side of the code. Found a way much better method.
Function Get-SyslogHostname
{
Param
(
# Socket of the Client
[Parameter(Mandatory = $true)]
[Net.Sockets.Socket]
$Socket
)
<#
According to RFC 5424 (section 6.2.4), we need to send our HOSTNAME field as one of these 5 (in order of priority)
1. FQDN
2. Static IP address
3. Hostname - Windows always has one of these, so this is our last resort
4. Dynamic IP address - We will never get to this one
5. the NILVALUE - or this one
Windows should always, in the worst case, have a result at 3, the hostname or computer name from which this command is run.
#>
# Get the Win32_ComputerSystem object
$Win32_ComputerSystem = Get-WmiObject win32_computersystem
if ($Win32_ComputerSystem.partofdomain) # If domain joined
{
# Use HOSTNAME Option 1 (FQDN), per RFC 5424 (section 6.2.4)
$Hostname = '{0}.{1}' -f $Win32_ComputerSystem.DNSHostname, $Win32_ComputerSystem.Domain
Write-Verbose -Message ('The machine is joined to an Active Directory domain, hostname value will be FQDN: {0}' -f $Hostname)
}
else
{
# Ask the appropriate client what the local endpoint address is
$LocalEndPoint = $Socket.LocalEndpoint.Address.IPAddressToString
# Get the adapter that the endpoint is assigned to
$NetworkAdapter = Get-NetIPAddress -IPAddress $LocalEndPoint
# Is that local endpoint a statically assigned ip address?
if ($NetworkAdapter.PrefixOrigin -eq 'Manual')
{
# Use HOSTNAME Option 2 (Static IP address), per RFC 5424 (section 6.2.4)
$Hostname = $LocalEndPoint
Write-Verbose -Message ('A statically assigned IP was detected as the source for the route to {0}, so the static IP ({1}) will be used as the HOSTNAME value.' -f $Server, $Hostname)
}
else
{
# Use HOSTNAME Option 3 (hostname), per RFC 5424 (section 6.2.4)
$Hostname = $Env:COMPUTERNAME
Write-Verbose -Message ('The hostname ({0}) will be used as the HOSTNAME value.' -f $Hostname)
}
}
Write-Debug -Message ('Get-SyslogHostname is returning value {0}' -f $Hostname)
$Hostname
}
Initial testing shows this is significantly faster than the original logic.
Next thing to ponder. The maximum length for TCP is 2048 bytes, do you know if that includes the framing?
For instance if I select Octet framing, does the 2048 byte limit include the OctetCount (and the following space), if so, we might need to trim have some additional logic.
Just to let you know, spent a bit of time today on this.
Still working through some restructuring, but some work in progress can be found on the version3 branch (https://github.com/poshsecurity/Posh-SYSLOG/blob/version3/Functions/Send-SyslogMessage.ps1).
Ok, so I think we are now in a ready state. I need to write more test cases, but I think the majority of the stuff that you have added, and then some cleanup I have wanted is now in there.
Could you please download the module from this branch, https://github.com/poshsecurity/Posh-SYSLOG/tree/version3, and confirm that everything is functioning as you would expect?
Once we know it is all up and running, I will push into master.
TCP transport support, pipelining optimizations for high volume scenarios, applied max message sizes per RFC (they are different), fqdn detection enhancement, static IP detection enhancement including switch param to enable it (disabled by default, cuz its slow), tests for TCP transport and large message truncation, had to remove old tests for FQDN detection because it is not based on environment vars anymore