jmpsec / osctrl

Fast and efficient osquery management
https://osctrl.net
MIT License
394 stars 51 forks source link

Error handling for non exist environment #505

Closed zhuoyuan-liu closed 2 months ago

zhuoyuan-liu commented 2 months ago
// Retrieve environment variable
    envVar := r.PathValue("env")
    if envVar == "" {
        h.Inc(metricEnrollErr)
        log.Println("Environment is missing")
        return
    }
    // Get environment
    env, err := h.Envs.Get(envVar)
    if err != nil {
        h.Inc(metricEnrollErr)
        log.Printf("error getting environment %v", err)
        return
    }
    // Check if environment accept enrolls
    if !env.AcceptEnrolls {
        h.Inc(metricEnrollErr)
        log.Printf("environment not enrolling %v", err)
        return
    }

I just found that the osctrl will return the default 200 status code if the environment does not exist. It makes confusing to the client side and it's hard for client-side debugging. I would suggest adding a proper status code and error message to the requests.

javuto commented 2 months ago

The idea behind this decision was to prevent recon attempts from an attacking actor. But I can make some changes to only respond to environments in osctrl-tls using the UUID, and not the name. And reply with a 404 if the environment does not exist? Would that help? Thanks!

zhuoyuan-liu commented 2 months ago

The idea makes sense to me. However, it is also a huge risk to expose osctrl publicly as a simple DDoS could bring it down. Also, this is a kind of tool for internal device management. In our setup, we deployed a nginx in front with mTLS to ensure all clients are trusted.

javuto commented 2 months ago

Implemented in https://github.com/jmpsec/osctrl/pull/511